From 0c5a0ecc7c8b203aa34a43f0fb8f3bf81cce7f41 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 13 Mar 2014 20:47:02 +0400 Subject: [PATCH] deps: allow allocations in gc epilogue/prologue See https://codereview.chromium.org/177243012/ --- deps/v8/include/v8.h | 26 ++++++++-------- deps/v8/src/heap-inl.h | 15 ++++++++++ deps/v8/src/heap.cc | 29 +++++++++++------- deps/v8/src/heap.h | 15 ++++++++++ deps/v8/test/cctest/test-api.cc | 53 +++++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 25 deletions(-) diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index dd8f2685bc..f3a21b60f6 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -4127,13 +4127,12 @@ class V8_EXPORT Isolate { /** * Enables the host application to receive a notification before a - * garbage collection. Allocations are not allowed in the - * callback function, you therefore cannot manipulate objects (set - * or delete properties for example) since it is possible such - * operations will result in the allocation of objects. It is possible - * to specify the GCType filter for your callback. But it is not possible to - * register the same callback function two times with different - * GCType filters. + * garbage collection. Allocations are allowed in the callback function, + * but the callback is not re-entrant: if the allocation inside it will + * trigger the Garbage Collection, the callback won't be called again. + * It is possible to specify the GCType filter for your callback. But it is + * not possible to register the same callback function two times with + * different GCType filters. */ void AddGCPrologueCallback( GCPrologueCallback callback, GCType gc_type_filter = kGCTypeAll); @@ -4146,13 +4145,12 @@ class V8_EXPORT Isolate { /** * Enables the host application to receive a notification after a - * garbage collection. Allocations are not allowed in the - * callback function, you therefore cannot manipulate objects (set - * or delete properties for example) since it is possible such - * operations will result in the allocation of objects. It is possible - * to specify the GCType filter for your callback. But it is not possible to - * register the same callback function two times with different - * GCType filters. + * garbage collection. Allocations are allowed in the callback function, + * but the callback is not re-entrant: if the allocation inside it will + * trigger the Garbage Collection, the callback won't be called again. + * It is possible to specify the GCType filter for your callback. But it is + * not possible to register the same callback function two times with + * different GCType filters. */ void AddGCEpilogueCallback( GCEpilogueCallback callback, GCType gc_type_filter = kGCTypeAll); diff --git a/deps/v8/src/heap-inl.h b/deps/v8/src/heap-inl.h index a45e3ab9d9..f74c4c7a7f 100644 --- a/deps/v8/src/heap-inl.h +++ b/deps/v8/src/heap-inl.h @@ -809,6 +809,21 @@ NoWeakObjectVerificationScope::~NoWeakObjectVerificationScope() { #endif +GCCallbacksScope::GCCallbacksScope(Heap* heap) : heap_(heap) { + heap_->gc_callbacks_depth_++; +} + + +GCCallbacksScope::~GCCallbacksScope() { + heap_->gc_callbacks_depth_--; +} + + +bool GCCallbacksScope::CheckReenter() { + return heap_->gc_callbacks_depth_ == 1; +} + + void VerifyPointersVisitor::VisitPointers(Object** start, Object** end) { for (Object** current = start; current < end; current++) { if ((*current)->IsHeapObject()) { diff --git a/deps/v8/src/heap.cc b/deps/v8/src/heap.cc index 42e56ca1eb..8454dd51cb 100644 --- a/deps/v8/src/heap.cc +++ b/deps/v8/src/heap.cc @@ -155,7 +155,8 @@ Heap::Heap() configured_(false), external_string_table_(this), chunks_queued_for_free_(NULL), - relocation_mutex_(NULL) { + relocation_mutex_(NULL), + gc_callbacks_depth_(0) { // Allow build-time customization of the max semispace size. Building // V8 with snapshots and a non-default max semispace size is much // easier if you can define it as part of the build environment. @@ -1084,11 +1085,14 @@ bool Heap::PerformGarbageCollection( GCType gc_type = collector == MARK_COMPACTOR ? kGCTypeMarkSweepCompact : kGCTypeScavenge; - { - GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL); - VMState state(isolate_); - HandleScope handle_scope(isolate_); - CallGCPrologueCallbacks(gc_type, kNoGCCallbackFlags); + { GCCallbacksScope scope(this); + if (scope.CheckReenter()) { + AllowHeapAllocation allow_allocation; + GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL); + VMState state(isolate_); + HandleScope handle_scope(isolate_); + CallGCPrologueCallbacks(gc_type, kNoGCCallbackFlags); + } } EnsureFromSpaceIsCommitted(); @@ -1193,11 +1197,14 @@ bool Heap::PerformGarbageCollection( amount_of_external_allocated_memory_; } - { - GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL); - VMState state(isolate_); - HandleScope handle_scope(isolate_); - CallGCEpilogueCallbacks(gc_type, gc_callback_flags); + { GCCallbacksScope scope(this); + if (scope.CheckReenter()) { + AllowHeapAllocation allow_allocation; + GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL); + VMState state(isolate_); + HandleScope handle_scope(isolate_); + CallGCEpilogueCallbacks(gc_type, gc_callback_flags); + } } #ifdef VERIFY_HEAP diff --git a/deps/v8/src/heap.h b/deps/v8/src/heap.h index 1ac4dfaa08..81c7d4732c 100644 --- a/deps/v8/src/heap.h +++ b/deps/v8/src/heap.h @@ -2510,6 +2510,8 @@ class Heap { bool relocation_mutex_locked_by_optimizer_thread_; #endif // DEBUG; + int gc_callbacks_depth_; + friend class Factory; friend class GCTracer; friend class DisallowAllocationFailure; @@ -2522,6 +2524,7 @@ class Heap { #ifdef VERIFY_HEAP friend class NoWeakObjectVerificationScope; #endif + friend class GCCallbacksScope; DISALLOW_COPY_AND_ASSIGN(Heap); }; @@ -2594,6 +2597,18 @@ class NoWeakObjectVerificationScope { #endif +class GCCallbacksScope { + public: + explicit inline GCCallbacksScope(Heap* heap); + inline ~GCCallbacksScope(); + + inline bool CheckReenter(); + + private: + Heap* heap_; +}; + + // Visitor class to verify interior pointers in spaces that do not contain // or care about intergenerational references. All heap object pointers have to // point into the heap to a location that has a map pointer at its first word. diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 9312057fa2..d2dbe63f56 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -18563,6 +18563,8 @@ int prologue_call_count = 0; int epilogue_call_count = 0; int prologue_call_count_second = 0; int epilogue_call_count_second = 0; +int prologue_call_count_alloc = 0; +int epilogue_call_count_alloc = 0; void PrologueCallback(v8::GCType, v8::GCCallbackFlags flags) { CHECK_EQ(flags, v8::kNoGCCallbackFlags); @@ -18624,6 +18626,46 @@ void EpilogueCallbackSecond(v8::Isolate* isolate, } +void PrologueCallbackAlloc(v8::Isolate* isolate, + v8::GCType, + v8::GCCallbackFlags flags) { + v8::HandleScope scope(isolate); + + CHECK_EQ(flags, v8::kNoGCCallbackFlags); + CHECK_EQ(gc_callbacks_isolate, isolate); + ++prologue_call_count_alloc; + + // Simulate full heap to see if we will reenter this callback + SimulateFullSpace(CcTest::heap()->new_space()); + + Local obj = Object::New(isolate); + ASSERT(!obj.IsEmpty()); + + CcTest::heap()->CollectAllGarbage( + i::Heap::Heap::kAbortIncrementalMarkingMask); +} + + +void EpilogueCallbackAlloc(v8::Isolate* isolate, + v8::GCType, + v8::GCCallbackFlags flags) { + v8::HandleScope scope(isolate); + + CHECK_EQ(flags, v8::kNoGCCallbackFlags); + CHECK_EQ(gc_callbacks_isolate, isolate); + ++epilogue_call_count_alloc; + + // Simulate full heap to see if we will reenter this callback + SimulateFullSpace(CcTest::heap()->new_space()); + + Local obj = Object::New(isolate); + ASSERT(!obj.IsEmpty()); + + CcTest::heap()->CollectAllGarbage( + i::Heap::Heap::kAbortIncrementalMarkingMask); +} + + TEST(GCCallbacksOld) { LocalContext context; @@ -18690,6 +18732,17 @@ TEST(GCCallbacks) { CHECK_EQ(2, epilogue_call_count); CHECK_EQ(2, prologue_call_count_second); CHECK_EQ(2, epilogue_call_count_second); + + CHECK_EQ(0, prologue_call_count_alloc); + CHECK_EQ(0, epilogue_call_count_alloc); + isolate->AddGCPrologueCallback(PrologueCallbackAlloc); + isolate->AddGCEpilogueCallback(EpilogueCallbackAlloc); + CcTest::heap()->CollectAllGarbage( + i::Heap::Heap::kAbortIncrementalMarkingMask); + CHECK_EQ(1, prologue_call_count_alloc); + CHECK_EQ(1, epilogue_call_count_alloc); + isolate->RemoveGCPrologueCallback(PrologueCallbackAlloc); + isolate->RemoveGCEpilogueCallback(EpilogueCallbackAlloc); }