cppgc: Allow overriding heap object name at runtime

Before this CL, the heap object name of unnamed objects(those not
inheriting from NameProvider) would be solely determined by whether the
build-time configuration cppgc_enable_object_names is enabled.

This patch adds a way to override that value at runtime. This is
useful for preserving default behavior with custom builds but at the
same time allow them to still enable the feature.

Bug: chromium:1321620
Change-Id: I3aa06db15e58d9ba9773be6797572f17f007e9ee
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3620279
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80338}
This commit is contained in:
Michael Lippautz 2022-05-02 18:21:20 +02:00 committed by V8 LUCI CQ
parent 46224e75f3
commit b0fbe1aff3
8 changed files with 85 additions and 34 deletions

View File

@ -6,6 +6,7 @@
#define INCLUDE_CPPGC_INTERNAL_NAME_TRAIT_H_
#include <cstddef>
#include <cstdint>
#include <type_traits>
#include "cppgc/name-provider.h"
@ -58,6 +59,11 @@ struct HeapObjectName {
bool name_was_hidden;
};
enum class HeapObjectNameForUnnamedObject : uint8_t {
kUseClassNameIfSupported,
kUseHiddenName,
};
class V8_EXPORT NameTraitBase {
protected:
static HeapObjectName GetNameFromTypeSignature(const char*);
@ -78,16 +84,24 @@ class NameTrait final : public NameTraitBase {
#endif // !CPPGC_SUPPORTS_OBJECT_NAMES
}
static HeapObjectName GetName(const void* obj) {
return GetNameFor(static_cast<const T*>(obj));
static HeapObjectName GetName(
const void* obj, HeapObjectNameForUnnamedObject name_retrieval_mode) {
return GetNameFor(static_cast<const T*>(obj), name_retrieval_mode);
}
private:
static HeapObjectName GetNameFor(const NameProvider* name_provider) {
static HeapObjectName GetNameFor(const NameProvider* name_provider,
HeapObjectNameForUnnamedObject) {
// Objects inheriting from `NameProvider` are not considered unnamed as
// users already provided a name for them.
return {name_provider->GetHumanReadableName(), false};
}
static HeapObjectName GetNameFor(...) {
static HeapObjectName GetNameFor(
const void*, HeapObjectNameForUnnamedObject name_retrieval_mode) {
if (name_retrieval_mode == HeapObjectNameForUnnamedObject::kUseHiddenName)
return {NameProvider::kHiddenName, true};
#if CPPGC_SUPPORTS_COMPILE_TIME_TYPENAME
return {GetTypename<T>(), false};
#elif CPPGC_SUPPORTS_OBJECT_NAMES
@ -112,7 +126,8 @@ class NameTrait final : public NameTraitBase {
}
};
using NameCallback = HeapObjectName (*)(const void*);
using NameCallback = HeapObjectName (*)(const void*,
HeapObjectNameForUnnamedObject);
} // namespace internal
} // namespace cppgc

View File

@ -37,15 +37,15 @@ class V8_EXPORT NameProvider {
static constexpr const char kNoNameDeducible[] = "<No name>";
/**
* Indicating whether internal names are hidden or not.
* Indicating whether the build supports extracting C++ names as object names.
*
* @returns true if C++ names should be hidden and represented by kHiddenName.
*/
static constexpr bool HideInternalNames() {
static constexpr bool SupportsCppClassNamesAsObjectNames() {
#if CPPGC_SUPPORTS_OBJECT_NAMES
return false;
#else // !CPPGC_SUPPORTS_OBJECT_NAMES
return true;
#else // !CPPGC_SUPPORTS_OBJECT_NAMES
return false;
#endif // !CPPGC_SUPPORTS_OBJECT_NAMES
}

View File

@ -13,7 +13,7 @@ namespace internal {
namespace {
HeapObjectName GetHiddenName(const void*) {
HeapObjectName GetHiddenName(const void*, HeapObjectNameForUnnamedObject) {
return {NameProvider::kHiddenName, true};
}

View File

@ -73,6 +73,8 @@ class PageBackend;
class PreFinalizerHandler;
class StatsCollector;
enum class HeapObjectNameForUnnamedObject : uint8_t;
// Base class for heap implementations.
class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
public:
@ -224,6 +226,14 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
return supported;
}
// Returns whether objects should derive their name from C++ class names. Also
// requires build-time support through `CPPGC_SUPPORTS_OBJECT_NAMES`.
HeapObjectNameForUnnamedObject name_of_unnamed_object() const {
// TODO(chromium:1321620): Wire up flag with heap snapshot if exposing
// internals is requested.
return HeapObjectNameForUnnamedObject::kUseClassNameIfSupported;
}
protected:
enum class GenerationSupport : uint8_t {
kSingleGeneration,

View File

@ -8,6 +8,7 @@
#include "src/base/macros.h"
#include "src/base/sanitizer/asan.h"
#include "src/heap/cppgc/gc-info-table.h"
#include "src/heap/cppgc/heap-base.h"
#include "src/heap/cppgc/heap-page.h"
namespace cppgc {
@ -38,7 +39,9 @@ void HeapObjectHeader::Finalize() {
HeapObjectName HeapObjectHeader::GetName() const {
const GCInfo& gc_info = GlobalGCInfoTable::GCInfoFromIndex(GetGCInfoIndex());
return gc_info.name(ObjectStart());
return gc_info.name(
ObjectStart(),
BasePage::FromPayload(this)->heap().name_of_unnamed_object());
}
} // namespace internal

View File

@ -76,7 +76,7 @@ void RecordObjectType(
std::unordered_map<const void*, size_t>& type_map,
std::vector<HeapStatistics::ObjectStatsEntry>& object_statistics,
HeapObjectHeader* header, size_t object_size) {
if (!NameProvider::HideInternalNames()) {
if (NameProvider::SupportsCppClassNamesAsObjectNames()) {
// Tries to insert a new entry into the typemap with a running counter. If
// the entry is already present, just returns the old one.
const auto it = type_map.insert({header->GetName().value, type_map.size()});
@ -97,7 +97,7 @@ HeapStatistics HeapStatisticsCollector::CollectDetailedStatistics(
stats.detail_level = HeapStatistics::DetailLevel::kDetailed;
current_stats_ = &stats;
if (!NameProvider::HideInternalNames()) {
if (NameProvider::SupportsCppClassNamesAsObjectNames()) {
// Add a dummy type so that a type index of zero has a valid mapping but
// shows an invalid type.
type_name_to_index_map_.insert({"Invalid type", 0});
@ -106,7 +106,7 @@ HeapStatistics HeapStatisticsCollector::CollectDetailedStatistics(
Traverse(heap->raw_heap());
FinalizeSpace(current_stats_, &current_space_stats_, &current_page_stats_);
if (!NameProvider::HideInternalNames()) {
if (NameProvider::SupportsCppClassNamesAsObjectNames()) {
stats.type_names.resize(type_name_to_index_map_.size());
for (auto& it : type_name_to_index_map_) {
stats.type_names[it.second] = reinterpret_cast<const char*>(it.first);

View File

@ -120,7 +120,7 @@ constexpr const char kExpectedCppCrossThreadRootsName[] =
template <typename T>
constexpr const char* GetExpectedName() {
if (std::is_base_of<cppgc::NameProvider, T>::value ||
!cppgc::NameProvider::HideInternalNames()) {
cppgc::NameProvider::SupportsCppClassNamesAsObjectNames()) {
return T::kExpectedName;
} else {
return cppgc::NameProvider::kHiddenName;
@ -157,7 +157,7 @@ TEST_F(UnifiedHeapSnapshotTest, RetainingUnnamedType) {
cppgc::MakeGarbageCollected<BaseWithoutName>(allocation_handle());
const v8::HeapSnapshot* snapshot = TakeHeapSnapshot();
EXPECT_TRUE(IsValidSnapshot(snapshot));
if (cppgc::NameProvider::HideInternalNames()) {
if (!cppgc::NameProvider::SupportsCppClassNamesAsObjectNames()) {
EXPECT_FALSE(ContainsRetainingPath(
*snapshot, {kExpectedCppRootsName, cppgc::NameProvider::kHiddenName}));
} else {

View File

@ -50,20 +50,43 @@ TEST(NameTraitTest, InternalNamesHiddenInOfficialBuild) {
}
TEST(NameTraitTest, DefaultName) {
EXPECT_STREQ(NameProvider::HideInternalNames()
? "InternalNode"
: "cppgc::internal::(anonymous namespace)::NoName",
NameTrait<NoName>::GetName(nullptr).value);
EXPECT_STREQ(NameProvider::HideInternalNames()
? "InternalNode"
: "cppgc::internal::(anonymous namespace)::OtherNoName",
NameTrait<OtherNoName>::GetName(nullptr).value);
EXPECT_STREQ(
NameProvider::SupportsCppClassNamesAsObjectNames()
? "cppgc::internal::(anonymous namespace)::NoName"
: "InternalNode",
NameTrait<NoName>::GetName(
nullptr, HeapObjectNameForUnnamedObject::kUseClassNameIfSupported)
.value);
EXPECT_STREQ(
NameProvider::SupportsCppClassNamesAsObjectNames()
? "cppgc::internal::(anonymous namespace)::OtherNoName"
: "InternalNode",
NameTrait<OtherNoName>::GetName(
nullptr, HeapObjectNameForUnnamedObject::kUseClassNameIfSupported)
.value);
// The following ignores `NameProvider::HideInternalNames()` and just always
// returns the hidden name, independent of the build support.
EXPECT_STREQ("InternalNode",
NameTrait<NoName>::GetName(
nullptr, HeapObjectNameForUnnamedObject::kUseHiddenName)
.value);
EXPECT_STREQ("InternalNode",
NameTrait<OtherNoName>::GetName(
nullptr, HeapObjectNameForUnnamedObject::kUseHiddenName)
.value);
}
TEST(NameTraitTest, CustomName) {
ClassWithName with_name("CustomName");
const char* name = NameTrait<ClassWithName>::GetName(&with_name).value;
EXPECT_STREQ("CustomName", name);
EXPECT_STREQ(
"CustomName",
NameTrait<ClassWithName>::GetName(
&with_name, HeapObjectNameForUnnamedObject::kUseClassNameIfSupported)
.value);
EXPECT_STREQ("CustomName",
NameTrait<ClassWithName>::GetName(
&with_name, HeapObjectNameForUnnamedObject::kUseHiddenName)
.value);
}
namespace {
@ -99,26 +122,26 @@ class HeapObjectHeaderNameTest : public testing::TestWithHeap {};
TEST_F(HeapObjectHeaderNameTest, LookupNameThroughGCInfo) {
auto* no_name = MakeGarbageCollected<NoName>(GetAllocationHandle());
auto no_name_tuple = HeapObjectHeader::FromObject(no_name).GetName();
if (NameProvider::HideInternalNames()) {
EXPECT_STREQ(NameProvider::kHiddenName, no_name_tuple.value);
EXPECT_TRUE(no_name_tuple.name_was_hidden);
} else {
if (NameProvider::SupportsCppClassNamesAsObjectNames()) {
EXPECT_STREQ("cppgc::internal::(anonymous namespace)::NoName",
no_name_tuple.value);
EXPECT_FALSE(no_name_tuple.name_was_hidden);
} else {
EXPECT_STREQ(NameProvider::kHiddenName, no_name_tuple.value);
EXPECT_TRUE(no_name_tuple.name_was_hidden);
}
auto* other_no_name =
MakeGarbageCollected<OtherNoName>(GetAllocationHandle());
auto other_no_name_tuple =
HeapObjectHeader::FromObject(other_no_name).GetName();
if (NameProvider::HideInternalNames()) {
EXPECT_STREQ(NameProvider::kHiddenName, no_name_tuple.value);
EXPECT_TRUE(no_name_tuple.name_was_hidden);
} else {
if (NameProvider::SupportsCppClassNamesAsObjectNames()) {
EXPECT_STREQ("cppgc::internal::(anonymous namespace)::OtherNoName",
other_no_name_tuple.value);
EXPECT_FALSE(other_no_name_tuple.name_was_hidden);
} else {
EXPECT_STREQ(NameProvider::kHiddenName, no_name_tuple.value);
EXPECT_TRUE(no_name_tuple.name_was_hidden);
}
auto* class_with_name =