From 93886e08372075c4eca55fe38375781de98b757e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 8 Sep 2017 15:34:45 +0200 Subject: [PATCH 1/3] deps: cherry-pick b6158eb6befae from V8 upstream Original commit message: [heap] Move gc callbacks from List to std::vector Bug: v8:6333 Change-Id: I4434c6cc59f886f1e37dfd315a3ad5fee28d3f63 Reviewed-on: https://chromium-review.googlesource.com/634907 Reviewed-by: Ulan Degenbaev Commit-Queue: Michael Lippautz Cr-Commit-Position: refs/heads/master@{#47601} --- deps/v8/src/heap/heap.cc | 68 +++++++++++++++++++++++++--------------- deps/v8/src/heap/heap.h | 9 +++--- 2 files changed, 46 insertions(+), 31 deletions(-) diff --git a/deps/v8/src/heap/heap.cc b/deps/v8/src/heap/heap.cc index 6cc718840aa833..1c2092492fc4df 100644 --- a/deps/v8/src/heap/heap.cc +++ b/deps/v8/src/heap/heap.cc @@ -57,6 +57,18 @@ namespace v8 { namespace internal { +bool Heap::GCCallbackPair::operator==(const Heap::GCCallbackPair& other) const { + return other.callback == callback; +} + +Heap::GCCallbackPair& Heap::GCCallbackPair::operator=( + const Heap::GCCallbackPair& other) { + callback = other.callback; + gc_type = other.gc_type; + pass_isolate = other.pass_isolate; + return *this; +} + struct Heap::StrongRootsList { Object** start; Object** end; @@ -1501,15 +1513,15 @@ bool Heap::PerformGarbageCollection( void Heap::CallGCPrologueCallbacks(GCType gc_type, GCCallbackFlags flags) { RuntimeCallTimerScope runtime_timer(isolate(), &RuntimeCallStats::GCPrologueCallback); - 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::GCCallback callback = reinterpret_cast( - gc_prologue_callbacks_[i].callback); + for (const GCCallbackPair& info : gc_prologue_callbacks_) { + if (gc_type & info.gc_type) { + if (!info.pass_isolate) { + v8::GCCallback callback = + reinterpret_cast(info.callback); callback(gc_type, flags); } else { v8::Isolate* isolate = reinterpret_cast(this->isolate()); - gc_prologue_callbacks_[i].callback(isolate, gc_type, flags); + info.callback(isolate, gc_type, flags); } } } @@ -1520,15 +1532,15 @@ void Heap::CallGCEpilogueCallbacks(GCType gc_type, GCCallbackFlags gc_callback_flags) { RuntimeCallTimerScope runtime_timer(isolate(), &RuntimeCallStats::GCEpilogueCallback); - 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::GCCallback callback = reinterpret_cast( - gc_epilogue_callbacks_[i].callback); + for (const GCCallbackPair& info : gc_epilogue_callbacks_) { + if (gc_type & info.gc_type) { + if (!info.pass_isolate) { + v8::GCCallback callback = + reinterpret_cast(info.callback); callback(gc_type, gc_callback_flags); } else { v8::Isolate* isolate = reinterpret_cast(this->isolate()); - gc_epilogue_callbacks_[i].callback(isolate, gc_type, gc_callback_flags); + info.callback(isolate, gc_type, gc_callback_flags); } } } @@ -5964,18 +5976,20 @@ void Heap::TearDown() { void Heap::AddGCPrologueCallback(v8::Isolate::GCCallback callback, GCType gc_type, bool pass_isolate) { - DCHECK(callback != NULL); - GCCallbackPair pair(callback, gc_type, pass_isolate); - DCHECK(!gc_prologue_callbacks_.Contains(pair)); - return gc_prologue_callbacks_.Add(pair); + DCHECK_NOT_NULL(callback); + DCHECK(gc_prologue_callbacks_.end() == + std::find(gc_prologue_callbacks_.begin(), gc_prologue_callbacks_.end(), + GCCallbackPair(callback, gc_type, pass_isolate))); + gc_prologue_callbacks_.emplace_back(callback, gc_type, pass_isolate); } void Heap::RemoveGCPrologueCallback(v8::Isolate::GCCallback callback) { - DCHECK(callback != NULL); - for (int i = 0; i < gc_prologue_callbacks_.length(); ++i) { + DCHECK_NOT_NULL(callback); + for (size_t i = 0; i < gc_prologue_callbacks_.size(); i++) { if (gc_prologue_callbacks_[i].callback == callback) { - gc_prologue_callbacks_.Remove(i); + gc_prologue_callbacks_[i] = gc_prologue_callbacks_.back(); + gc_prologue_callbacks_.pop_back(); return; } } @@ -5985,18 +5999,20 @@ void Heap::RemoveGCPrologueCallback(v8::Isolate::GCCallback callback) { void Heap::AddGCEpilogueCallback(v8::Isolate::GCCallback callback, GCType gc_type, bool pass_isolate) { - DCHECK(callback != NULL); - GCCallbackPair pair(callback, gc_type, pass_isolate); - DCHECK(!gc_epilogue_callbacks_.Contains(pair)); - return gc_epilogue_callbacks_.Add(pair); + DCHECK_NOT_NULL(callback); + DCHECK(gc_epilogue_callbacks_.end() == + std::find(gc_epilogue_callbacks_.begin(), gc_epilogue_callbacks_.end(), + GCCallbackPair(callback, gc_type, pass_isolate))); + gc_epilogue_callbacks_.emplace_back(callback, gc_type, pass_isolate); } void Heap::RemoveGCEpilogueCallback(v8::Isolate::GCCallback callback) { - DCHECK(callback != NULL); - for (int i = 0; i < gc_epilogue_callbacks_.length(); ++i) { + DCHECK_NOT_NULL(callback); + for (size_t i = 0; i < gc_epilogue_callbacks_.size(); i++) { if (gc_epilogue_callbacks_[i].callback == callback) { - gc_epilogue_callbacks_.Remove(i); + gc_epilogue_callbacks_[i] = gc_epilogue_callbacks_.back(); + gc_epilogue_callbacks_.pop_back(); return; } } diff --git a/deps/v8/src/heap/heap.h b/deps/v8/src/heap/heap.h index e90838e29538bb..22b92351474dc5 100644 --- a/deps/v8/src/heap/heap.h +++ b/deps/v8/src/heap/heap.h @@ -1593,9 +1593,8 @@ class Heap { bool pass_isolate) : callback(callback), gc_type(gc_type), pass_isolate(pass_isolate) {} - bool operator==(const GCCallbackPair& other) const { - return other.callback == callback; - } + bool operator==(const GCCallbackPair& other) const; + GCCallbackPair& operator=(const GCCallbackPair& other); v8::Isolate::GCCallback callback; GCType gc_type; @@ -2263,8 +2262,8 @@ class Heap { Object* encountered_transition_arrays_; - List gc_epilogue_callbacks_; - List gc_prologue_callbacks_; + std::vector gc_epilogue_callbacks_; + std::vector gc_prologue_callbacks_; GetExternallyAllocatedMemoryInBytesCallback external_memory_callback_; From c82d22e024611176356561785e1ede0cf325d86f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 8 Sep 2017 15:36:50 +0200 Subject: [PATCH 2/3] deps: cherry-pick 9b21865822243 from V8 upstream Original commit message: [api] Add optional data pointer to GC callbacks This can be useful when there may be multiple callbacks attached by code that's not directly tied to a single isolate, e.g. working on a per-context basis. This also allows rephrasing the global non-isolate APIs in terms of this new API, rather than working around it inside `src/heap`. TBR=hpayer@chromium.org Bug: Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I2e490ec40d1a34ea812f25f41ef9741d2116d965 Reviewed-on: https://chromium-review.googlesource.com/647548 Reviewed-by: Yang Guo Reviewed-by: Adam Klein Commit-Queue: Yang Guo Cr-Commit-Position: refs/heads/master@{#47923} --- deps/v8/include/v8.h | 9 +++++ deps/v8/src/api.cc | 65 +++++++++++++++++++++--------- deps/v8/src/heap/heap.cc | 71 ++++++++++++++------------------- deps/v8/src/heap/heap.h | 34 ++++++++-------- deps/v8/test/cctest/test-api.cc | 59 +++++++++++++++++++++++++++ 5 files changed, 162 insertions(+), 76 deletions(-) diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index e37c549cb46b81..eaac6db0a17dad 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -7203,6 +7203,8 @@ class V8_EXPORT Isolate { typedef void (*GCCallback)(Isolate* isolate, GCType type, GCCallbackFlags flags); + typedef void (*GCCallbackWithData)(Isolate* isolate, GCType type, + GCCallbackFlags flags, void* data); /** * Enables the host application to receive a notification before a @@ -7213,6 +7215,8 @@ class V8_EXPORT Isolate { * not possible to register the same callback function two times with * different GCType filters. */ + void AddGCPrologueCallback(GCCallbackWithData callback, void* data = nullptr, + GCType gc_type_filter = kGCTypeAll); void AddGCPrologueCallback(GCCallback callback, GCType gc_type_filter = kGCTypeAll); @@ -7220,6 +7224,7 @@ class V8_EXPORT Isolate { * This function removes callback which was installed by * AddGCPrologueCallback function. */ + void RemoveGCPrologueCallback(GCCallbackWithData, void* data = nullptr); void RemoveGCPrologueCallback(GCCallback callback); /** @@ -7236,6 +7241,8 @@ class V8_EXPORT Isolate { * not possible to register the same callback function two times with * different GCType filters. */ + void AddGCEpilogueCallback(GCCallbackWithData callback, void* data = nullptr, + GCType gc_type_filter = kGCTypeAll); void AddGCEpilogueCallback(GCCallback callback, GCType gc_type_filter = kGCTypeAll); @@ -7243,6 +7250,8 @@ class V8_EXPORT Isolate { * This function removes callback which was installed by * AddGCEpilogueCallback function. */ + void RemoveGCEpilogueCallback(GCCallbackWithData callback, + void* data = nullptr); void RemoveGCEpilogueCallback(GCCallback callback); typedef size_t (*GetExternallyAllocatedMemoryInBytesCallback)(); diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc index 09da3ac8835266..671960a0b72924 100644 --- a/deps/v8/src/api.cc +++ b/deps/v8/src/api.cc @@ -8341,41 +8341,70 @@ v8::Local Isolate::ThrowException(v8::Local value) { return v8::Undefined(reinterpret_cast(isolate)); } -void Isolate::AddGCPrologueCallback(GCCallback callback, GCType gc_type) { +void Isolate::AddGCPrologueCallback(GCCallbackWithData callback, void* data, + GCType gc_type) { i::Isolate* isolate = reinterpret_cast(this); - isolate->heap()->AddGCPrologueCallback(callback, gc_type); + isolate->heap()->AddGCPrologueCallback(callback, gc_type, data); } - -void Isolate::RemoveGCPrologueCallback(GCCallback callback) { +void Isolate::RemoveGCPrologueCallback(GCCallbackWithData callback, + void* data) { i::Isolate* isolate = reinterpret_cast(this); - isolate->heap()->RemoveGCPrologueCallback(callback); + isolate->heap()->RemoveGCPrologueCallback(callback, data); } +void Isolate::AddGCEpilogueCallback(GCCallbackWithData callback, void* data, + GCType gc_type) { + i::Isolate* isolate = reinterpret_cast(this); + isolate->heap()->AddGCEpilogueCallback(callback, gc_type, data); +} -void Isolate::AddGCEpilogueCallback(GCCallback callback, GCType gc_type) { +void Isolate::RemoveGCEpilogueCallback(GCCallbackWithData callback, + void* data) { i::Isolate* isolate = reinterpret_cast(this); - isolate->heap()->AddGCEpilogueCallback(callback, gc_type); + isolate->heap()->RemoveGCEpilogueCallback(callback, data); } +static void CallGCCallbackWithoutData(Isolate* isolate, GCType type, + GCCallbackFlags flags, void* data) { + reinterpret_cast(data)(isolate, type, flags); +} -void Isolate::RemoveGCEpilogueCallback(GCCallback callback) { - i::Isolate* isolate = reinterpret_cast(this); - isolate->heap()->RemoveGCEpilogueCallback(callback); +void Isolate::AddGCPrologueCallback(GCCallback callback, GCType gc_type) { + void* data = reinterpret_cast(callback); + AddGCPrologueCallback(CallGCCallbackWithoutData, data, gc_type); } +void Isolate::RemoveGCPrologueCallback(GCCallback callback) { + void* data = reinterpret_cast(callback); + RemoveGCPrologueCallback(CallGCCallbackWithoutData, data); +} -void V8::AddGCPrologueCallback(GCCallback callback, GCType gc_type) { - i::Isolate* isolate = i::Isolate::Current(); - isolate->heap()->AddGCPrologueCallback( - reinterpret_cast(callback), gc_type, false); +void Isolate::AddGCEpilogueCallback(GCCallback callback, GCType gc_type) { + void* data = reinterpret_cast(callback); + AddGCEpilogueCallback(CallGCCallbackWithoutData, data, gc_type); +} + +void Isolate::RemoveGCEpilogueCallback(GCCallback callback) { + void* data = reinterpret_cast(callback); + RemoveGCEpilogueCallback(CallGCCallbackWithoutData, data); } +static void CallGCCallbackWithoutIsolate(Isolate* isolate, GCType type, + GCCallbackFlags flags, void* data) { + reinterpret_cast(data)(type, flags); +} -void V8::AddGCEpilogueCallback(GCCallback callback, GCType gc_type) { - i::Isolate* isolate = i::Isolate::Current(); - isolate->heap()->AddGCEpilogueCallback( - reinterpret_cast(callback), gc_type, false); +void V8::AddGCPrologueCallback(v8::GCCallback callback, GCType gc_type) { + void* data = reinterpret_cast(callback); + Isolate::GetCurrent()->AddGCPrologueCallback(CallGCCallbackWithoutIsolate, + data, gc_type); +} + +void V8::AddGCEpilogueCallback(v8::GCCallback callback, GCType gc_type) { + void* data = reinterpret_cast(callback); + Isolate::GetCurrent()->AddGCEpilogueCallback(CallGCCallbackWithoutIsolate, + data, gc_type); } void Isolate::SetEmbedderHeapTracer(EmbedderHeapTracer* tracer) { diff --git a/deps/v8/src/heap/heap.cc b/deps/v8/src/heap/heap.cc index 1c2092492fc4df..687f17f901ac31 100644 --- a/deps/v8/src/heap/heap.cc +++ b/deps/v8/src/heap/heap.cc @@ -57,15 +57,16 @@ namespace v8 { namespace internal { -bool Heap::GCCallbackPair::operator==(const Heap::GCCallbackPair& other) const { - return other.callback == callback; +bool Heap::GCCallbackTuple::operator==( + const Heap::GCCallbackTuple& other) const { + return other.callback == callback && other.data == data; } -Heap::GCCallbackPair& Heap::GCCallbackPair::operator=( - const Heap::GCCallbackPair& other) { +Heap::GCCallbackTuple& Heap::GCCallbackTuple::operator=( + const Heap::GCCallbackTuple& other) { callback = other.callback; gc_type = other.gc_type; - pass_isolate = other.pass_isolate; + data = other.data; return *this; } @@ -1513,35 +1514,21 @@ bool Heap::PerformGarbageCollection( void Heap::CallGCPrologueCallbacks(GCType gc_type, GCCallbackFlags flags) { RuntimeCallTimerScope runtime_timer(isolate(), &RuntimeCallStats::GCPrologueCallback); - for (const GCCallbackPair& info : gc_prologue_callbacks_) { + for (const GCCallbackTuple& info : gc_prologue_callbacks_) { if (gc_type & info.gc_type) { - if (!info.pass_isolate) { - v8::GCCallback callback = - reinterpret_cast(info.callback); - callback(gc_type, flags); - } else { - v8::Isolate* isolate = reinterpret_cast(this->isolate()); - info.callback(isolate, gc_type, flags); - } + v8::Isolate* isolate = reinterpret_cast(this->isolate()); + info.callback(isolate, gc_type, flags, info.data); } } } - -void Heap::CallGCEpilogueCallbacks(GCType gc_type, - GCCallbackFlags gc_callback_flags) { +void Heap::CallGCEpilogueCallbacks(GCType gc_type, GCCallbackFlags flags) { RuntimeCallTimerScope runtime_timer(isolate(), &RuntimeCallStats::GCEpilogueCallback); - for (const GCCallbackPair& info : gc_epilogue_callbacks_) { + for (const GCCallbackTuple& info : gc_epilogue_callbacks_) { if (gc_type & info.gc_type) { - if (!info.pass_isolate) { - v8::GCCallback callback = - reinterpret_cast(info.callback); - callback(gc_type, gc_callback_flags); - } else { - v8::Isolate* isolate = reinterpret_cast(this->isolate()); - info.callback(isolate, gc_type, gc_callback_flags); - } + v8::Isolate* isolate = reinterpret_cast(this->isolate()); + info.callback(isolate, gc_type, flags, info.data); } } } @@ -5973,21 +5960,21 @@ void Heap::TearDown() { memory_allocator_ = nullptr; } - -void Heap::AddGCPrologueCallback(v8::Isolate::GCCallback callback, - GCType gc_type, bool pass_isolate) { +void Heap::AddGCPrologueCallback(v8::Isolate::GCCallbackWithData callback, + GCType gc_type, void* data) { DCHECK_NOT_NULL(callback); DCHECK(gc_prologue_callbacks_.end() == std::find(gc_prologue_callbacks_.begin(), gc_prologue_callbacks_.end(), - GCCallbackPair(callback, gc_type, pass_isolate))); - gc_prologue_callbacks_.emplace_back(callback, gc_type, pass_isolate); + GCCallbackTuple(callback, gc_type, data))); + gc_prologue_callbacks_.emplace_back(callback, gc_type, data); } - -void Heap::RemoveGCPrologueCallback(v8::Isolate::GCCallback callback) { +void Heap::RemoveGCPrologueCallback(v8::Isolate::GCCallbackWithData callback, + void* data) { DCHECK_NOT_NULL(callback); for (size_t i = 0; i < gc_prologue_callbacks_.size(); i++) { - if (gc_prologue_callbacks_[i].callback == callback) { + if (gc_prologue_callbacks_[i].callback == callback && + gc_prologue_callbacks_[i].data == data) { gc_prologue_callbacks_[i] = gc_prologue_callbacks_.back(); gc_prologue_callbacks_.pop_back(); return; @@ -5996,21 +5983,21 @@ void Heap::RemoveGCPrologueCallback(v8::Isolate::GCCallback callback) { UNREACHABLE(); } - -void Heap::AddGCEpilogueCallback(v8::Isolate::GCCallback callback, - GCType gc_type, bool pass_isolate) { +void Heap::AddGCEpilogueCallback(v8::Isolate::GCCallbackWithData callback, + GCType gc_type, void* data) { DCHECK_NOT_NULL(callback); DCHECK(gc_epilogue_callbacks_.end() == std::find(gc_epilogue_callbacks_.begin(), gc_epilogue_callbacks_.end(), - GCCallbackPair(callback, gc_type, pass_isolate))); - gc_epilogue_callbacks_.emplace_back(callback, gc_type, pass_isolate); + GCCallbackTuple(callback, gc_type, data))); + gc_epilogue_callbacks_.emplace_back(callback, gc_type, data); } - -void Heap::RemoveGCEpilogueCallback(v8::Isolate::GCCallback callback) { +void Heap::RemoveGCEpilogueCallback(v8::Isolate::GCCallbackWithData callback, + void* data) { DCHECK_NOT_NULL(callback); for (size_t i = 0; i < gc_epilogue_callbacks_.size(); i++) { - if (gc_epilogue_callbacks_[i].callback == callback) { + if (gc_epilogue_callbacks_[i].callback == callback && + gc_epilogue_callbacks_[i].data == data) { gc_epilogue_callbacks_[i] = gc_epilogue_callbacks_.back(); gc_epilogue_callbacks_.pop_back(); return; diff --git a/deps/v8/src/heap/heap.h b/deps/v8/src/heap/heap.h index 22b92351474dc5..2ac1f663598843 100644 --- a/deps/v8/src/heap/heap.h +++ b/deps/v8/src/heap/heap.h @@ -1422,13 +1422,15 @@ class Heap { // Prologue/epilogue callback methods.======================================== // =========================================================================== - void AddGCPrologueCallback(v8::Isolate::GCCallback callback, - GCType gc_type_filter, bool pass_isolate = true); - void RemoveGCPrologueCallback(v8::Isolate::GCCallback callback); + void AddGCPrologueCallback(v8::Isolate::GCCallbackWithData callback, + GCType gc_type_filter, void* data); + void RemoveGCPrologueCallback(v8::Isolate::GCCallbackWithData callback, + void* data); - void AddGCEpilogueCallback(v8::Isolate::GCCallback callback, - GCType gc_type_filter, bool pass_isolate = true); - void RemoveGCEpilogueCallback(v8::Isolate::GCCallback callback); + void AddGCEpilogueCallback(v8::Isolate::GCCallbackWithData callback, + GCType gc_type_filter, void* data); + void RemoveGCEpilogueCallback(v8::Isolate::GCCallbackWithData callback, + void* data); void CallGCPrologueCallbacks(GCType gc_type, GCCallbackFlags flags); void CallGCEpilogueCallbacks(GCType gc_type, GCCallbackFlags flags); @@ -1588,17 +1590,17 @@ class Heap { RootListIndex index; }; - struct GCCallbackPair { - GCCallbackPair(v8::Isolate::GCCallback callback, GCType gc_type, - bool pass_isolate) - : callback(callback), gc_type(gc_type), pass_isolate(pass_isolate) {} + struct GCCallbackTuple { + GCCallbackTuple(v8::Isolate::GCCallbackWithData callback, GCType gc_type, + void* data) + : callback(callback), gc_type(gc_type), data(data) {} - bool operator==(const GCCallbackPair& other) const; - GCCallbackPair& operator=(const GCCallbackPair& other); + bool operator==(const GCCallbackTuple& other) const; + GCCallbackTuple& operator=(const GCCallbackTuple& other); - v8::Isolate::GCCallback callback; + v8::Isolate::GCCallbackWithData callback; GCType gc_type; - bool pass_isolate; + void* data; }; typedef String* (*ExternalStringTableUpdaterCallback)(Heap* heap, @@ -2262,8 +2264,8 @@ class Heap { Object* encountered_transition_arrays_; - std::vector gc_epilogue_callbacks_; - std::vector gc_prologue_callbacks_; + std::vector gc_epilogue_callbacks_; + std::vector gc_prologue_callbacks_; GetExternallyAllocatedMemoryInBytesCallback external_memory_callback_; diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 2d64279bf80dc6..ff77dd26cc5273 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -19626,6 +19626,19 @@ void EpilogueCallbackSecond(v8::Isolate* isolate, ++epilogue_call_count_second; } +void PrologueCallbackNew(v8::Isolate* isolate, v8::GCType, + v8::GCCallbackFlags flags, void* data) { + CHECK_EQ(flags, v8::kNoGCCallbackFlags); + CHECK_EQ(gc_callbacks_isolate, isolate); + ++*static_cast(data); +} + +void EpilogueCallbackNew(v8::Isolate* isolate, v8::GCType, + v8::GCCallbackFlags flags, void* data) { + CHECK_EQ(flags, v8::kNoGCCallbackFlags); + CHECK_EQ(gc_callbacks_isolate, isolate); + ++*static_cast(data); +} void PrologueCallbackAlloc(v8::Isolate* isolate, v8::GCType, @@ -19700,6 +19713,52 @@ TEST(GCCallbacksOld) { CHECK_EQ(2, epilogue_call_count_second); } +TEST(GCCallbacksWithData) { + LocalContext context; + + gc_callbacks_isolate = context->GetIsolate(); + int prologue1 = 0; + int epilogue1 = 0; + int prologue2 = 0; + int epilogue2 = 0; + + context->GetIsolate()->AddGCPrologueCallback(PrologueCallbackNew, &prologue1); + context->GetIsolate()->AddGCEpilogueCallback(EpilogueCallbackNew, &epilogue1); + CHECK_EQ(0, prologue1); + CHECK_EQ(0, epilogue1); + CHECK_EQ(0, prologue2); + CHECK_EQ(0, epilogue2); + CcTest::CollectAllGarbage(); + CHECK_EQ(1, prologue1); + CHECK_EQ(1, epilogue1); + CHECK_EQ(0, prologue2); + CHECK_EQ(0, epilogue2); + context->GetIsolate()->AddGCPrologueCallback(PrologueCallbackNew, &prologue2); + context->GetIsolate()->AddGCEpilogueCallback(EpilogueCallbackNew, &epilogue2); + CcTest::CollectAllGarbage(); + CHECK_EQ(2, prologue1); + CHECK_EQ(2, epilogue1); + CHECK_EQ(1, prologue2); + CHECK_EQ(1, epilogue2); + context->GetIsolate()->RemoveGCPrologueCallback(PrologueCallbackNew, + &prologue1); + context->GetIsolate()->RemoveGCEpilogueCallback(EpilogueCallbackNew, + &epilogue1); + CcTest::CollectAllGarbage(); + CHECK_EQ(2, prologue1); + CHECK_EQ(2, epilogue1); + CHECK_EQ(2, prologue2); + CHECK_EQ(2, epilogue2); + context->GetIsolate()->RemoveGCPrologueCallback(PrologueCallbackNew, + &prologue2); + context->GetIsolate()->RemoveGCEpilogueCallback(EpilogueCallbackNew, + &epilogue2); + CcTest::CollectAllGarbage(); + CHECK_EQ(2, prologue1); + CHECK_EQ(2, epilogue1); + CHECK_EQ(2, prologue2); + CHECK_EQ(2, epilogue2); +} TEST(GCCallbacks) { LocalContext context; From 70ddf36046f5e2e56d583caa3b04fd97c4739936 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 4 Sep 2017 03:07:13 +0200 Subject: [PATCH 3/3] src: keep track of env properly in node_perf.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, measuring GC timing using `node_perf` is somewhat broken, because Isolates and Node Environments do not necessarily match 1:1; each environment adds its own hook, so possibly the hook code runs multiple times, but since it can’t reliably compute its corresponding event loop based on the Isolate, each run targets the same Environment right now. This fixes that problem by using new overloads of the GC tracking APIs that can pass data to the callback through opaque pointers. --- src/node_perf.cc | 34 +++++++++++++++++++--------------- src/node_perf.h | 7 +++++++ 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/node_perf.cc b/src/node_perf.cc index 48917d5d4ea971..0cd89ae19e0fa3 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -174,13 +174,14 @@ void SetupPerformanceObservers(const FunctionCallbackInfo& args) { env->set_performance_entry_callback(args[0].As()); } -inline void PerformanceGCCallback(uv_async_t* handle) { +void PerformanceGCCallback(uv_async_t* handle) { PerformanceEntry::Data* data = static_cast(handle->data); - Isolate* isolate = Isolate::GetCurrent(); + Environment* env = data->env(); + Isolate* isolate = env->isolate(); HandleScope scope(isolate); - Environment* env = Environment::GetCurrent(isolate); Local context = env->context(); + Context::Scope context_scope(context); Local fn; Local obj; PerformanceGCKind kind = static_cast(data->data()); @@ -203,28 +204,31 @@ inline void PerformanceGCCallback(uv_async_t* handle) { uv_close(reinterpret_cast(handle), closeCB); } -inline void MarkGarbageCollectionStart(Isolate* isolate, - v8::GCType type, - v8::GCCallbackFlags flags) { +void MarkGarbageCollectionStart(Isolate* isolate, + v8::GCType type, + v8::GCCallbackFlags flags) { performance_last_gc_start_mark_ = PERFORMANCE_NOW(); performance_last_gc_type_ = type; } -inline void MarkGarbageCollectionEnd(Isolate* isolate, - v8::GCType type, - v8::GCCallbackFlags flags) { +void MarkGarbageCollectionEnd(Isolate* isolate, + v8::GCType type, + v8::GCCallbackFlags flags, + void* data) { + Environment* env = static_cast(data); uv_async_t *async = new uv_async_t; async->data = - new PerformanceEntry::Data("gc", "gc", + new PerformanceEntry::Data(env, "gc", "gc", performance_last_gc_start_mark_, PERFORMANCE_NOW(), type); - uv_async_init(uv_default_loop(), async, PerformanceGCCallback); + uv_async_init(env->event_loop(), async, PerformanceGCCallback); uv_async_send(async); } -inline void SetupGarbageCollectionTracking(Isolate* isolate) { - isolate->AddGCPrologueCallback(MarkGarbageCollectionStart); - isolate->AddGCEpilogueCallback(MarkGarbageCollectionEnd); +inline void SetupGarbageCollectionTracking(Environment* env) { + env->isolate()->AddGCPrologueCallback(MarkGarbageCollectionStart); + env->isolate()->AddGCEpilogueCallback(MarkGarbageCollectionEnd, + static_cast(env)); } inline Local GetName(Local fn) { @@ -380,7 +384,7 @@ void Init(Local target, constants, attr).ToChecked(); - SetupGarbageCollectionTracking(isolate); + SetupGarbageCollectionTracking(env); } } // namespace performance diff --git a/src/node_perf.h b/src/node_perf.h index 412479c90236d1..ca4374ebc58c19 100644 --- a/src/node_perf.h +++ b/src/node_perf.h @@ -56,17 +56,23 @@ class PerformanceEntry : public BaseObject { class Data { public: Data( + Environment* env, const char* name, const char* type, uint64_t startTime, uint64_t endTime, int data = 0) : + env_(env), name_(name), type_(type), startTime_(startTime), endTime_(endTime), data_(data) {} + Environment* env() const { + return env_; + } + std::string name() const { return name_; } @@ -88,6 +94,7 @@ class PerformanceEntry : public BaseObject { } private: + Environment* env_; std::string name_; std::string type_; uint64_t startTime_;