From 8da9d969f927971d20eeacbbca7da34d80bc68b5 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 24 Nov 2023 22:38:22 +0100 Subject: [PATCH] deps: V8: cherry-pick 0fd478bcdabd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [heap-profiler]: expose QueryObjects() to v8::HeapProfiler This allows embedders to use this API for testing memory leaks more reliably. See https://github.com/nodejs/node/pull/50572 for an example about how the API can be used. Change-Id: Ic3d1268e2b331c37e8ec92997b764b9b5486f8c2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5006373 Reviewed-by: Camillo Bruni Reviewed-by: Simon Zünd Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#91123} Refs: https://github.com/v8/v8/commit/0fd478bcdabd3400d9d74c47c4883c085ef37d18 PR-URL: https://github.com/nodejs/node/pull/50572 Reviewed-By: Geoffrey Booth Reviewed-By: Stephen Belanger --- common.gypi | 2 +- deps/v8/include/v8-profiler.h | 10 ++++ deps/v8/src/api/api.cc | 10 ++++ deps/v8/src/debug/debug-interface.cc | 9 ---- deps/v8/src/debug/debug-interface.h | 10 ---- deps/v8/src/inspector/v8-debugger.cc | 5 +- deps/v8/src/profiler/heap-profiler.cc | 2 +- deps/v8/src/profiler/heap-profiler.h | 5 +- deps/v8/test/cctest/test-heap-profiler.cc | 62 +++++++++++++++++++++++ 9 files changed, 89 insertions(+), 26 deletions(-) diff --git a/common.gypi b/common.gypi index 28522c9fbf83b4..c6b9fa05b44c96 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.17', + 'v8_embedder_string': '-node.18', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-profiler.h b/deps/v8/include/v8-profiler.h index 75c9d6a4ae7863..3ec0cd4bb31318 100644 --- a/deps/v8/include/v8-profiler.h +++ b/deps/v8/include/v8-profiler.h @@ -921,12 +921,22 @@ class V8_EXPORT EmbedderGraph { virtual ~EmbedderGraph() = default; }; +class QueryObjectPredicate { + public: + virtual ~QueryObjectPredicate() = default; + virtual bool Filter(v8::Local object) = 0; +}; + /** * Interface for controlling heap profiling. Instance of the * profiler can be retrieved using v8::Isolate::GetHeapProfiler. */ class V8_EXPORT HeapProfiler { public: + void QueryObjects(v8::Local context, + QueryObjectPredicate* predicate, + std::vector>* objects); + enum SamplingFlags { kSamplingNoFlags = 0, kSamplingForceGC = 1 << 0, diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index 2708ad9b2e55c7..7807545da57f23 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -10917,6 +10917,16 @@ int HeapProfiler::GetSnapshotCount() { return reinterpret_cast(this)->GetSnapshotsCount(); } +void HeapProfiler::QueryObjects(Local v8_context, + QueryObjectPredicate* predicate, + std::vector>* objects) { + i::Isolate* isolate = reinterpret_cast(v8_context->GetIsolate()); + i::HeapProfiler* profiler = reinterpret_cast(this); + DCHECK_EQ(isolate, profiler->isolate()); + ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); + profiler->QueryObjects(Utils::OpenHandle(*v8_context), predicate, objects); +} + const HeapSnapshot* HeapProfiler::GetHeapSnapshot(int index) { return reinterpret_cast( reinterpret_cast(this)->GetSnapshot(index)); diff --git a/deps/v8/src/debug/debug-interface.cc b/deps/v8/src/debug/debug-interface.cc index fb3b6b69e4be80..51e3633853ba56 100644 --- a/deps/v8/src/debug/debug-interface.cc +++ b/deps/v8/src/debug/debug-interface.cc @@ -1211,15 +1211,6 @@ v8::MaybeLocal EvaluateGlobalForTesting( RETURN_ESCAPED(result); } -void QueryObjects(v8::Local v8_context, - QueryObjectPredicate* predicate, - std::vector>* objects) { - i::Isolate* isolate = reinterpret_cast(v8_context->GetIsolate()); - ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); - isolate->heap_profiler()->QueryObjects(Utils::OpenHandle(*v8_context), - predicate, objects); -} - void GlobalLexicalScopeNames(v8::Local v8_context, std::vector>* names) { i::Handle context = Utils::OpenHandle(*v8_context); diff --git a/deps/v8/src/debug/debug-interface.h b/deps/v8/src/debug/debug-interface.h index 5dfd505fed6f14..eb346b326392a7 100644 --- a/deps/v8/src/debug/debug-interface.h +++ b/deps/v8/src/debug/debug-interface.h @@ -530,16 +530,6 @@ class V8_EXPORT_PRIVATE StackTraceIterator { bool throw_on_side_effect) = 0; }; -class QueryObjectPredicate { - public: - virtual ~QueryObjectPredicate() = default; - virtual bool Filter(v8::Local object) = 0; -}; - -void QueryObjects(v8::Local context, - QueryObjectPredicate* predicate, - std::vector>* objects); - void GlobalLexicalScopeNames(v8::Local context, std::vector>* names); diff --git a/deps/v8/src/inspector/v8-debugger.cc b/deps/v8/src/inspector/v8-debugger.cc index 3d4733b5ba936e..d8a08b422505f7 100644 --- a/deps/v8/src/inspector/v8-debugger.cc +++ b/deps/v8/src/inspector/v8-debugger.cc @@ -8,6 +8,7 @@ #include "include/v8-context.h" #include "include/v8-function.h" #include "include/v8-microtask-queue.h" +#include "include/v8-profiler.h" #include "include/v8-util.h" #include "src/inspector/inspected-context.h" #include "src/inspector/protocol/Protocol.h" @@ -38,7 +39,7 @@ void cleanupExpiredWeakPointers(Map& map) { } } -class MatchPrototypePredicate : public v8::debug::QueryObjectPredicate { +class MatchPrototypePredicate : public v8::QueryObjectPredicate { public: MatchPrototypePredicate(V8InspectorImpl* inspector, v8::Local context, @@ -994,7 +995,7 @@ v8::Local V8Debugger::queryObjects(v8::Local context, v8::Isolate* isolate = context->GetIsolate(); std::vector> v8_objects; MatchPrototypePredicate predicate(m_inspector, context, prototype); - v8::debug::QueryObjects(context, &predicate, &v8_objects); + isolate->GetHeapProfiler()->QueryObjects(context, &predicate, &v8_objects); v8::MicrotasksScope microtasksScope(context, v8::MicrotasksScope::kDoNotRunMicrotasks); diff --git a/deps/v8/src/profiler/heap-profiler.cc b/deps/v8/src/profiler/heap-profiler.cc index a23e6d94d1717e..2a3ed138022d5a 100644 --- a/deps/v8/src/profiler/heap-profiler.cc +++ b/deps/v8/src/profiler/heap-profiler.cc @@ -247,7 +247,7 @@ Heap* HeapProfiler::heap() const { return ids_->heap(); } Isolate* HeapProfiler::isolate() const { return heap()->isolate(); } void HeapProfiler::QueryObjects(Handle context, - debug::QueryObjectPredicate* predicate, + v8::QueryObjectPredicate* predicate, std::vector>* objects) { // We need a stack marker here to allow deterministic passes over the stack. // The garbage collection and the two object heap iterators should scan the diff --git a/deps/v8/src/profiler/heap-profiler.h b/deps/v8/src/profiler/heap-profiler.h index 2756ade8141531..851972a729a806 100644 --- a/deps/v8/src/profiler/heap-profiler.h +++ b/deps/v8/src/profiler/heap-profiler.h @@ -30,7 +30,7 @@ class StringsStorage; // generate consistent IDs for moved objects. class HeapProfilerNativeMoveListener { public: - HeapProfilerNativeMoveListener(HeapProfiler* profiler) + explicit HeapProfilerNativeMoveListener(HeapProfiler* profiler) : profiler_(profiler) {} HeapProfilerNativeMoveListener(const HeapProfilerNativeMoveListener& other) = delete; @@ -116,8 +116,7 @@ class HeapProfiler : public HeapObjectAllocationTracker { Isolate* isolate() const; - void QueryObjects(Handle context, - debug::QueryObjectPredicate* predicate, + void QueryObjects(Handle context, QueryObjectPredicate* predicate, std::vector>* objects); void set_native_move_listener( std::unique_ptr listener) { diff --git a/deps/v8/test/cctest/test-heap-profiler.cc b/deps/v8/test/cctest/test-heap-profiler.cc index 1ca4a4808de2b3..4c685f90c26c54 100644 --- a/deps/v8/test/cctest/test-heap-profiler.cc +++ b/deps/v8/test/cctest/test-heap-profiler.cc @@ -30,6 +30,7 @@ #include #include +#include #include "include/v8-function.h" #include "include/v8-json.h" @@ -4064,6 +4065,67 @@ TEST(SamplingHeapProfilerSampleDuringDeopt) { heap_profiler->StopSamplingHeapProfiler(); } +namespace { +class TestQueryObjectPredicate : public v8::QueryObjectPredicate { + public: + TestQueryObjectPredicate(v8::Local context, + v8::Local symbol) + : context_(context), symbol_(symbol) {} + + bool Filter(v8::Local object) override { + return object->HasOwnProperty(context_, symbol_).FromMaybe(false); + } + + private: + v8::Local context_; + v8::Local symbol_; +}; + +class IncludeAllQueryObjectPredicate : public v8::QueryObjectPredicate { + public: + IncludeAllQueryObjectPredicate() {} + bool Filter(v8::Local object) override { return true; } +}; +} // anonymous namespace + +TEST(QueryObjects) { + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope scope(isolate); + v8::Local context = env.local(); + + v8::Local sym = + v8::Symbol::New(isolate, v8_str("query_object_test")); + context->Global()->Set(context, v8_str("test_symbol"), sym).Check(); + v8::Local arr = CompileRun(R"( + const arr = []; + for (let i = 0; i < 10; ++i) { + arr.push({[test_symbol]: true}); + } + arr; + )"); + context->Global()->Set(context, v8_str("arr"), arr).Check(); + v8::HeapProfiler* heap_profiler = isolate->GetHeapProfiler(); + + { + TestQueryObjectPredicate predicate(context, sym); + std::vector> out; + heap_profiler->QueryObjects(context, &predicate, &out); + + CHECK_EQ(out.size(), 10); + for (size_t i = 0; i < out.size(); ++i) { + CHECK(out[i].Get(isolate)->HasOwnProperty(context, sym).FromMaybe(false)); + } + } + + { + IncludeAllQueryObjectPredicate predicate; + std::vector> out; + heap_profiler->QueryObjects(context, &predicate, &out); + CHECK_GE(out.size(), 10); + } +} + TEST(WeakReference) { v8::Isolate* isolate = CcTest::isolate(); i::Isolate* i_isolate = CcTest::i_isolate();