[heap-profiler] Expose 'globalObjectsAsRoots' via inspector interface

This CL adds an argument to the heap profiler that allows to control
whether global objects (e.g. 'window' in JavaScript) are treated as
roots in the heap snapshot. Doing so hides blink-internal details and
is often a good choice when user-JS leaks are investigated. Sometimes,
however, this introduces spurious retainer cycles, which are hard to
debug.

Previously, this option was exposed as a V8 flag. The blink
implications of the build-time V8 flag are now available via
the new blink flag `enable_additional_blink_object_names`.

Tbr: hpayer@chromium.org
Bug: chromium:1034504
Change-Id: Ibe9412917ae598a3ff0c3dc956ab0bc179f50a21
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967387Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65491}

BUILD.gn

@@ -392,9 +392,11 @@ config("features") {
   }
   defines +=
       [ "V8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=${v8_typed_array_max_size_in_heap}" ]
-  if (v8_enable_raw_heap_snapshots) {
-    defines += [ "V8_ENABLE_RAW_HEAP_SNAPSHOTS" ]
-  }
+
+  assert(
+      !v8_enable_raw_heap_snapshots,
+      "This flag is deprecated and is now available through the inspector interface as an argument to profiler's method `takeHeapSnapshot`. Consider using blink's flag `enable_additional_blink_object_names` to get better naming of internal objects.")
+
   if (v8_enable_future) {
     defines += [ "V8_ENABLE_FUTURE" ]
   }

gni/v8.gni

