[unified-heap] Add finalization trigger

Allows embedders using the EmbedderHeapTracer to synchronously finalize
an already running garbage collection

Bug: chromium:843903
Tbr: ulan@chromium.org
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Iea01451ea2c1204c34dc7904732abae6b63e1704
Reviewed-on: https://chromium-review.googlesource.com/1128971
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54378}

include/v8.h

@@ -145,6 +145,7 @@ class DeferredHandles;
 class Heap;
 class HeapObject;
 class Isolate;
+class LocalEmbedderHeapTracer;
 class Object;
 struct ScriptStreamingData;
 template<typename T> class CustomArguments;
@@ -7065,6 +7066,23 @@ class V8_EXPORT EmbedderHeapTracer {
    */
   virtual void AbortTracing() = 0;
 
+  /*
+   * Called by the embedder to request immediaet finalization of the currently
+   * running tracing phase that has been started with TracePrologue and not
+   * yet finished with TraceEpilogue.
+   *
+   * Will be a noop when currently not in tracing.
+   *
+   * This is an experimental feature.
+   */
+  void FinalizeTracing();
+
+  /*
+   * Returns the v8::Isolate this tracer is attached too and |nullptr| if it
+   * is not attached to any v8::Isolate.
+   */
+  v8::Isolate* isolate() const { return isolate_; }
+
   /**
    * Returns the number of wrappers that are still to be traced by the embedder.
    */
@@ -7073,6 +7091,10 @@ class V8_EXPORT EmbedderHeapTracer {
 
  protected:
   virtual ~EmbedderHeapTracer() = default;
+
+  v8::Isolate* isolate_ = nullptr;
+
+  friend class internal::LocalEmbedderHeapTracer;
 };
 
 /**

src/api.cc

@@ -10636,6 +10636,16 @@ void Testing::DeoptimizeAll(Isolate* isolate) {
   i::Deoptimizer::DeoptimizeAll(i_isolate);
 }
 
+void EmbedderHeapTracer::FinalizeTracing() {
+  if (isolate_) {
+    i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
+    if (isolate->heap()->incremental_marking()->IsMarking()) {
+      isolate->heap()->CollectAllGarbage(
+          i::Heap::kNoGCFlags, i::GarbageCollectionReason::kExternalFinalize,
+          kNoGCCallbackFlags);
+    }
+  }
+}
 
 namespace internal {
 

src/heap/embedder-tracing.h

@@ -18,10 +18,23 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
  public:
   typedef std::pair<void*, void*> WrapperInfo;
 
-  LocalEmbedderHeapTracer()
-      : remote_tracer_(nullptr), num_v8_marking_worklist_was_empty_(0) {}
+  explicit LocalEmbedderHeapTracer(Isolate* isolate)
+      : isolate_(isolate),
+        remote_tracer_(nullptr),
+        num_v8_marking_worklist_was_empty_(0) {}
+
+  ~LocalEmbedderHeapTracer() {
+    if (remote_tracer_) remote_tracer_->isolate_ = nullptr;
+  }
+
+  void SetRemoteTracer(EmbedderHeapTracer* tracer) {
+    if (remote_tracer_) remote_tracer_->isolate_ = nullptr;
+
+    remote_tracer_ = tracer;
+    if (remote_tracer_)
+      remote_tracer_->isolate_ = reinterpret_cast<v8::Isolate*>(isolate_);
+  }
 
-  void SetRemoteTracer(EmbedderHeapTracer* tracer) { remote_tracer_ = tracer; }
   bool InUse() { return remote_tracer_ != nullptr; }
 
   void TracePrologue();
@@ -58,6 +71,7 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
  private:
   typedef std::vector<WrapperInfo> WrapperCache;
 
+  Isolate* const isolate_;
   EmbedderHeapTracer* remote_tracer_;
   WrapperCache cached_wrappers_to_trace_;
   size_t num_v8_marking_worklist_was_empty_;

src/heap/heap.cc

@@ -3664,6 +3664,8 @@ const char* Heap::GarbageCollectionReasonToString(
       return "snapshot creator";
     case GarbageCollectionReason::kTesting:
       return "testing";
+    case GarbageCollectionReason::kExternalFinalize:
+      return "external finalize";
     case GarbageCollectionReason::kUnknown:
       return "unknown";
   }
@@ -4680,7 +4682,7 @@ void Heap::SetUp() {
     dead_object_stats_ = new ObjectStats(this);
   }
   scavenge_job_ = new ScavengeJob();
-  local_embedder_heap_tracer_ = new LocalEmbedderHeapTracer();
+  local_embedder_heap_tracer_ = new LocalEmbedderHeapTracer(isolate());
 
   LOG(isolate_, IntPtrTEvent("heap-capacity", Capacity()));
   LOG(isolate_, IntPtrTEvent("heap-available", Available()));

src/heap/heap.h

@@ -242,7 +242,8 @@ enum class GarbageCollectionReason {
   kRuntime = 18,
   kSamplingProfiler = 19,
   kSnapshotCreator = 20,
-  kTesting = 21
+  kTesting = 21,
+  kExternalFinalize = 22
   // If you add new items here, then update the incremental_marking_reason,
   // mark_compact_reason, and scavenge_reason counters in counters.h.
   // Also update src/tools/metrics/histograms/histograms.xml in chromium.
@@ -913,14 +914,14 @@ class Heap {
   // Performs garbage collection operation.
   // Returns whether there is a chance that another major GC could
   // collect more garbage.
-  bool CollectGarbage(
+  V8_EXPORT_PRIVATE bool CollectGarbage(
       AllocationSpace space, GarbageCollectionReason gc_reason,
       const GCCallbackFlags gc_callback_flags = kNoGCCallbackFlags);
 
   // Performs a full garbage collection.  If (flags & kMakeHeapIterableMask) is
   // non-zero, then the slower precise sweeper is used, which leaves the heap
   // in a state where we can iterate over the heap visiting all objects.
-  void CollectAllGarbage(
+  V8_EXPORT_PRIVATE void CollectAllGarbage(
       int flags, GarbageCollectionReason gc_reason,
       const GCCallbackFlags gc_callback_flags = kNoGCCallbackFlags);
 

test/cctest/heap/test-embedder-tracing.cc

@@ -76,6 +76,22 @@ class TestEmbedderHeapTracer final : public v8::EmbedderHeapTracer {
   std::vector<v8::Persistent<v8::Object>*> to_register_with_v8_;
 };
 
+class TemporaryEmbedderHeapTracerScope {
+ public:
+  TemporaryEmbedderHeapTracerScope(v8::Isolate* isolate,
+                                   EmbedderHeapTracer* tracer)
+      : isolate_(isolate) {
+    isolate_->SetEmbedderHeapTracer(tracer);
+  }
+
+  ~TemporaryEmbedderHeapTracerScope() {
+    isolate_->SetEmbedderHeapTracer(nullptr);
+  }
+
+ private:
+  v8::Isolate* const isolate_;
+};
+
 }  // namespace
 
 TEST(V8RegisteringEmbedderReference) {
@@ -85,7 +101,7 @@ TEST(V8RegisteringEmbedderReference) {
   CcTest::InitializeVM();
   v8::Isolate* isolate = CcTest::isolate();
   TestEmbedderHeapTracer tracer(isolate);
-  isolate->SetEmbedderHeapTracer(&tracer);
+  TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   v8::HandleScope scope(isolate);
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
@@ -105,7 +121,7 @@ TEST(EmbedderRegisteringV8Reference) {
   CcTest::InitializeVM();
   v8::Isolate* isolate = CcTest::isolate();
   TestEmbedderHeapTracer tracer(isolate);
-  isolate->SetEmbedderHeapTracer(&tracer);
+  TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   v8::HandleScope scope(isolate);
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
@@ -139,7 +155,7 @@ TEST(TracingInRevivedSubgraph) {
   CcTest::InitializeVM();
   v8::Isolate* isolate = CcTest::isolate();
   TestEmbedderHeapTracer tracer(isolate);
-  isolate->SetEmbedderHeapTracer(&tracer);
+  TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   v8::HandleScope scope(isolate);
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
@@ -167,7 +183,7 @@ TEST(TracingInEphemerons) {
   CcTest::InitializeVM();
   v8::Isolate* isolate = CcTest::isolate();
   TestEmbedderHeapTracer tracer(isolate);
-  isolate->SetEmbedderHeapTracer(&tracer);
+  TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   v8::HandleScope scope(isolate);
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
@@ -192,6 +208,49 @@ TEST(TracingInEphemerons) {
   CHECK(tracer.IsRegisteredFromV8(first_field));
 }
 
+TEST(FinalizeTracingIsNoopWhenNotMarking) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  v8::Isolate* isolate = CcTest::isolate();
+  Isolate* i_isolate = CcTest::i_isolate();
+  TestEmbedderHeapTracer tracer(isolate);
+  TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
+
+  // Finalize a potentially running garbage collection.
+  i_isolate->heap()->CollectGarbage(OLD_SPACE,
+                                    GarbageCollectionReason::kTesting);
+  CHECK(i_isolate->heap()->incremental_marking()->IsStopped());
+
+  int gc_counter = i_isolate->heap()->gc_count();
+  tracer.FinalizeTracing();
+  CHECK(i_isolate->heap()->incremental_marking()->IsStopped());
+  CHECK_EQ(gc_counter, i_isolate->heap()->gc_count());
+}
+
+TEST(FinalizeTracingWhenMarking) {
+  ManualGCScope manual_gc;
+  CcTest::InitializeVM();
+  v8::Isolate* isolate = CcTest::isolate();
+  Isolate* i_isolate = CcTest::i_isolate();
+  TestEmbedderHeapTracer tracer(isolate);
+  TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
+
+  // Finalize a potentially running garbage collection.
+  i_isolate->heap()->CollectGarbage(OLD_SPACE,
+                                    GarbageCollectionReason::kTesting);
+  if (i_isolate->heap()->mark_compact_collector()->sweeping_in_progress()) {
+    i_isolate->heap()->mark_compact_collector()->EnsureSweepingCompleted();
+  }
+  CHECK(i_isolate->heap()->incremental_marking()->IsStopped());
+
+  i::IncrementalMarking* marking = i_isolate->heap()->incremental_marking();
+  marking->Start(i::GarbageCollectionReason::kTesting);
+  // Sweeping is not runing so we should immediately start marking.
+  CHECK(marking->IsMarking());
+  tracer.FinalizeTracing();
+  CHECK(marking->IsStopped());
+}
+
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8

test/unittests/heap/embedder-tracing-unittest.cc

@@ -3,11 +3,15 @@
 // found in the LICENSE file.
 
 #include "src/heap/embedder-tracing.h"
+#include "test/unittests/test-utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace v8 {
 namespace internal {
+
+using LocalEmbedderHeapTracerWithIsolate = TestWithIsolate;
+
 namespace heap {
 
 using testing::StrictMock;
@@ -38,14 +42,14 @@ class MockEmbedderHeapTracer : public EmbedderHeapTracer {
 };
 
 TEST(LocalEmbedderHeapTracer, InUse) {
-  LocalEmbedderHeapTracer local_tracer;
   MockEmbedderHeapTracer mock_remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&mock_remote_tracer);
   EXPECT_TRUE(local_tracer.InUse());
 }
 
 TEST(LocalEmbedderHeapTracer, NoRemoteTracer) {
-  LocalEmbedderHeapTracer local_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   // We should be able to call all functions without a remote tracer being
   // attached.
   EXPECT_FALSE(local_tracer.InUse());
@@ -59,32 +63,32 @@ TEST(LocalEmbedderHeapTracer, NoRemoteTracer) {
 }
 
 TEST(LocalEmbedderHeapTracer, TracePrologueForwards) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   EXPECT_CALL(remote_tracer, TracePrologue());
   local_tracer.TracePrologue();
 }
 
 TEST(LocalEmbedderHeapTracer, TraceEpilogueForwards) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   EXPECT_CALL(remote_tracer, TraceEpilogue());
   local_tracer.TraceEpilogue();
 }
 
 TEST(LocalEmbedderHeapTracer, AbortTracingForwards) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   EXPECT_CALL(remote_tracer, AbortTracing());
   local_tracer.AbortTracing();
 }
 
 TEST(LocalEmbedderHeapTracer, AbortTracingClearsCachedWrappers) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   local_tracer.AddWrapperToTrace(CreateWrapperInfo());
   EXPECT_CALL(remote_tracer, AbortTracing());
@@ -93,31 +97,31 @@ TEST(LocalEmbedderHeapTracer, AbortTracingClearsCachedWrappers) {
 }
 
 TEST(LocalEmbedderHeapTracer, EnterFinalPauseForwards) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   EXPECT_CALL(remote_tracer, EnterFinalPause());
   local_tracer.EnterFinalPause();
 }
 
 TEST(LocalEmbedderHeapTracer, IsRemoteTracingDoneIncludesRemote) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   EXPECT_CALL(remote_tracer, IsTracingDone());
   local_tracer.IsRemoteTracingDone();
 }
 
 TEST(LocalEmbedderHeapTracer, NumberOfCachedWrappersToTraceExcludesRemote) {
-  LocalEmbedderHeapTracer local_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
   local_tracer.SetRemoteTracer(&remote_tracer);
   local_tracer.NumberOfCachedWrappersToTrace();
 }
 
 TEST(LocalEmbedderHeapTracer, RegisterWrappersWithRemoteTracer) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   local_tracer.AddWrapperToTrace(CreateWrapperInfo());
   EXPECT_EQ(1u, local_tracer.NumberOfCachedWrappersToTrace());
@@ -129,8 +133,8 @@ TEST(LocalEmbedderHeapTracer, RegisterWrappersWithRemoteTracer) {
 }
 
 TEST(LocalEmbedderHeapTracer, TraceFinishes) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   local_tracer.AddWrapperToTrace(CreateWrapperInfo());
   EXPECT_EQ(1u, local_tracer.NumberOfCachedWrappersToTrace());
@@ -144,8 +148,8 @@ TEST(LocalEmbedderHeapTracer, TraceFinishes) {
 }
 
 TEST(LocalEmbedderHeapTracer, TraceDoesNotFinish) {
-  LocalEmbedderHeapTracer local_tracer;
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
   local_tracer.SetRemoteTracer(&remote_tracer);
   local_tracer.AddWrapperToTrace(CreateWrapperInfo());
   EXPECT_EQ(1u, local_tracer.NumberOfCachedWrappersToTrace());
@@ -158,6 +162,23 @@ TEST(LocalEmbedderHeapTracer, TraceDoesNotFinish) {
   EXPECT_EQ(0u, local_tracer.NumberOfCachedWrappersToTrace());
 }
 
+TEST_F(LocalEmbedderHeapTracerWithIsolate, SetRemoteTracerSetsIsolate) {
+  StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(isolate());
+  local_tracer.SetRemoteTracer(&remote_tracer);
+  EXPECT_EQ(isolate(), reinterpret_cast<Isolate*>(remote_tracer.isolate()));
+}
+
+TEST_F(LocalEmbedderHeapTracerWithIsolate, DestructorClearsIsolate) {
+  StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  {
+    LocalEmbedderHeapTracer local_tracer(isolate());
+    local_tracer.SetRemoteTracer(&remote_tracer);
+    EXPECT_EQ(isolate(), reinterpret_cast<Isolate*>(remote_tracer.isolate()));
+  }
+  EXPECT_EQ(nullptr, remote_tracer.isolate());
+}
+
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8