From d62e941b32ec7c4bf7224e0a084595dd6a942225 Mon Sep 17 00:00:00 2001 From: ishabi Date: Tue, 28 Jul 2026 12:02:30 +0200 Subject: [PATCH] src: fix heap value deduplication in embedder graph Signed-off-by: ishabi --- src/heap_utils.cc | 40 ++++++++++++----------- test/parallel/test-heap-embedder-graph.js | 23 +++++++++++++ 2 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-heap-embedder-graph.js diff --git a/src/heap_utils.cc b/src/heap_utils.cc index e52685546a7ac2..72c1a73aa6c06f 100644 --- a/src/heap_utils.cc +++ b/src/heap_utils.cc @@ -57,19 +57,13 @@ class JSGraphJSNode : public EmbedderGraph::Node { CHECK(!val.IsEmpty()); } - struct Equal { - inline bool operator()(JSGraphJSNode* a, JSGraphJSNode* b) const { - Local data_a = a->V8Value(); - Local data_b = a->V8Value(); - if (data_a->IsValue()) { - if (!data_b->IsValue()) { - return false; - } - return data_a.As()->SameValue(data_b.As()); - } - return data_a == data_b; + bool IsSame(Local other) { + Local value = V8Value(); + if (value->IsValue() && other->IsValue()) { + return value.As()->SameValue(other.As()); } - }; + return value == other; + } private: Global persistent_; @@ -80,12 +74,15 @@ class JSGraph : public EmbedderGraph { explicit JSGraph(Isolate* isolate) : isolate_(isolate) {} Node* V8Node(const Local& value) override { - std::unique_ptr n { new JSGraphJSNode(isolate_, value) }; - auto it = engine_nodes_.find(n.get()); - if (it != engine_nodes_.end()) - return *it; - engine_nodes_.insert(n.get()); - return AddNode(std::unique_ptr(n.release())); + for (JSGraphJSNode* node : engine_nodes_) { + if (node->IsSame(value)) { + return node; + } + } + + auto node = std::make_unique(isolate_, value); + engine_nodes_.push_back(node.get()); + return AddNode(std::move(node)); } Node* V8Node(const Local& value) override { @@ -207,7 +204,7 @@ class JSGraph : public EmbedderGraph { private: Isolate* isolate_; std::unordered_set> nodes_; - std::set engine_nodes_; + std::vector engine_nodes_; std::unordered_map>> edges_; }; @@ -215,6 +212,11 @@ void BuildEmbedderGraph(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); JSGraph graph(env->isolate()); Environment::BuildEmbedderGraph(env->isolate(), &graph, env); + // This binding is used only by tests. Include supplied values so tests can + // verify that JSGraph returns one graph node for each distinct V8 value. + for (int i = 0; i < args.Length(); i++) { + graph.V8Node(args[i]); + } Local ret; if (graph.CreateObject().ToLocal(&ret)) args.GetReturnValue().Set(ret); diff --git a/test/parallel/test-heap-embedder-graph.js b/test/parallel/test-heap-embedder-graph.js new file mode 100644 index 00000000000000..1873fd1a49ca32 --- /dev/null +++ b/test/parallel/test-heap-embedder-graph.js @@ -0,0 +1,23 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const assert = require('assert'); +const { internalBinding } = require('internal/test/binding'); + +const { buildEmbedderGraph } = internalBinding('heap_utils'); + +const first = {}; +const second = {}; +const bigint = BigInt('123456789012345678901234567890'); +const sameBigint = BigInt('123456789012345678901234567890'); +const graph = buildEmbedderGraph(first, first, second, bigint, sameBigint); + +function findNodes(value) { + return graph.filter((node) => Object.hasOwn(node, 'value') && + Object.is(node.value, value)); +} + +assert.strictEqual(findNodes(first).length, 1); +assert.strictEqual(findNodes(second).length, 1); +assert.strictEqual(findNodes(bigint).length, 1);