From 8aef442917e82491f0fc66426f71f865353d94e2 Mon Sep 17 00:00:00 2001 From: mlippautz Date: Tue, 18 Aug 2015 05:03:45 -0700 Subject: [PATCH] [api,heap] Fix external GC callbacks. * Add types to properly report what has been executed in the GC * Unify GCPrologueCallback and GCEpilogueCallback into GCCallback * Report processing of second round weak handels, either synchronously or asynchronously BUG=chromium:521946 LOG=N Review URL: https://codereview.chromium.org/1298113003 Cr-Commit-Position: refs/heads/master@{#30218} --- include/v8.h | 69 +++++++++++++++++++++++++++++---------------------- src/api.cc | 22 ++++++---------- src/global-handles.cc | 8 ++++++ src/heap/heap.cc | 30 +++++++++++----------- src/heap/heap.h | 47 +++++++++++++---------------------- 5 files changed, 89 insertions(+), 87 deletions(-) diff --git a/include/v8.h b/include/v8.h index 95edfdb..9a57776 100644 --- a/include/v8.h +++ b/include/v8.h @@ -4951,16 +4951,19 @@ typedef bool (*AllowCodeGenerationFromStringsCallback)(Local context); // --- Garbage Collection Callbacks --- /** - * Applications can register callback functions which will be called - * before and after a garbage collection. Allocations are not - * allowed in the callback functions, you therefore cannot manipulate - * objects (set or delete properties for example) since it is possible - * such operations will result in the allocation of objects. + * Applications can register callback functions which will be called before and + * after certain garbage collection operations. Allocations are not allowed in + * the callback functions, you therefore cannot manipulate objects (set or + * delete properties for example) since it is possible such operations will + * result in the allocation of objects. */ enum GCType { kGCTypeScavenge = 1 << 0, kGCTypeMarkSweepCompact = 1 << 1, - kGCTypeAll = kGCTypeScavenge | kGCTypeMarkSweepCompact + kGCTypeIncrementalMarking = 1 << 2, + kGCTypeProcessWeakCallbacks = 1 << 3, + kGCTypeAll = kGCTypeScavenge | kGCTypeMarkSweepCompact | + kGCTypeIncrementalMarking | kGCTypeProcessWeakCallbacks }; enum GCCallbackFlags { @@ -4970,8 +4973,13 @@ enum GCCallbackFlags { kGCCallbackFlagSynchronousPhantomCallbackProcessing = 1 << 3 }; -typedef void (*GCPrologueCallback)(GCType type, GCCallbackFlags flags); -typedef void (*GCEpilogueCallback)(GCType type, GCCallbackFlags flags); +V8_DEPRECATE_SOON("Use GCCallBack instead", + typedef void (*GCPrologueCallback)(GCType type, + GCCallbackFlags flags)); +V8_DEPRECATE_SOON("Use GCCallBack instead", + typedef void (*GCEpilogueCallback)(GCType type, + GCCallbackFlags flags)); +typedef void (*GCCallback)(GCType type, GCCallbackFlags flags); typedef void (*InterruptCallback)(Isolate* isolate, void* data); @@ -5547,12 +5555,16 @@ class V8_EXPORT Isolate { template void SetReference(const Persistent& parent, const Persistent& child); - typedef void (*GCPrologueCallback)(Isolate* isolate, - GCType type, - GCCallbackFlags flags); - typedef void (*GCEpilogueCallback)(Isolate* isolate, - GCType type, - GCCallbackFlags flags); + V8_DEPRECATE_SOON("Use GCCallBack instead", + typedef void (*GCPrologueCallback)(Isolate* isolate, + GCType type, + GCCallbackFlags flags)); + V8_DEPRECATE_SOON("Use GCCallBack instead", + typedef void (*GCEpilogueCallback)(Isolate* isolate, + GCType type, + GCCallbackFlags flags)); + typedef void (*GCCallback)(Isolate* isolate, GCType type, + GCCallbackFlags flags); /** * Enables the host application to receive a notification before a @@ -5563,14 +5575,14 @@ class V8_EXPORT Isolate { * not possible to register the same callback function two times with * different GCType filters. */ - void AddGCPrologueCallback( - GCPrologueCallback callback, GCType gc_type_filter = kGCTypeAll); + void AddGCPrologueCallback(GCCallback callback, + GCType gc_type_filter = kGCTypeAll); /** * This function removes callback which was installed by * AddGCPrologueCallback function. */ - void RemoveGCPrologueCallback(GCPrologueCallback callback); + void RemoveGCPrologueCallback(GCCallback callback); /** * Enables the host application to receive a notification after a @@ -5581,15 +5593,14 @@ class V8_EXPORT Isolate { * not possible to register the same callback function two times with * different GCType filters. */ - void AddGCEpilogueCallback( - GCEpilogueCallback callback, GCType gc_type_filter = kGCTypeAll); + void AddGCEpilogueCallback(GCCallback callback, + GCType gc_type_filter = kGCTypeAll); /** * This function removes callback which was installed by * AddGCEpilogueCallback function. */ - void RemoveGCEpilogueCallback(GCEpilogueCallback callback); - + void RemoveGCEpilogueCallback(GCCallback callback); /** * Forcefully terminate the current thread of JavaScript execution @@ -6046,7 +6057,7 @@ class V8_EXPORT V8 { */ static V8_DEPRECATE_SOON( "Use isolate version", - void AddGCPrologueCallback(GCPrologueCallback callback, + void AddGCPrologueCallback(GCCallback callback, GCType gc_type_filter = kGCTypeAll)); /** @@ -6055,7 +6066,7 @@ class V8_EXPORT V8 { */ V8_INLINE static V8_DEPRECATE_SOON( "Use isolate version", - void RemoveGCPrologueCallback(GCPrologueCallback callback)); + void RemoveGCPrologueCallback(GCCallback callback)); /** * Enables the host application to receive a notification after a @@ -6069,7 +6080,7 @@ class V8_EXPORT V8 { */ static V8_DEPRECATE_SOON( "Use isolate version", - void AddGCEpilogueCallback(GCEpilogueCallback callback, + void AddGCEpilogueCallback(GCCallback callback, GCType gc_type_filter = kGCTypeAll)); /** @@ -6078,7 +6089,7 @@ class V8_EXPORT V8 { */ V8_INLINE static V8_DEPRECATE_SOON( "Use isolate version", - void RemoveGCEpilogueCallback(GCEpilogueCallback callback)); + void RemoveGCEpilogueCallback(GCCallback callback)); /** * Enables the host application to provide a mechanism to be notified @@ -8257,17 +8268,17 @@ void V8::SetFatalErrorHandler(FatalErrorCallback callback) { } -void V8::RemoveGCPrologueCallback(GCPrologueCallback callback) { +void V8::RemoveGCPrologueCallback(GCCallback callback) { Isolate* isolate = Isolate::GetCurrent(); isolate->RemoveGCPrologueCallback( - reinterpret_cast(callback)); + reinterpret_cast(callback)); } -void V8::RemoveGCEpilogueCallback(GCEpilogueCallback callback) { +void V8::RemoveGCEpilogueCallback(GCCallback callback) { Isolate* isolate = Isolate::GetCurrent(); isolate->RemoveGCEpilogueCallback( - reinterpret_cast(callback)); + reinterpret_cast(callback)); } diff --git a/src/api.cc b/src/api.cc index 469d9c6..f9a8739 100644 --- a/src/api.cc +++ b/src/api.cc @@ -6983,47 +6983,41 @@ void Isolate::SetReference(internal::Object** parent, } -void Isolate::AddGCPrologueCallback(GCPrologueCallback callback, - GCType gc_type) { +void Isolate::AddGCPrologueCallback(GCCallback callback, GCType gc_type) { i::Isolate* isolate = reinterpret_cast(this); isolate->heap()->AddGCPrologueCallback(callback, gc_type); } -void Isolate::RemoveGCPrologueCallback(GCPrologueCallback callback) { +void Isolate::RemoveGCPrologueCallback(GCCallback callback) { i::Isolate* isolate = reinterpret_cast(this); isolate->heap()->RemoveGCPrologueCallback(callback); } -void Isolate::AddGCEpilogueCallback(GCEpilogueCallback callback, - GCType gc_type) { +void Isolate::AddGCEpilogueCallback(GCCallback callback, GCType gc_type) { i::Isolate* isolate = reinterpret_cast(this); isolate->heap()->AddGCEpilogueCallback(callback, gc_type); } -void Isolate::RemoveGCEpilogueCallback(GCEpilogueCallback callback) { +void Isolate::RemoveGCEpilogueCallback(GCCallback callback) { i::Isolate* isolate = reinterpret_cast(this); isolate->heap()->RemoveGCEpilogueCallback(callback); } -void V8::AddGCPrologueCallback(GCPrologueCallback callback, GCType gc_type) { +void V8::AddGCPrologueCallback(GCCallback callback, GCType gc_type) { i::Isolate* isolate = i::Isolate::Current(); isolate->heap()->AddGCPrologueCallback( - reinterpret_cast(callback), - gc_type, - false); + reinterpret_cast(callback), gc_type, false); } -void V8::AddGCEpilogueCallback(GCEpilogueCallback callback, GCType gc_type) { +void V8::AddGCEpilogueCallback(GCCallback callback, GCType gc_type) { i::Isolate* isolate = i::Isolate::Current(); isolate->heap()->AddGCEpilogueCallback( - reinterpret_cast(callback), - gc_type, - false); + reinterpret_cast(callback), gc_type, false); } diff --git a/src/global-handles.cc b/src/global-handles.cc index fcf896f..befa173 100644 --- a/src/global-handles.cc +++ b/src/global-handles.cc @@ -507,7 +507,11 @@ class GlobalHandles::PendingPhantomCallbacksSecondPassTask } void RunInternal() override { + isolate_->heap()->CallGCPrologueCallbacks( + GCType::kGCTypeProcessWeakCallbacks, kNoGCCallbackFlags); InvokeSecondPassPhantomCallbacks(&pending_phantom_callbacks_, isolate_); + isolate_->heap()->CallGCEpilogueCallbacks( + GCType::kGCTypeProcessWeakCallbacks, kNoGCCallbackFlags); } private: @@ -841,7 +845,11 @@ int GlobalHandles::DispatchPendingPhantomCallbacks( } if (pending_phantom_callbacks_.length() > 0) { if (FLAG_optimize_for_size || FLAG_predictable || synchronous_second_pass) { + isolate()->heap()->CallGCPrologueCallbacks( + GCType::kGCTypeProcessWeakCallbacks, kNoGCCallbackFlags); InvokeSecondPassPhantomCallbacks(&pending_phantom_callbacks_, isolate()); + isolate()->heap()->CallGCEpilogueCallbacks( + GCType::kGCTypeProcessWeakCallbacks, kNoGCCallbackFlags); } else { auto task = new PendingPhantomCallbacksSecondPassTask( &pending_phantom_callbacks_, isolate()); diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 6f15468..a965592 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -772,6 +772,8 @@ void Heap::OverApproximateWeakClosure(const char* gc_reason) { GCTracer::Scope scope(tracer(), GCTracer::Scope::EXTERNAL); VMState state(isolate_); HandleScope handle_scope(isolate_); + // TODO(mlippautz): Report kGCTypeIncremental once blink updates its + // filtering. CallGCPrologueCallbacks(kGCTypeMarkSweepCompact, kNoGCCallbackFlags); } } @@ -783,6 +785,8 @@ void Heap::OverApproximateWeakClosure(const char* gc_reason) { GCTracer::Scope scope(tracer(), GCTracer::Scope::EXTERNAL); VMState state(isolate_); HandleScope handle_scope(isolate_); + // TODO(mlippautz): Report kGCTypeIncremental once blink updates its + // filtering. CallGCEpilogueCallbacks(kGCTypeMarkSweepCompact, kNoGCCallbackFlags); } } @@ -1288,10 +1292,9 @@ bool Heap::PerformGarbageCollection( void Heap::CallGCPrologueCallbacks(GCType gc_type, GCCallbackFlags flags) { for (int i = 0; i < gc_prologue_callbacks_.length(); ++i) { if (gc_type & gc_prologue_callbacks_[i].gc_type) { - if (!gc_prologue_callbacks_[i].pass_isolate_) { - v8::GCPrologueCallback callback = - reinterpret_cast( - gc_prologue_callbacks_[i].callback); + if (!gc_prologue_callbacks_[i].pass_isolate) { + v8::GCCallback callback = reinterpret_cast( + gc_prologue_callbacks_[i].callback); callback(gc_type, flags); } else { v8::Isolate* isolate = reinterpret_cast(this->isolate()); @@ -1306,10 +1309,9 @@ void Heap::CallGCEpilogueCallbacks(GCType gc_type, GCCallbackFlags gc_callback_flags) { for (int i = 0; i < gc_epilogue_callbacks_.length(); ++i) { if (gc_type & gc_epilogue_callbacks_[i].gc_type) { - if (!gc_epilogue_callbacks_[i].pass_isolate_) { - v8::GCPrologueCallback callback = - reinterpret_cast( - gc_epilogue_callbacks_[i].callback); + if (!gc_epilogue_callbacks_[i].pass_isolate) { + v8::GCCallback callback = reinterpret_cast( + gc_epilogue_callbacks_[i].callback); callback(gc_type, gc_callback_flags); } else { v8::Isolate* isolate = reinterpret_cast(this->isolate()); @@ -5951,16 +5953,16 @@ void Heap::TearDown() { } -void Heap::AddGCPrologueCallback(v8::Isolate::GCPrologueCallback callback, +void Heap::AddGCPrologueCallback(v8::Isolate::GCCallback callback, GCType gc_type, bool pass_isolate) { DCHECK(callback != NULL); - GCPrologueCallbackPair pair(callback, gc_type, pass_isolate); + GCCallbackPair pair(callback, gc_type, pass_isolate); DCHECK(!gc_prologue_callbacks_.Contains(pair)); return gc_prologue_callbacks_.Add(pair); } -void Heap::RemoveGCPrologueCallback(v8::Isolate::GCPrologueCallback callback) { +void Heap::RemoveGCPrologueCallback(v8::Isolate::GCCallback callback) { DCHECK(callback != NULL); for (int i = 0; i < gc_prologue_callbacks_.length(); ++i) { if (gc_prologue_callbacks_[i].callback == callback) { @@ -5972,16 +5974,16 @@ void Heap::RemoveGCPrologueCallback(v8::Isolate::GCPrologueCallback callback) { } -void Heap::AddGCEpilogueCallback(v8::Isolate::GCEpilogueCallback callback, +void Heap::AddGCEpilogueCallback(v8::Isolate::GCCallback callback, GCType gc_type, bool pass_isolate) { DCHECK(callback != NULL); - GCEpilogueCallbackPair pair(callback, gc_type, pass_isolate); + GCCallbackPair pair(callback, gc_type, pass_isolate); DCHECK(!gc_epilogue_callbacks_.Contains(pair)); return gc_epilogue_callbacks_.Add(pair); } -void Heap::RemoveGCEpilogueCallback(v8::Isolate::GCEpilogueCallback callback) { +void Heap::RemoveGCEpilogueCallback(v8::Isolate::GCCallback callback) { DCHECK(callback != NULL); for (int i = 0; i < gc_epilogue_callbacks_.length(); ++i) { if (gc_epilogue_callbacks_[i].callback == callback) { diff --git a/src/heap/heap.h b/src/heap/heap.h index 43b4d25..b6c9ddf 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -882,13 +882,13 @@ class Heap { PromotionQueue* promotion_queue() { return &promotion_queue_; } - void AddGCPrologueCallback(v8::Isolate::GCPrologueCallback callback, + void AddGCPrologueCallback(v8::Isolate::GCCallback callback, GCType gc_type_filter, bool pass_isolate = true); - void RemoveGCPrologueCallback(v8::Isolate::GCPrologueCallback callback); + void RemoveGCPrologueCallback(v8::Isolate::GCCallback callback); - void AddGCEpilogueCallback(v8::Isolate::GCEpilogueCallback callback, + void AddGCEpilogueCallback(v8::Isolate::GCCallback callback, GCType gc_type_filter, bool pass_isolate = true); - void RemoveGCEpilogueCallback(v8::Isolate::GCEpilogueCallback callback); + void RemoveGCEpilogueCallback(v8::Isolate::GCCallback callback); // Heap root getters. We have versions with and without type::cast() here. // You can't use type::cast during GC because the assert fails. @@ -1857,35 +1857,22 @@ class Heap { void AddPrivateGlobalSymbols(Handle private_intern_table); - // GC callback function, called before and after mark-compact GC. - // Allocations in the callback function are disallowed. - struct GCPrologueCallbackPair { - GCPrologueCallbackPair(v8::Isolate::GCPrologueCallback callback, - GCType gc_type, bool pass_isolate) - : callback(callback), gc_type(gc_type), pass_isolate_(pass_isolate) {} - bool operator==(const GCPrologueCallbackPair& pair) const { - return pair.callback == callback; - } - v8::Isolate::GCPrologueCallback callback; - GCType gc_type; - // TODO(dcarney): remove variable - bool pass_isolate_; - }; - List gc_prologue_callbacks_; - - struct GCEpilogueCallbackPair { - GCEpilogueCallbackPair(v8::Isolate::GCPrologueCallback callback, - GCType gc_type, bool pass_isolate) - : callback(callback), gc_type(gc_type), pass_isolate_(pass_isolate) {} - bool operator==(const GCEpilogueCallbackPair& pair) const { - return pair.callback == callback; + struct GCCallbackPair { + GCCallbackPair(v8::Isolate::GCCallback callback, GCType gc_type, + bool pass_isolate) + : callback(callback), gc_type(gc_type), pass_isolate(pass_isolate) {} + + bool operator==(const GCCallbackPair& other) const { + return other.callback == callback; } - v8::Isolate::GCPrologueCallback callback; + + v8::Isolate::GCCallback callback; GCType gc_type; - // TODO(dcarney): remove variable - bool pass_isolate_; + bool pass_isolate; }; - List gc_epilogue_callbacks_; + + List gc_epilogue_callbacks_; + List gc_prologue_callbacks_; // Code that should be run before and after each GC. Includes some // reporting/verification activities when compiled with DEBUG set. -- 2.7.4