[api] Rework heap snapshot exposing internals

- Repurpose flag `treat_global_objects_as_roots` when taking a heap
  snapshot for toggling whether internals should be exposed (to
  `hide_internals`).
- Use the toggle in creating heap snapshots for exposing class names
  as object names for C++ objects that have not explicitly been given a
  name.

Change-Id: I77d71babfdfe53269964fe81ed985037a431c28b
Bug: chromium:1321620
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3623740Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80391}

include/v8-profiler.h

@@ -977,14 +977,71 @@ class V8_EXPORT HeapProfiler {
     virtual ~ObjectNameResolver() = default;
   };
 
+  enum class HeapSnapshotMode {
+    /**
+     * Heap snapshot for regular developers.
+     */
+    kRegular,
+    /**
+     * Heap snapshot is exposing internals that may be useful for experts.
+     */
+    kExposeInternals,
+  };
+
+  enum class NumericsMode {
+    /**
+     * Numeric values are hidden as they are values of the corresponding
+     * objects.
+     */
+    kHideNumericValues,
+    /**
+     * Numeric values are exposed in artificial fields.
+     */
+    kExposeNumericValues
+  };
+
+  struct HeapSnapshotOptions final {
+    // Manually define default constructor here to be able to use it in
+    // `TakeSnapshot()` below.
+    // NOLINTNEXTLINE
+    HeapSnapshotOptions() {}
+
+    /**
+     * The control used to report intermediate progress to.
+     */
+    ActivityControl* control = nullptr;
+    /**
+     * The resolver used by the snapshot generator to get names for V8 objects.
+     */
+    ObjectNameResolver* global_object_name_resolver = nullptr;
+    /**
+     * Mode for taking the snapshot, see `HeapSnapshotMode`.
+     */
+    HeapSnapshotMode snapshot_mode = HeapSnapshotMode::kRegular;
+    /**
+     * Mode for dealing with numeric values, see `NumericsMode`.
+     */
+    NumericsMode numerics_mode = NumericsMode::kHideNumericValues;
+  };
+
   /**
-   * Takes a heap snapshot and returns it.
+   * Takes a heap snapshot.
+   *
+   * \returns the snapshot.
+   */
+  const HeapSnapshot* TakeHeapSnapshot(
+      const HeapSnapshotOptions& options = HeapSnapshotOptions());
+
+  /**
+   * Takes a heap snapshot. See `HeapSnapshotOptions` for details on the
+   * parameters.
+   *
+   * \returns the snapshot.
    */
   const HeapSnapshot* TakeHeapSnapshot(
-      ActivityControl* control = nullptr,
+      ActivityControl* control,
       ObjectNameResolver* global_object_name_resolver = nullptr,
-      bool treat_global_objects_as_roots = true,
-      bool capture_numeric_value = false);
+      bool hide_internals = true, bool capture_numeric_value = false);
 
   /**
    * Starts tracking of heap objects population statistics. After calling

src/api/api.cc

@@ -10229,12 +10229,24 @@ void HeapProfiler::ClearObjectIds() {
 }
 
 const HeapSnapshot* HeapProfiler::TakeHeapSnapshot(
-    ActivityControl* control, ObjectNameResolver* resolver,
-    bool treat_global_objects_as_roots, bool capture_numeric_value) {
+    const HeapSnapshotOptions& options) {
   return reinterpret_cast<const HeapSnapshot*>(
-      reinterpret_cast<i::HeapProfiler*>(this)->TakeSnapshot(
-          control, resolver, treat_global_objects_as_roots,
-          capture_numeric_value));
+      reinterpret_cast<i::HeapProfiler*>(this)->TakeSnapshot(options));
+}
+
+const HeapSnapshot* HeapProfiler::TakeHeapSnapshot(ActivityControl* control,
+                                                   ObjectNameResolver* resolver,
+                                                   bool hide_internals,
+                                                   bool capture_numeric_value) {
+  HeapSnapshotOptions options;
+  options.control = control;
+  options.global_object_name_resolver = resolver;
+  options.snapshot_mode = hide_internals ? HeapSnapshotMode::kRegular
+                                         : HeapSnapshotMode::kExposeInternals;
+  options.numerics_mode = capture_numeric_value
+                              ? NumericsMode::kExposeNumericValues
+                              : NumericsMode::kHideNumericValues;
+  return TakeHeapSnapshot(options);
 }
 
 void HeapProfiler::StartTrackingHeapObjects(bool track_allocations) {

src/heap/cppgc/heap-base.h

@@ -229,9 +229,10 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
   // Returns whether objects should derive their name from C++ class names. Also
   // requires build-time support through `CPPGC_SUPPORTS_OBJECT_NAMES`.
   HeapObjectNameForUnnamedObject name_of_unnamed_object() const {
-    // TODO(chromium:1321620): Wire up flag with heap snapshot if exposing
-    // internals is requested.
-    return HeapObjectNameForUnnamedObject::kUseClassNameIfSupported;
+    return name_for_unnamed_object_;
+  }
+  void set_name_of_unnamed_object(HeapObjectNameForUnnamedObject value) {
+    name_for_unnamed_object_ = value;
   }
 
  protected:
@@ -314,6 +315,9 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
   const SweepingType sweeping_support_;
   GenerationSupport generation_support_;
 
+  HeapObjectNameForUnnamedObject name_for_unnamed_object_ =
+      HeapObjectNameForUnnamedObject::kUseHiddenName;
+
   friend class MarkerBase::IncrementalMarkingTask;
   friend class cppgc::subtle::DisallowGarbageCollectionScope;
   friend class cppgc::subtle::NoGarbageCollectionScope;

src/heap/heap.cc

@@ -7566,5 +7566,18 @@ EmbedderStackStateScope::~EmbedderStackStateScope() {
   local_tracer_->embedder_stack_state_ = old_stack_state_;
 }
 
+CppClassNamesAsHeapObjectNameScope::CppClassNamesAsHeapObjectNameScope(
+    v8::CppHeap* heap)
+    : cpp_heap_(CppHeap::From(heap)),
+      saved_heap_object_name_value_(cpp_heap_->name_of_unnamed_object()) {
+  cpp_heap_->set_name_of_unnamed_object(
+      cppgc::internal::HeapObjectNameForUnnamedObject::
+          kUseClassNameIfSupported);
+}
+
+CppClassNamesAsHeapObjectNameScope::~CppClassNamesAsHeapObjectNameScope() {
+  cpp_heap_->set_name_of_unnamed_object(saved_heap_object_name_value_);
+}
+
 }  // namespace internal
 }  // namespace v8

src/heap/heap.h

@@ -41,6 +41,14 @@
 #include "src/utils/allocation.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"  // nogncheck
 
+namespace cppgc {
+namespace internal {
+
+enum class HeapObjectNameForUnnamedObject : uint8_t;
+
+}  // namespace internal
+}  // namespace cppgc
+
 namespace v8 {
 
 namespace debug {
@@ -2883,6 +2891,17 @@ class V8_EXPORT_PRIVATE V8_NODISCARD EmbedderStackStateScope final {
   const EmbedderHeapTracer::EmbedderStackState old_stack_state_;
 };
 
+class V8_NODISCARD CppClassNamesAsHeapObjectNameScope final {
+ public:
+  explicit CppClassNamesAsHeapObjectNameScope(v8::CppHeap* heap);
+  ~CppClassNamesAsHeapObjectNameScope();
+
+ private:
+  CppHeap* const cpp_heap_;
+  const cppgc::internal::HeapObjectNameForUnnamedObject
+      saved_heap_object_name_value_;
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/profiler/heap-profiler.cc

@@ -4,10 +4,13 @@
 
 #include "src/profiler/heap-profiler.h"
 
+#include "include/v8-profiler.h"
 #include "src/api/api-inl.h"
+#include "src/base/optional.h"
 #include "src/debug/debug.h"
 #include "src/heap/combined-heap.h"
 #include "src/heap/heap-inl.h"
+#include "src/heap/heap.h"
 #include "src/objects/js-array-buffer-inl.h"
 #include "src/profiler/allocation-tracker.h"
 #include "src/profiler/heap-snapshot-generator-inl.h"
@@ -79,14 +82,17 @@ v8::EmbedderGraph::Node::Detachedness HeapProfiler::GetDetachedness(
 }
 
 HeapSnapshot* HeapProfiler::TakeSnapshot(
-    v8::ActivityControl* control,
-    v8::HeapProfiler::ObjectNameResolver* resolver,
-    bool treat_global_objects_as_roots, bool capture_numeric_value) {
+    const v8::HeapProfiler::HeapSnapshotOptions options) {
   is_taking_snapshot_ = true;
-  HeapSnapshot* result = new HeapSnapshot(this, treat_global_objects_as_roots,
-                                          capture_numeric_value);
+  HeapSnapshot* result =
+      new HeapSnapshot(this, options.snapshot_mode, options.numerics_mode);
   {
-    HeapSnapshotGenerator generator(result, control, resolver, heap());
+    base::Optional<CppClassNamesAsHeapObjectNameScope> use_cpp_class_name;
+    if (result->expose_internals() && heap()->cpp_heap())
+      use_cpp_class_name.emplace(heap()->cpp_heap());
+
+    HeapSnapshotGenerator generator(
+        result, options.control, options.global_object_name_resolver, heap());
     if (!generator.GenerateSnapshot()) {
       delete result;
       result = nullptr;

src/profiler/heap-profiler.h

@@ -31,10 +31,8 @@ class HeapProfiler : public HeapObjectAllocationTracker {
   HeapProfiler(const HeapProfiler&) = delete;
   HeapProfiler& operator=(const HeapProfiler&) = delete;
 
-  HeapSnapshot* TakeSnapshot(v8::ActivityControl* control,
-                             v8::HeapProfiler::ObjectNameResolver* resolver,
-                             bool treat_global_objects_as_roots,
-                             bool capture_numeric_value);
+  HeapSnapshot* TakeSnapshot(
+      const v8::HeapProfiler::HeapSnapshotOptions options);
 
   bool StartSamplingHeapProfiler(uint64_t sample_interval, int stack_depth,
                                  v8::HeapProfiler::SamplingFlags);

src/profiler/heap-snapshot-generator.cc

@@ -386,11 +386,12 @@ const char* HeapEntry::TypeAsString() const {
   }
 }
 
-HeapSnapshot::HeapSnapshot(HeapProfiler* profiler, bool global_objects_as_roots,
-                           bool capture_numeric_value)
+HeapSnapshot::HeapSnapshot(HeapProfiler* profiler,
+                           v8::HeapProfiler::HeapSnapshotMode snapshot_mode,
+                           v8::HeapProfiler::NumericsMode numerics_mode)
     : profiler_(profiler),
-      treat_global_objects_as_roots_(global_objects_as_roots),
-      capture_numeric_value_(capture_numeric_value) {
+      snapshot_mode_(snapshot_mode),
+      numerics_mode_(numerics_mode) {
   // It is very important to keep objects that form a heap snapshot
   // as small as possible. Check assumptions about data structure sizes.
   STATIC_ASSERT(kSystemPointerSize != 4 || sizeof(HeapGraphEdge) == 12);
@@ -2291,7 +2292,7 @@ void V8HeapExplorer::SetGcSubrootReference(Root root, const char* description,
 
   // For full heap snapshots we do not emit user roots but rather rely on
   // regular GC roots to retain objects.
-  if (!snapshot_->treat_global_objects_as_roots()) return;
+  if (snapshot_->expose_internals()) return;
 
   // Add a shortcut to JS global object reference at snapshot root.
   // That allows the user to easily find global objects. They are

src/profiler/heap-snapshot-generator.h

@@ -217,8 +217,9 @@ class HeapEntry {
 // HeapSnapshotGenerator fills in a HeapSnapshot.
 class HeapSnapshot {
  public:
-  explicit HeapSnapshot(HeapProfiler* profiler, bool global_objects_as_roots,
-                        bool capture_numeric_value);
+  HeapSnapshot(HeapProfiler* profiler,
+               v8::HeapProfiler::HeapSnapshotMode snapshot_mode,
+               v8::HeapProfiler::NumericsMode numerics_mode);
   HeapSnapshot(const HeapSnapshot&) = delete;
   HeapSnapshot& operator=(const HeapSnapshot&) = delete;
   void Delete();
@@ -240,10 +241,14 @@ class HeapSnapshot {
     return max_snapshot_js_object_id_;
   }
   bool is_complete() const { return !children_.empty(); }
-  bool treat_global_objects_as_roots() const {
-    return treat_global_objects_as_roots_;
+  bool capture_numeric_value() const {
+    return numerics_mode_ ==
+           v8::HeapProfiler::NumericsMode::kExposeNumericValues;
+  }
+  bool expose_internals() const {
+    return snapshot_mode_ ==
+           v8::HeapProfiler::HeapSnapshotMode::kExposeInternals;
   }
-  bool capture_numeric_value() const { return capture_numeric_value_; }
 
   void AddLocation(HeapEntry* entry, int scriptId, int line, int col);
   HeapEntry* AddEntry(HeapEntry::Type type,
@@ -275,8 +280,8 @@ class HeapSnapshot {
   std::unordered_map<SnapshotObjectId, HeapEntry*> entries_by_id_cache_;
   std::vector<SourceLocation> locations_;
   SnapshotObjectId max_snapshot_js_object_id_ = -1;
-  bool treat_global_objects_as_roots_;
-  bool capture_numeric_value_;
+  v8::HeapProfiler::HeapSnapshotMode snapshot_mode_;
+  v8::HeapProfiler::NumericsMode numerics_mode_;
 };
 
 

src/runtime/runtime-test.cc

@@ -6,6 +6,7 @@
 #include <memory>
 
 #include "include/v8-function.h"
+#include "include/v8-profiler.h"
 #include "src/api/api-inl.h"
 #include "src/base/numbers/double.h"
 #include "src/base/platform/mutex.h"
@@ -996,10 +997,10 @@ RUNTIME_FUNCTION(Runtime_TakeHeapSnapshot) {
   HeapProfiler* heap_profiler = isolate->heap_profiler();
   // Since this API is intended for V8 devs, we do not treat globals as roots
   // here on purpose.
-  HeapSnapshot* snapshot = heap_profiler->TakeSnapshot(
-      /* control = */ nullptr, /* resolver = */ nullptr,
-      /* treat_global_objects_as_roots = */ false,
-      /* capture_numeric_value = */ true);
+  v8::HeapProfiler::HeapSnapshotOptions options;
+  options.numerics_mode = v8::HeapProfiler::NumericsMode::kExposeNumericValues;
+  options.snapshot_mode = v8::HeapProfiler::HeapSnapshotMode::kExposeInternals;
+  HeapSnapshot* snapshot = heap_profiler->TakeSnapshot(options);
   FileOutputStream stream(filename.c_str());
   HeapSnapshotJSONSerializer serializer(snapshot);
   serializer.Serialize(&stream);

