From fe0c91cb6ccd8df3edd0310ef8b1a584da2ad442 Mon Sep 17 00:00:00 2001 From: Michael Lippautz Date: Wed, 29 Apr 2020 17:14:31 +0200 Subject: [PATCH] heap: Rework forced GCs Forced GCs can either be invoked internally or communicate the fact that they are forced externally via API. Before this CL, all uses were passing kGCCallbackFlagForced to indicate that the GC was forced. This flag is used by embedders though to trigger followup actions. E.g., it can be used to trigger a follow up call to GarbageCollectionForTesting() call which requires --expose-gc. This patch changes the semantics as follows: - Internal forced GCs use a Heap GC flag (kForcedGC) - External forced GCs and GC extension use kGCCallbackFlagForced Bug: chromium:1074061 Change-Id: Ide7ea0ccdf88b8c8cac002289aef5b7eb0f9748c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2172747 Reviewed-by: Maya Lekova Reviewed-by: Andreas Haas Reviewed-by: Ulan Degenbaev Commit-Queue: Michael Lippautz Cr-Commit-Position: refs/heads/master@{#67498} --- src/compiler/compilation-dependencies.cc | 3 +-- src/heap/heap.cc | 14 +++++++------- src/heap/heap.h | 3 +++ src/wasm/c-api.cc | 4 ++-- test/cctest/test-api.cc | 4 ++-- test/wasm-api-tests/callbacks.cc | 4 ++-- 6 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/compiler/compilation-dependencies.cc b/src/compiler/compilation-dependencies.cc index d9dbdd6c5b..b9ed54256a 100644 --- a/src/compiler/compilation-dependencies.cc +++ b/src/compiler/compilation-dependencies.cc @@ -534,8 +534,7 @@ bool CompilationDependencies::Commit(Handle code) { // that triggers its deoptimization. if (FLAG_stress_gc_during_compilation) { broker_->isolate()->heap()->PreciseCollectAllGarbage( - Heap::kNoGCFlags, GarbageCollectionReason::kTesting, - kGCCallbackFlagForced); + Heap::kForcedGC, GarbageCollectionReason::kTesting, kNoGCCallbackFlags); } #ifdef DEBUG for (auto dep : dependencies_) { diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 404f824c79..72a8096366 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -1380,16 +1380,15 @@ void Heap::CollectAllAvailableGarbage(GarbageCollectionReason gc_reason) { // The optimizing compiler may be unnecessarily holding on to memory. isolate()->AbortConcurrentOptimization(BlockingBehavior::kDontBlock); isolate()->ClearSerializerData(); - set_current_gc_flags(kReduceMemoryFootprintMask); + set_current_gc_flags( + kReduceMemoryFootprintMask | + (gc_reason == GarbageCollectionReason::kLowMemoryNotification ? kForcedGC + : 0)); isolate_->compilation_cache()->Clear(); const int kMaxNumberOfAttempts = 7; const int kMinNumberOfAttempts = 2; - const v8::GCCallbackFlags callback_flags = - gc_reason == GarbageCollectionReason::kLowMemoryNotification - ? v8::kGCCallbackFlagForced - : v8::kGCCallbackFlagCollectAllAvailableGarbage; for (int attempt = 0; attempt < kMaxNumberOfAttempts; attempt++) { - if (!CollectGarbage(OLD_SPACE, gc_reason, callback_flags) && + if (!CollectGarbage(OLD_SPACE, gc_reason, kNoGCCallbackFlags) && attempt + 1 >= kMinNumberOfAttempts) { break; } @@ -1512,7 +1511,8 @@ bool Heap::CollectGarbage(AllocationSpace space, const v8::GCCallbackFlags gc_callback_flags) { const char* collector_reason = nullptr; GarbageCollector collector = SelectGarbageCollector(space, &collector_reason); - is_current_gc_forced_ = gc_callback_flags & v8::kGCCallbackFlagForced; + is_current_gc_forced_ = gc_callback_flags & v8::kGCCallbackFlagForced || + current_gc_flags_ & kForcedGC; DevToolsTraceEventScope devtools_trace_event_scope( this, IsYoungGenerationCollector(collector) ? "MinorGC" : "MajorGC", diff --git a/src/heap/heap.h b/src/heap/heap.h index 8386fcc69b..71bb1c89dc 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -300,6 +300,9 @@ class Heap { static const int kNoGCFlags = 0; static const int kReduceMemoryFootprintMask = 1; + // GCs that are forced, either through testing configurations (requring + // --expose-gc) or through DevTools (using LowMemoryNotificaton). + static const int kForcedGC = 2; // The minimum size of a HeapObject on the heap. static const int kMinObjectSizeInTaggedWords = 2; diff --git a/src/wasm/c-api.cc b/src/wasm/c-api.cc index d098a5f57f..cd5d04bd2d 100644 --- a/src/wasm/c-api.cc +++ b/src/wasm/c-api.cc @@ -265,8 +265,8 @@ auto Engine::make(own&& config) -> own { StoreImpl::~StoreImpl() { #ifdef DEBUG reinterpret_cast(isolate_)->heap()->PreciseCollectAllGarbage( - i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting, - v8::kGCCallbackFlagForced); + i::Heap::kForcedGC, i::GarbageCollectionReason::kTesting, + v8::kNoGCCallbackFlags); #endif context()->Exit(); isolate_->Dispose(); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index faa963cb8b..ab78d16ebc 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -26850,8 +26850,8 @@ static void CallIsolate2(const v8::FunctionCallbackInfo& args) { v8::Local::New(isolate_2, context_2); v8::Context::Scope context_scope(context); reinterpret_cast(isolate_2)->heap()->CollectAllGarbage( - i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting, - v8::kGCCallbackFlagForced); + i::Heap::kForcedGC, i::GarbageCollectionReason::kTesting, + v8::kNoGCCallbackFlags); CompileRun("f2() //# sourceURL=isolate2b"); } diff --git a/test/wasm-api-tests/callbacks.cc b/test/wasm-api-tests/callbacks.cc index 350a425d47..8d53b767bd 100644 --- a/test/wasm-api-tests/callbacks.cc +++ b/test/wasm-api-tests/callbacks.cc @@ -31,8 +31,8 @@ own Stage4_GC(void* env, const Val args[], Val results[]) { printf("Stage4...\n"); i::Isolate* isolate = reinterpret_cast(env); isolate->heap()->PreciseCollectAllGarbage( - i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting, - v8::kGCCallbackFlagForced); + i::Heap::kForcedGC, i::GarbageCollectionReason::kTesting, + v8::kNoGCCallbackFlags); results[0] = Val::i32(args[0].i32() + 1); return nullptr; }