cppgc-js: Account for C++ object sizes

Previously, for wrapper/wrappable pairs, only JS object size was
accounted for. With this change, the C++ part is also accounted for.

Change-Id: Ibd945cb28c808d8c01fa41453f94a6de9883b764
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2615258
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71959}
This commit is contained in:
Michael Lippautz 2021-01-07 18:40:15 +01:00 committed by Commit Bot
parent ff3fe27a30
commit 34e7ae615d
4 changed files with 42 additions and 27 deletions

View File

@ -34,11 +34,12 @@ using cppgc::internal::HeapObjectHeader;
// Node representing a C++ object on the heap. // Node representing a C++ object on the heap.
class EmbedderNode : public v8::EmbedderGraph::Node { class EmbedderNode : public v8::EmbedderGraph::Node {
public: public:
explicit EmbedderNode(const char* name) : name_(name) {} explicit EmbedderNode(const char* name, size_t size)
: name_(name), size_(size) {}
~EmbedderNode() override = default; ~EmbedderNode() override = default;
const char* Name() final { return name_; } const char* Name() final { return name_; }
size_t SizeInBytes() override { return 0; } size_t SizeInBytes() final { return size_; }
void SetWrapperNode(v8::EmbedderGraph::Node* wrapper_node) { void SetWrapperNode(v8::EmbedderGraph::Node* wrapper_node) {
wrapper_node_ = wrapper_node; wrapper_node_ = wrapper_node;
@ -52,6 +53,7 @@ class EmbedderNode : public v8::EmbedderGraph::Node {
private: private:
const char* name_; const char* name_;
size_t size_;
Node* wrapper_node_ = nullptr; Node* wrapper_node_ = nullptr;
Detachedness detachedness_ = Detachedness::kUnknown; Detachedness detachedness_ = Detachedness::kUnknown;
}; };
@ -59,11 +61,10 @@ class EmbedderNode : public v8::EmbedderGraph::Node {
// Node representing an artificial root group, e.g., set of Persistent handles. // Node representing an artificial root group, e.g., set of Persistent handles.
class EmbedderRootNode final : public EmbedderNode { class EmbedderRootNode final : public EmbedderNode {
public: public:
explicit EmbedderRootNode(const char* name) : EmbedderNode(name) {} explicit EmbedderRootNode(const char* name) : EmbedderNode(name, 0) {}
~EmbedderRootNode() final = default; ~EmbedderRootNode() final = default;
bool IsRootNode() final { return true; } bool IsRootNode() final { return true; }
size_t SizeInBytes() final { return 0; }
}; };
// Canonical state representing real and artificial (e.g. root) objects. // Canonical state representing real and artificial (e.g. root) objects.
@ -373,7 +374,7 @@ class CppGraphBuilderImpl final {
EmbedderNode* AddNode(const HeapObjectHeader& header) { EmbedderNode* AddNode(const HeapObjectHeader& header) {
return static_cast<EmbedderNode*>( return static_cast<EmbedderNode*>(
graph_.AddNode(std::unique_ptr<v8::EmbedderGraph::Node>{ graph_.AddNode(std::unique_ptr<v8::EmbedderGraph::Node>{
new EmbedderNode(header.GetName().value)})); new EmbedderNode(header.GetName().value, header.GetSize())}));
} }
void AddEdge(State& parent, const HeapObjectHeader& header) { void AddEdge(State& parent, const HeapObjectHeader& header) {

View File

@ -2000,6 +2000,9 @@ void NativeObjectsExplorer::MergeNodeIntoEntry(
entry->set_name(MergeNames( entry->set_name(MergeNames(
names_, EmbedderGraphNodeName(names_, original_node), entry->name())); names_, EmbedderGraphNodeName(names_, original_node), entry->name()));
entry->set_type(EmbedderGraphNodeType(original_node)); entry->set_type(EmbedderGraphNodeType(original_node));
DCHECK_GE(entry->self_size() + original_node->SizeInBytes(),
entry->self_size());
entry->add_self_size(original_node->SizeInBytes());
} }
HeapEntry* NativeObjectsExplorer::EntryForEmbedderGraphNode( HeapEntry* NativeObjectsExplorer::EntryForEmbedderGraphNode(

View File

@ -126,6 +126,7 @@ class HeapEntry {
void set_name(const char* name) { name_ = name; } void set_name(const char* name) { name_ = name; }
SnapshotObjectId id() const { return id_; } SnapshotObjectId id() const { return id_; }
size_t self_size() const { return self_size_; } size_t self_size() const { return self_size_; }
void add_self_size(size_t size) { self_size_ += size; }
unsigned trace_node_id() const { return trace_node_id_; } unsigned trace_node_id() const { return trace_node_id_; }
int index() const { return index_; } int index() const { return index_; }
V8_INLINE int children_count() const; V8_INLINE int children_count() const;

View File

@ -14,7 +14,9 @@
#include "include/v8-profiler.h" #include "include/v8-profiler.h"
#include "src/api/api-inl.h" #include "src/api/api-inl.h"
#include "src/heap/cppgc-js/cpp-heap.h" #include "src/heap/cppgc-js/cpp-heap.h"
#include "src/heap/cppgc/heap-object-header.h"
#include "src/heap/cppgc/object-allocator.h" #include "src/heap/cppgc/object-allocator.h"
#include "src/objects/heap-object.h"
#include "src/objects/objects-inl.h" #include "src/objects/objects-inl.h"
#include "src/profiler/heap-snapshot-generator-inl.h" #include "src/profiler/heap-snapshot-generator-inl.h"
#include "src/profiler/heap-snapshot-generator.h" #include "src/profiler/heap-snapshot-generator.h"
@ -314,6 +316,18 @@ cppgc::Persistent<GCedWithJSRef> SetupWrapperWrappablePair(
return std::move(gc_w_js_ref); return std::move(gc_w_js_ref);
} }
template <typename Callback>
void ForEachEntryWithName(const v8::HeapSnapshot* snapshot, const char* needle,
Callback callback) {
const HeapSnapshot* heap_snapshot =
reinterpret_cast<const HeapSnapshot*>(snapshot);
for (const HeapEntry& entry : heap_snapshot->entries()) {
if (strcmp(entry.name(), needle) == 0) {
callback(entry);
}
}
}
} // namespace } // namespace
TEST_F(UnifiedHeapSnapshotTest, JSReferenceForcesVisibleObject) { TEST_F(UnifiedHeapSnapshotTest, JSReferenceForcesVisibleObject) {
@ -335,8 +349,8 @@ TEST_F(UnifiedHeapSnapshotTest, JSReferenceForcesVisibleObject) {
TEST_F(UnifiedHeapSnapshotTest, MergedWrapperNode) { TEST_F(UnifiedHeapSnapshotTest, MergedWrapperNode) {
// Test ensures that the snapshot sets a wrapper node for C++->JS references // Test ensures that the snapshot sets a wrapper node for C++->JS references
// that have a class id set and that object nodes are merged into the C++ // that have a class id set and that object nodes are merged. In practice, the
// node, i.e., the directly reachable JS object is merged into the C++ object. // C++ node is merged into the existing JS node.
JsTestingScope testing_scope(v8_isolate()); JsTestingScope testing_scope(v8_isolate());
cppgc::Persistent<GCedWithJSRef> gc_w_js_ref = SetupWrapperWrappablePair( cppgc::Persistent<GCedWithJSRef> gc_w_js_ref = SetupWrapperWrappablePair(
testing_scope, allocation_handle(), "MergedObject"); testing_scope, allocation_handle(), "MergedObject");
@ -356,14 +370,22 @@ TEST_F(UnifiedHeapSnapshotTest, MergedWrapperNode) {
.ToChecked(); .ToChecked();
const v8::HeapSnapshot* snapshot = TakeHeapSnapshot(); const v8::HeapSnapshot* snapshot = TakeHeapSnapshot();
EXPECT_TRUE(IsValidSnapshot(snapshot)); EXPECT_TRUE(IsValidSnapshot(snapshot));
EXPECT_TRUE( EXPECT_TRUE(ContainsRetainingPath(
ContainsRetainingPath(*snapshot, *snapshot,
{ {
kExpectedCppRootsName, // NOLINT kExpectedCppRootsName, // NOLINT
GetExpectedName<GCedWithJSRef>(), // NOLINT GetExpectedName<GCedWithJSRef>(), // NOLINT
// MergedObject is merged into GCedWithJSRef. // GCedWithJSRef is merged into MergedObject, replacing its name.
"NextObject" // NOLINT "NextObject" // NOLINT
})); }));
const size_t cpp_size =
cppgc::internal::HeapObjectHeader::FromPayload(gc_w_js_ref.Get())
.GetSize();
const size_t js_size = Utils::OpenHandle(*wrapper_object)->Size();
ForEachEntryWithName(snapshot, GetExpectedName<GCedWithJSRef>(),
[cpp_size, js_size](const HeapEntry& entry) {
EXPECT_EQ(cpp_size + js_size, entry.self_size());
});
} }
namespace { namespace {
@ -388,18 +410,6 @@ class DetachednessHandler {
// static // static
size_t DetachednessHandler::callback_count = 0; size_t DetachednessHandler::callback_count = 0;
template <typename Callback>
void ForEachEntryWithName(const v8::HeapSnapshot* snapshot, const char* needle,
Callback callback) {
const HeapSnapshot* heap_snapshot =
reinterpret_cast<const HeapSnapshot*>(snapshot);
for (const HeapEntry& entry : heap_snapshot->entries()) {
if (strcmp(entry.name(), needle) == 0) {
callback(entry);
}
}
}
constexpr uint8_t kExpectedDetachedValueForUnknown = constexpr uint8_t kExpectedDetachedValueForUnknown =
static_cast<uint8_t>(v8::EmbedderGraph::Node::Detachedness::kUnknown); static_cast<uint8_t>(v8::EmbedderGraph::Node::Detachedness::kUnknown);
constexpr uint8_t kExpectedDetachedValueForAttached = constexpr uint8_t kExpectedDetachedValueForAttached =