@@ -30,8 +30,8 @@ declare_args() {
   # Support for backtrace_symbols on linux.
   v8_enable_backtrace = ""
 
-  # Enables raw heap snapshots containing internals. Used for debugging memory
-  # on platform and embedder level.
+  # This flag is deprecated and is now available through the inspector interface
+  # as an argument to profiler's method `takeHeapSnapshot`.
   v8_enable_raw_heap_snapshots = false
 
   # Enable several snapshots side-by-side (e.g. default and for trusted code).
@@ -69,8 +69,7 @@ if (v8_use_multi_snapshots) {
   # Silently disable multi snapshots if they're incompatible with the current
   # build configuration. This allows us to set v8_use_multi_snapshots=true on
   # all bots, and e.g. no-snapshot bots will automatically do the right thing.
-  v8_use_multi_snapshots =
-      v8_use_external_startup_data && !build_with_chromium
+  v8_use_multi_snapshots = v8_use_external_startup_data && !build_with_chromium
 }
 
 if (v8_enable_backtrace == "") {

include/js_protocol.pdl

@@ -653,11 +653,14 @@ experimental domain HeapProfiler
       # If true 'reportHeapSnapshotProgress' events will be generated while snapshot is being taken
       # when the tracking is stopped.
       optional boolean reportProgress
+      optional boolean treatGlobalObjectsAsRoots
 
   command takeHeapSnapshot
     parameters
       # If true 'reportHeapSnapshotProgress' events will be generated while snapshot is being taken.
       optional boolean reportProgress
+      # If true, a raw snapshot without artifical roots will be generated
+      optional boolean treatGlobalObjectsAsRoots
 
   event addHeapSnapshotChunk
     parameters

include/v8-profiler.h

@@ -844,7 +844,8 @@ class V8_EXPORT HeapProfiler {
    */
   const HeapSnapshot* TakeHeapSnapshot(
       ActivityControl* control = nullptr,
-      ObjectNameResolver* global_object_name_resolver = nullptr);
+      ObjectNameResolver* global_object_name_resolver = nullptr,
+      bool treat_global_objects_as_roots = true);
 
   /**
    * Starts tracking of heap objects population statistics. After calling

src/api/api.cc

@@ -10692,10 +10692,11 @@ void HeapProfiler::ClearObjectIds() {
 }
 
 const HeapSnapshot* HeapProfiler::TakeHeapSnapshot(
-    ActivityControl* control, ObjectNameResolver* resolver) {
+    ActivityControl* control, ObjectNameResolver* resolver,
+    bool treat_global_objects_as_roots) {
   return reinterpret_cast<const HeapSnapshot*>(
-      reinterpret_cast<i::HeapProfiler*>(this)->TakeSnapshot(control,
-                                                             resolver));
+      reinterpret_cast<i::HeapProfiler*>(this)->TakeSnapshot(
+          control, resolver, treat_global_objects_as_roots));
 }
 
 void HeapProfiler::StartTrackingHeapObjects(bool track_allocations) {

src/flags/flag-definitions.h

@@ -281,12 +281,6 @@ HARMONY_SHIPPING(FLAG_SHIPPING_FEATURES)
 DEFINE_BOOL(icu_timezone_data, true, "get information about timezones from ICU")
 #endif
 
-#ifdef V8_ENABLE_RAW_HEAP_SNAPSHOTS
-#define V8_ENABLE_RAW_HEAP_SNAPSHOTS_BOOL true
-#else
-#define V8_ENABLE_RAW_HEAP_SNAPSHOTS_BOOL false
-#endif  // V8_ENABLE_RAW_HEAP_SNAPSHOTS
-
 #ifdef V8_ENABLE_DOUBLE_CONST_STORE_CHECK
 #define V8_ENABLE_DOUBLE_CONST_STORE_CHECK_BOOL true
 #else
@@ -1671,9 +1665,6 @@ DEFINE_BOOL(unbox_double_fields, V8_DOUBLE_FIELDS_UNBOXING,
             "enable in-object double fields unboxing (64-bit only)")
 DEFINE_IMPLICATION(unbox_double_fields, track_double_fields)
 
-DEFINE_BOOL(raw_heap_snapshots, V8_ENABLE_RAW_HEAP_SNAPSHOTS_BOOL,
-            "enable raw heap snapshots contain garbage collection internals")
-
 // Cleanup...
 #undef FLAG_FULL
 #undef FLAG_READONLY

src/inspector/v8-heap-profiler-agent-impl.cc

@@ -187,9 +187,10 @@ Response V8HeapProfilerAgentImpl::startTrackingHeapObjects(
 }
 
 Response V8HeapProfilerAgentImpl::stopTrackingHeapObjects(
-    Maybe<bool> reportProgress) {
+    Maybe<bool> reportProgress, Maybe<bool> treatGlobalObjectsAsRoots) {
   requestHeapStatsUpdate();
-  takeHeapSnapshot(std::move(reportProgress));
+  takeHeapSnapshot(std::move(reportProgress),
+                   std::move(treatGlobalObjectsAsRoots));
   stopTrackingHeapObjectsInternal();
   return Response::OK();
 }
@@ -211,7 +212,8 @@ Response V8HeapProfilerAgentImpl::disable() {
   return Response::OK();
 }
 
-Response V8HeapProfilerAgentImpl::takeHeapSnapshot(Maybe<bool> reportProgress) {
+Response V8HeapProfilerAgentImpl::takeHeapSnapshot(
+    Maybe<bool> reportProgress, Maybe<bool> treatGlobalObjectsAsRoots) {
   v8::HeapProfiler* profiler = m_isolate->GetHeapProfiler();
   if (!profiler) return Response::Error("Cannot access v8 heap profiler");
   std::unique_ptr<HeapSnapshotProgress> progress;
@@ -219,8 +221,8 @@ Response V8HeapProfilerAgentImpl::takeHeapSnapshot(Maybe<bool> reportProgress) {
     progress.reset(new HeapSnapshotProgress(&m_frontend));
 
   GlobalObjectNameResolver resolver(m_session);
-  const v8::HeapSnapshot* snapshot =
-      profiler->TakeHeapSnapshot(progress.get(), &resolver);
+  const v8::HeapSnapshot* snapshot = profiler->TakeHeapSnapshot(
+      progress.get(), &resolver, treatGlobalObjectsAsRoots.fromMaybe(true));
   if (!snapshot) return Response::Error("Failed to take heap snapshot");
   HeapSnapshotOutputStream stream(&m_frontend);
   snapshot->Serialize(&stream);

src/inspector/v8-heap-profiler-agent-impl.h

@@ -31,11 +31,14 @@ class V8HeapProfilerAgentImpl : public protocol::HeapProfiler::Backend {
 
   Response enable() override;
   Response startTrackingHeapObjects(Maybe<bool> trackAllocations) override;
-  Response stopTrackingHeapObjects(Maybe<bool> reportProgress) override;
+  Response stopTrackingHeapObjects(
+      Maybe<bool> reportProgress,
+      Maybe<bool> treatGlobalObjectsAsRoots) override;
 
   Response disable() override;
 
-  Response takeHeapSnapshot(Maybe<bool> reportProgress) override;
+  Response takeHeapSnapshot(Maybe<bool> reportProgress,
+                            Maybe<bool> treatGlobalObjectsAsRoots) override;
 
   Response getObjectByHeapObjectId(
       const String16& heapSnapshotObjectId, Maybe<String16> objectGroup,

src/profiler/heap-profiler.cc

@@ -64,8 +64,9 @@ void HeapProfiler::BuildEmbedderGraph(Isolate* isolate,
 
 HeapSnapshot* HeapProfiler::TakeSnapshot(
     v8::ActivityControl* control,
-    v8::HeapProfiler::ObjectNameResolver* resolver) {
-  HeapSnapshot* result = new HeapSnapshot(this);
+    v8::HeapProfiler::ObjectNameResolver* resolver,
+    bool treat_global_objects_as_roots) {
+  HeapSnapshot* result = new HeapSnapshot(this, treat_global_objects_as_roots);
   {
     HeapSnapshotGenerator generator(result, control, resolver, heap());
     if (!generator.GenerateSnapshot()) {

src/profiler/heap-profiler.h

@@ -29,9 +29,9 @@ class HeapProfiler : public HeapObjectAllocationTracker {
   explicit HeapProfiler(Heap* heap);
   ~HeapProfiler() override;
 
-  HeapSnapshot* TakeSnapshot(
-      v8::ActivityControl* control,
-      v8::HeapProfiler::ObjectNameResolver* resolver);
+  HeapSnapshot* TakeSnapshot(v8::ActivityControl* control,
+                             v8::HeapProfiler::ObjectNameResolver* resolver,
+                             bool treat_global_objects_as_roots);
 
   bool StartSamplingHeapProfiler(uint64_t sample_interval, int stack_depth,
                                  v8::HeapProfiler::SamplingFlags);

src/profiler/heap-snapshot-generator.cc

@@ -181,7 +181,9 @@ const char* HeapEntry::TypeAsString() {
   }
 }
 
-HeapSnapshot::HeapSnapshot(HeapProfiler* profiler) : profiler_(profiler) {
+HeapSnapshot::HeapSnapshot(HeapProfiler* profiler, bool global_objects_as_roots)
+    : profiler_(profiler),
+      treat_global_objects_as_roots_(global_objects_as_roots) {
   // 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) ||
@@ -1719,7 +1721,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 (FLAG_raw_heap_snapshots) return;
+  if (!snapshot_->treat_global_objects_as_roots()) 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

@@ -176,7 +176,7 @@ class HeapEntry {
 // HeapSnapshotGenerator fills in a HeapSnapshot.
 class HeapSnapshot {
  public:
-  explicit HeapSnapshot(HeapProfiler* profiler);
+  explicit HeapSnapshot(HeapProfiler* profiler, bool global_objects_as_roots);
   void Delete();
 
   HeapProfiler* profiler() const { return profiler_; }
@@ -194,6 +194,9 @@ 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_;
+  }
 
   void AddLocation(HeapEntry* entry, int scriptId, int line, int col);
   HeapEntry* AddEntry(HeapEntry::Type type,
@@ -225,6 +228,7 @@ 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_;
 
   DISALLOW_COPY_AND_ASSIGN(HeapSnapshot);
 };