test/unittests/heap/cppgc-js/unified-heap-snapshot-unittest.cc

@@ -32,7 +32,8 @@ class UnifiedHeapSnapshotTest : public UnifiedHeapTest {
  public:
   const v8::HeapSnapshot* TakeHeapSnapshot() {
     v8::HeapProfiler* heap_profiler = v8_isolate()->GetHeapProfiler();
-    return heap_profiler->TakeHeapSnapshot();
+    return heap_profiler->TakeHeapSnapshot(nullptr, nullptr,
+                                           false /* hide internals */);
   }
 };
 

test/unittests/heap/cppgc/name-trait-unittest.cc

@@ -120,6 +120,8 @@ TEST(NameTraitTest, ParsingPrettyFunction) {
 class HeapObjectHeaderNameTest : public testing::TestWithHeap {};
 
 TEST_F(HeapObjectHeaderNameTest, LookupNameThroughGCInfo) {
+  Heap::From(GetHeap())->set_name_of_unnamed_object(
+      HeapObjectNameForUnnamedObject::kUseClassNameIfSupported);
   auto* no_name = MakeGarbageCollected<NoName>(GetAllocationHandle());
   auto no_name_tuple = HeapObjectHeader::FromObject(no_name).GetName();
   if (NameProvider::SupportsCppClassNamesAsObjectNames()) {