From b3db0cefbdac8b672d6605dccc8c3c6ee62ec0d8 Mon Sep 17 00:00:00 2001 From: "Gang Zhao (Hermes)" Date: Thu, 7 Nov 2024 17:16:48 -0800 Subject: [PATCH] Collapse write barrier function for HV and SHV (#1501) Summary: Pull Request resolved: https://github.com/facebook/hermes/pull/1501 Collapse them to `GCHermesValueBase` since the implementation is exactly the same. This will make future changes to these functions easier. Differential Revision: D62158417 --- include/hermes/VM/GCBase.h | 26 ++++----- include/hermes/VM/HadesGC.h | 101 ++++++++++++++++++----------------- include/hermes/VM/MallocGC.h | 26 ++++----- lib/VM/gcs/HadesGC.cpp | 86 ----------------------------- 4 files changed, 76 insertions(+), 163 deletions(-) diff --git a/include/hermes/VM/GCBase.h b/include/hermes/VM/GCBase.h index c1114809745..4a3c89ecbab 100644 --- a/include/hermes/VM/GCBase.h +++ b/include/hermes/VM/GCBase.h @@ -1152,29 +1152,25 @@ class GCBase { #ifdef HERMESVM_GC_RUNTIME /// Default implementations for read and write barriers: do nothing. - void writeBarrier(const GCHermesValue *loc, HermesValue value); - void writeBarrier(const GCSmallHermesValue *loc, SmallHermesValue value); + template + void writeBarrier(const GCHermesValueBase *loc, HVType value); void writeBarrier(const GCPointerBase *loc, const GCCell *value); - void constructorWriteBarrier(const GCHermesValue *loc, HermesValue value); + template void constructorWriteBarrier( - const GCSmallHermesValue *loc, - SmallHermesValue value); + const GCHermesValueBase *loc, + HVType value); void constructorWriteBarrier(const GCPointerBase *loc, const GCCell *value); - void writeBarrierRange(const GCHermesValue *start, uint32_t numHVs); - void writeBarrierRange(const GCSmallHermesValue *start, uint32_t numHVs); - void constructorWriteBarrierRange( - const GCHermesValue *start, - uint32_t numHVs); + template void constructorWriteBarrierRange( - const GCSmallHermesValue *start, + const GCHermesValueBase *start, uint32_t numHVs); - void snapshotWriteBarrier(const GCHermesValue *loc); - void snapshotWriteBarrier(const GCSmallHermesValue *loc); + template + void snapshotWriteBarrier(const GCHermesValueBase *loc); void snapshotWriteBarrier(const GCPointerBase *loc); void snapshotWriteBarrier(const GCSymbolID *symbol); - void snapshotWriteBarrierRange(const GCHermesValue *start, uint32_t numHVs); + template void snapshotWriteBarrierRange( - const GCSmallHermesValue *start, + const GCHermesValueBase *start, uint32_t numHVs); void weakRefReadBarrier(HermesValue value); void weakRefReadBarrier(GCCell *value); diff --git a/include/hermes/VM/HadesGC.h b/include/hermes/VM/HadesGC.h index 1ff0d7219c8..cb52aff734d 100644 --- a/include/hermes/VM/HadesGC.h +++ b/include/hermes/VM/HadesGC.h @@ -152,7 +152,8 @@ class HadesGC final : public GCBase { /// be in the heap). If value is a pointer, execute a write barrier. /// NOTE: The write barrier call must be placed *before* the write to the /// pointer, so that the current value can be fetched. - void writeBarrier(const GCHermesValue *loc, HermesValue value) { + template + void writeBarrier(const GCHermesValueBase *loc, HVType value) { assert( !calledByBackgroundThread() && "Write barrier invoked by background thread."); @@ -160,17 +161,16 @@ class HadesGC final : public GCBase { if (LLVM_UNLIKELY(!inYoungGen(loc))) writeBarrierSlow(loc, value); } - void writeBarrierSlow(const GCHermesValue *loc, HermesValue value); - - void writeBarrier(const GCSmallHermesValue *loc, SmallHermesValue value) { - assert( - !calledByBackgroundThread() && - "Write barrier invoked by background thread."); - // A pointer that lives in YG never needs any write barriers. - if (LLVM_UNLIKELY(!inYoungGen(loc))) - writeBarrierSlow(loc, value); + template + void writeBarrierSlow(const GCHermesValueBase *loc, HVType value) { + if (ogMarkingBarriers_) { + snapshotWriteBarrierInternal(*loc); + } + if (!value.isPointer()) { + return; + } + relocationWriteBarrier(loc, value.getPointer(getPointerBase())); } - void writeBarrierSlow(const GCSmallHermesValue *loc, SmallHermesValue value); /// The given pointer value is being written at the given loc (required to /// be in the heap). The value may be null. Execute a write barrier. @@ -188,23 +188,25 @@ class HadesGC final : public GCBase { /// Special versions of \p writeBarrier for when there was no previous value /// initialized into the space. - void constructorWriteBarrier(const GCHermesValue *loc, HermesValue value) { - // A pointer that lives in YG never needs any write barriers. - if (LLVM_UNLIKELY(!inYoungGen(loc))) - constructorWriteBarrierSlow(loc, value); - } - void constructorWriteBarrierSlow(const GCHermesValue *loc, HermesValue value); - + template void constructorWriteBarrier( - const GCSmallHermesValue *loc, - SmallHermesValue value) { + const GCHermesValueBase *loc, + HVType value) { // A pointer that lives in YG never needs any write barriers. if (LLVM_UNLIKELY(!inYoungGen(loc))) constructorWriteBarrierSlow(loc, value); } + template void constructorWriteBarrierSlow( - const GCSmallHermesValue *loc, - SmallHermesValue value); + const GCHermesValueBase *loc, + HVType value) { + // A constructor never needs to execute a SATB write barrier, since its + // previous value was definitely not live. + if (!value.isPointer()) { + return; + } + relocationWriteBarrier(loc, value.getPointer(getPointerBase())); + } void constructorWriteBarrier(const GCPointerBase *loc, const GCCell *value) { // A pointer that lives in YG never needs any write barriers. @@ -212,33 +214,34 @@ class HadesGC final : public GCBase { relocationWriteBarrier(loc, value); } + template void constructorWriteBarrierRange( - const GCHermesValue *start, + const GCHermesValueBase *start, uint32_t numHVs) { // A pointer that lives in YG never needs any write barriers. if (LLVM_UNLIKELY(!inYoungGen(start))) constructorWriteBarrierRangeSlow(start, numHVs); } + template void constructorWriteBarrierRangeSlow( - const GCHermesValue *start, - uint32_t numHVs); - - void constructorWriteBarrierRange( - const GCSmallHermesValue *start, + const GCHermesValueBase *start, uint32_t numHVs) { - // A pointer that lives in YG never needs any write barriers. - if (LLVM_UNLIKELY(!inYoungGen(start))) - constructorWriteBarrierRangeSlow(start, numHVs); - } - void constructorWriteBarrierRangeSlow( - const GCSmallHermesValue *start, - uint32_t numHVs); + assert( + AlignedHeapSegment::containedInSame(start, start + numHVs) && + "Range must start and end within a heap segment."); - void snapshotWriteBarrier(const GCHermesValue *loc) { - if (LLVM_UNLIKELY(!inYoungGen(loc) && ogMarkingBarriers_)) - snapshotWriteBarrierInternal(*loc); + // Most constructors should be running in the YG, so in the common case, we + // can avoid doing anything for the whole range. If the range is in the OG, + // then just dirty all the cards corresponding to it, and we can scan them + // for pointers later. This is less precise but makes the write barrier + // faster. + + AlignedHeapSegment::cardTableCovering(start)->dirtyCardsForAddressRange( + start, start + numHVs); } - void snapshotWriteBarrier(const GCSmallHermesValue *loc) { + + template + void snapshotWriteBarrier(const GCHermesValueBase *loc) { if (LLVM_UNLIKELY(!inYoungGen(loc) && ogMarkingBarriers_)) snapshotWriteBarrierInternal(*loc); } @@ -252,23 +255,21 @@ class HadesGC final : public GCBase { snapshotWriteBarrierInternal(*loc); } - void snapshotWriteBarrierRange(const GCHermesValue *start, uint32_t numHVs) { - if (LLVM_UNLIKELY(!inYoungGen(start) && ogMarkingBarriers_)) - snapshotWriteBarrierRangeSlow(start, numHVs); - } - void snapshotWriteBarrierRangeSlow( - const GCHermesValue *start, - uint32_t numHVs); - + template void snapshotWriteBarrierRange( - const GCSmallHermesValue *start, + const GCHermesValueBase *start, uint32_t numHVs) { if (LLVM_UNLIKELY(!inYoungGen(start) && ogMarkingBarriers_)) snapshotWriteBarrierRangeSlow(start, numHVs); } + template void snapshotWriteBarrierRangeSlow( - const GCSmallHermesValue *start, - uint32_t numHVs); + const GCHermesValueBase *start, + uint32_t numHVs) { + for (uint32_t i = 0; i < numHVs; ++i) { + snapshotWriteBarrierInternal(start[i]); + } + } /// Add read barrier for \p value. This is only used when reading entry /// value from WeakMap/WeakSet. diff --git a/include/hermes/VM/MallocGC.h b/include/hermes/VM/MallocGC.h index 98ed5c523a3..b77c51d7615 100644 --- a/include/hermes/VM/MallocGC.h +++ b/include/hermes/VM/MallocGC.h @@ -233,22 +233,24 @@ class MallocGC final : public GCBase { virtual void creditExternalMemory(GCCell *alloc, uint32_t size) override; virtual void debitExternalMemory(GCCell *alloc, uint32_t size) override; - void writeBarrier(const GCHermesValue *, HermesValue) {} - void writeBarrier(const GCSmallHermesValue *, SmallHermesValue) {} + template + void writeBarrier(const GCHermesValueBase *, HVType) {} void writeBarrier(const GCPointerBase *, const GCCell *) {} - void constructorWriteBarrier(const GCHermesValue *, HermesValue) {} - void constructorWriteBarrier(const GCSmallHermesValue *, SmallHermesValue) {} + template + void constructorWriteBarrier(const GCHermesValueBase *, HVType) {} void constructorWriteBarrier(const GCPointerBase *, const GCCell *) {} - void writeBarrierRange(const GCHermesValue *, uint32_t) {} - void writeBarrierRange(const GCSmallHermesValue *, uint32_t) {} - void constructorWriteBarrierRange(const GCHermesValue *, uint32_t) {} - void constructorWriteBarrierRange(const GCSmallHermesValue *, uint32_t) {} - void snapshotWriteBarrier(const GCHermesValue *) {} - void snapshotWriteBarrier(const GCSmallHermesValue *) {} + template + void writeBarrierRange(const GCHermesValueBase *, uint32_t) {} + template + void constructorWriteBarrierRange( + const GCHermesValueBase *, + uint32_t) {} + template + void snapshotWriteBarrier(const GCHermesValueBase *) {} void snapshotWriteBarrier(const GCPointerBase *) {} void snapshotWriteBarrier(const GCSymbolID *) {} - void snapshotWriteBarrierRange(const GCHermesValue *, uint32_t) {} - void snapshotWriteBarrierRange(const GCSmallHermesValue *, uint32_t) {} + template + void snapshotWriteBarrierRange(const GCHermesValueBase *, uint32_t) {} void weakRefReadBarrier(HermesValue) {} void weakRefReadBarrier(GCCell *) {} diff --git a/lib/VM/gcs/HadesGC.cpp b/lib/VM/gcs/HadesGC.cpp index 87dfdfbb6be..96efe56db15 100644 --- a/lib/VM/gcs/HadesGC.cpp +++ b/lib/VM/gcs/HadesGC.cpp @@ -1902,28 +1902,6 @@ void HadesGC::debitExternalMemory(GCCell *cell, uint32_t sz) { } } -void HadesGC::writeBarrierSlow(const GCHermesValue *loc, HermesValue value) { - if (ogMarkingBarriers_) { - snapshotWriteBarrierInternal(*loc); - } - if (!value.isPointer()) { - return; - } - relocationWriteBarrier(loc, value.getPointer()); -} - -void HadesGC::writeBarrierSlow( - const GCSmallHermesValue *loc, - SmallHermesValue value) { - if (ogMarkingBarriers_) { - snapshotWriteBarrierInternal(*loc); - } - if (!value.isPointer()) { - return; - } - relocationWriteBarrier(loc, value.getPointer(getPointerBase())); -} - void HadesGC::writeBarrierSlow(const GCPointerBase *loc, const GCCell *value) { if (*loc && ogMarkingBarriers_) snapshotWriteBarrierInternal(*loc); @@ -1932,70 +1910,6 @@ void HadesGC::writeBarrierSlow(const GCPointerBase *loc, const GCCell *value) { relocationWriteBarrier(loc, value); } -void HadesGC::constructorWriteBarrierSlow( - const GCHermesValue *loc, - HermesValue value) { - // A constructor never needs to execute a SATB write barrier, since its - // previous value was definitely not live. - if (!value.isPointer()) { - return; - } - relocationWriteBarrier(loc, value.getPointer()); -} - -void HadesGC::constructorWriteBarrierSlow( - const GCSmallHermesValue *loc, - SmallHermesValue value) { - // A constructor never needs to execute a SATB write barrier, since its - // previous value was definitely not live. - if (!value.isPointer()) { - return; - } - relocationWriteBarrier(loc, value.getPointer(getPointerBase())); -} - -void HadesGC::constructorWriteBarrierRangeSlow( - const GCHermesValue *start, - uint32_t numHVs) { - assert( - AlignedHeapSegment::containedInSame(start, start + numHVs) && - "Range must start and end within a heap segment."); - - // Most constructors should be running in the YG, so in the common case, we - // can avoid doing anything for the whole range. If the range is in the OG, - // then just dirty all the cards corresponding to it, and we can scan them for - // pointers later. This is less precise but makes the write barrier faster. - - AlignedHeapSegment::cardTableCovering(start)->dirtyCardsForAddressRange( - start, start + numHVs); -} - -void HadesGC::constructorWriteBarrierRangeSlow( - const GCSmallHermesValue *start, - uint32_t numHVs) { - assert( - AlignedHeapSegment::containedInSame(start, start + numHVs) && - "Range must start and end within a heap segment."); - AlignedHeapSegment::cardTableCovering(start)->dirtyCardsForAddressRange( - start, start + numHVs); -} - -void HadesGC::snapshotWriteBarrierRangeSlow( - const GCHermesValue *start, - uint32_t numHVs) { - for (uint32_t i = 0; i < numHVs; ++i) { - snapshotWriteBarrierInternal(start[i]); - } -} - -void HadesGC::snapshotWriteBarrierRangeSlow( - const GCSmallHermesValue *start, - uint32_t numHVs) { - for (uint32_t i = 0; i < numHVs; ++i) { - snapshotWriteBarrierInternal(start[i]); - } -} - void HadesGC::snapshotWriteBarrierInternal(GCCell *oldValue) { assert( (oldValue->isValid()) &&