Commit 0897b1c3 authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

Revert "heap: Remove GetHeapObjectHeader from mixins"

This reverts commit bde0ab78.

Reason for revert: Possibly causing https://bugs.chromium.org/p/chromium/issues/detail?id=1071874

Original change's description:
> heap: Remove GetHeapObjectHeader from mixins
> 
> The methods in the macro are put on every GarbageCollectedMixin which
> is expensive in terms of binary size. The HoH getter can be replaced
> with GetTraceDescriptor().
> 
> Change-Id: I0daaf3fc51a95f71baf7f6c7bed387e94963601f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2139758
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#760049}

TBR=haraken@chromium.org,mlippautz@chromium.org

Bug: chromium:1071874
Change-Id: I7d00fe89223ae32c7a3ad0d5f28f2e056eca6fb8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2154947Reviewed-by: default avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760135}
parent 1dc7053d
...@@ -23,8 +23,8 @@ void ActiveScriptWrappableBase::TraceActiveScriptWrappables( ...@@ -23,8 +23,8 @@ void ActiveScriptWrappableBase::TraceActiveScriptWrappables(
for (const auto& active_wrappable : *active_script_wrappables) { for (const auto& active_wrappable : *active_script_wrappables) {
// Ignore objects that are currently under construction. They are kept alive // Ignore objects that are currently under construction. They are kept alive
// via conservative stack scan. // via conservative stack scan.
HeapObjectHeader const* const header = HeapObjectHeader::FromPayload( HeapObjectHeader const* const header =
active_wrappable->GetTraceDescriptor().base_object_payload); active_wrappable->GetHeapObjectHeader();
if ((header == BlinkGC::kNotFullyConstructedObject) || if ((header == BlinkGC::kNotFullyConstructedObject) ||
header->IsInConstruction()) header->IsInConstruction())
continue; continue;
......
...@@ -15,6 +15,7 @@ namespace blink { ...@@ -15,6 +15,7 @@ namespace blink {
template <typename T> template <typename T>
class GarbageCollected; class GarbageCollected;
class HeapObjectHeader;
// GC_PLUGIN_IGNORE is used to make the plugin ignore a particular class or // GC_PLUGIN_IGNORE is used to make the plugin ignore a particular class or
// field when checking for proper usage. When using GC_PLUGIN_IGNORE // field when checking for proper usage. When using GC_PLUGIN_IGNORE
...@@ -106,6 +107,10 @@ class PLATFORM_EXPORT GarbageCollectedMixin { ...@@ -106,6 +107,10 @@ class PLATFORM_EXPORT GarbageCollectedMixin {
// scenarios, e.g., initializing stores. As a result, we cannot depend on the // scenarios, e.g., initializing stores. As a result, we cannot depend on the
// write barriers for catching writes to member fields and thus have to // write barriers for catching writes to member fields and thus have to
// process the object (instead of just marking only the header). // process the object (instead of just marking only the header).
virtual HeapObjectHeader* GetHeapObjectHeader() const {
return reinterpret_cast<HeapObjectHeader*>(
BlinkGC::kNotFullyConstructedObject);
}
virtual TraceDescriptor GetTraceDescriptor() const { virtual TraceDescriptor GetTraceDescriptor() const {
return {BlinkGC::kNotFullyConstructedObject, nullptr}; return {BlinkGC::kNotFullyConstructedObject, nullptr};
} }
...@@ -113,11 +118,15 @@ class PLATFORM_EXPORT GarbageCollectedMixin { ...@@ -113,11 +118,15 @@ class PLATFORM_EXPORT GarbageCollectedMixin {
#define DEFINE_GARBAGE_COLLECTED_MIXIN_METHODS(TYPE) \ #define DEFINE_GARBAGE_COLLECTED_MIXIN_METHODS(TYPE) \
public: \ public: \
TraceDescriptor GetTraceDescriptor() const override { \ HeapObjectHeader* GetHeapObjectHeader() const override { \
static_assert( \ static_assert( \
WTF::IsSubclassOfTemplate<typename std::remove_const<TYPE>::type, \ WTF::IsSubclassOfTemplate<typename std::remove_const<TYPE>::type, \
blink::GarbageCollected>::value, \ blink::GarbageCollected>::value, \
"only garbage collected objects can have garbage collected mixins"); \ "only garbage collected objects can have garbage collected mixins"); \
return HeapObjectHeader::FromPayload(static_cast<const TYPE*>(this)); \
} \
\
TraceDescriptor GetTraceDescriptor() const override { \
return {const_cast<TYPE*>(static_cast<const TYPE*>(this)), \ return {const_cast<TYPE*>(static_cast<const TYPE*>(this)), \
TraceTrait<TYPE>::Trace}; \ TraceTrait<TYPE>::Trace}; \
} \ } \
...@@ -157,6 +166,10 @@ class PLATFORM_EXPORT GarbageCollectedMixin { ...@@ -157,6 +166,10 @@ class PLATFORM_EXPORT GarbageCollectedMixin {
// }; // };
#define MERGE_GARBAGE_COLLECTED_MIXINS() \ #define MERGE_GARBAGE_COLLECTED_MIXINS() \
public: \ public: \
HeapObjectHeader* GetHeapObjectHeader() const override { \
return reinterpret_cast<HeapObjectHeader*>( \
BlinkGC::kNotFullyConstructedObject); \
} \
TraceDescriptor GetTraceDescriptor() const override { \ TraceDescriptor GetTraceDescriptor() const override { \
return {BlinkGC::kNotFullyConstructedObject, nullptr}; \ return {BlinkGC::kNotFullyConstructedObject, nullptr}; \
} \ } \
......
...@@ -173,7 +173,7 @@ class ObjectAliveTrait<T, true> { ...@@ -173,7 +173,7 @@ class ObjectAliveTrait<T, true> {
NO_SANITIZE_ADDRESS NO_SANITIZE_ADDRESS
static bool IsHeapObjectAlive(const T* object) { static bool IsHeapObjectAlive(const T* object) {
static_assert(sizeof(T), "T must be fully defined"); static_assert(sizeof(T), "T must be fully defined");
const HeapObjectHeader* header = TraceTrait<T>::GetHeapObjectHeader(object); const HeapObjectHeader* header = object->GetHeapObjectHeader();
if (header == BlinkGC::kNotFullyConstructedObject) { if (header == BlinkGC::kNotFullyConstructedObject) {
// Objects under construction are always alive. // Objects under construction are always alive.
return true; return true;
......
...@@ -1373,8 +1373,7 @@ using ObjectRegistry = HeapHashMap<void*, Member<RegisteringMixin>>; ...@@ -1373,8 +1373,7 @@ using ObjectRegistry = HeapHashMap<void*, Member<RegisteringMixin>>;
class RegisteringMixin : public GarbageCollectedMixin { class RegisteringMixin : public GarbageCollectedMixin {
public: public:
explicit RegisteringMixin(ObjectRegistry* registry) { explicit RegisteringMixin(ObjectRegistry* registry) {
HeapObjectHeader* header = HeapObjectHeader* header = GetHeapObjectHeader();
TraceTrait<RegisteringMixin>::GetHeapObjectHeader(this);
const void* uninitialized_value = BlinkGC::kNotFullyConstructedObject; const void* uninitialized_value = BlinkGC::kNotFullyConstructedObject;
EXPECT_EQ(uninitialized_value, header); EXPECT_EQ(uninitialized_value, header);
registry->insert(reinterpret_cast<void*>(this), this); registry->insert(reinterpret_cast<void*>(this), this);
...@@ -1438,8 +1437,7 @@ TEST_F(IncrementalMarkingTest, WriteBarrierDuringMixinConstruction) { ...@@ -1438,8 +1437,7 @@ TEST_F(IncrementalMarkingTest, WriteBarrierDuringMixinConstruction) {
TEST_F(IncrementalMarkingTest, OverrideAfterMixinConstruction) { TEST_F(IncrementalMarkingTest, OverrideAfterMixinConstruction) {
ObjectRegistry registry; ObjectRegistry registry;
RegisteringMixin* mixin = MakeGarbageCollected<RegisteringObject>(&registry); RegisteringMixin* mixin = MakeGarbageCollected<RegisteringObject>(&registry);
HeapObjectHeader* header = HeapObjectHeader* header = mixin->GetHeapObjectHeader();
TraceTrait<RegisteringMixin>::GetHeapObjectHeader(mixin);
const void* uninitialized_value = BlinkGC::kNotFullyConstructedObject; const void* uninitialized_value = BlinkGC::kNotFullyConstructedObject;
EXPECT_NE(uninitialized_value, header); EXPECT_NE(uninitialized_value, header);
} }
......
...@@ -9,8 +9,6 @@ ...@@ -9,8 +9,6 @@
namespace blink { namespace blink {
class HeapObjectHeader;
// Marking verifier that checks that a child is marked if its parent is marked. // Marking verifier that checks that a child is marked if its parent is marked.
class MarkingVerifier final : public Visitor { class MarkingVerifier final : public Visitor {
public: public:
......
...@@ -55,9 +55,7 @@ struct AdjustPointerTrait<T, true> { ...@@ -55,9 +55,7 @@ struct AdjustPointerTrait<T, true> {
} }
static HeapObjectHeader* GetHeapObjectHeader(const void* self) { static HeapObjectHeader* GetHeapObjectHeader(const void* self) {
const void* payload = return static_cast<const T*>(self)->GetHeapObjectHeader();
static_cast<const T*>(self)->GetTraceDescriptor().base_object_payload;
return payload ? HeapObjectHeader::FromPayload(payload) : nullptr;
} }
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment