Skip to content

Commit

Permalink
Pass the pointer of owning object in constructorWriteBarrierRange (#1511
Browse files Browse the repository at this point in the history
)

Summary:
Pull Request resolved: #1511

Add a `GCCell` pointer parameter to `constructorWriteBarrierRange`,
which represents the owning object address. It's used to get the
correct card table for pointers live in large segment.

Differential Revision: D62171114
  • Loading branch information
lavenzg authored and facebook-github-bot committed Nov 8, 2024
1 parent 2733153 commit ad792f6
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 17 deletions.
10 changes: 10 additions & 0 deletions include/hermes/VM/AlignedHeapSegment.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,16 @@ class AlignedHeapSegmentBase {
return markBits->at(ind);
}

#ifndef NDEBUG
/// Get the storage end of segment that \p cell resides in.
static char *storageEnd(const GCCell *cell) {
auto *start = alignedStorageStart(cell);
auto *segmentInfo = reinterpret_cast<const SHSegmentInfo *>(start);
return start +
(segmentInfo->shiftedSegmentSize << HERMESVM_LOG_HEAP_SEGMENT_SIZE);
}
#endif

protected:
AlignedHeapSegmentBase() = default;

Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/ArrayStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ class ArrayStorageBase final
auto *fromStart = other->data();
auto *fromEnd = fromStart + otherSz;
GCHVType::uninitialized_copy(
fromStart, fromEnd, data() + sz, runtime.getHeap());
fromStart, fromEnd, data() + sz, runtime.getHeap(), this);
size_.store(sz + otherSz, std::memory_order_release);
}

Expand Down
1 change: 1 addition & 0 deletions include/hermes/VM/GCBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,7 @@ class GCBase {
void constructorWriteBarrier(const GCPointerBase *loc, const GCCell *value);
template <typename HVType>
void constructorWriteBarrierRange(
const GCCell *owningObj,
const GCHermesValueBase<HVType> *start,
uint32_t numHVs);
template <typename HVType>
Expand Down
21 changes: 12 additions & 9 deletions include/hermes/VM/HadesGC.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,28 +216,31 @@ class HadesGC final : public GCBase {

template <typename HVType>
void constructorWriteBarrierRange(
const GCCell *owningObj,
const GCHermesValueBase<HVType> *start,
uint32_t numHVs) {
// A pointer that lives in YG never needs any write barriers.
if (LLVM_UNLIKELY(!inYoungGen(start)))
constructorWriteBarrierRangeSlow(start, numHVs);
constructorWriteBarrierRangeSlow(owningObj, start, numHVs);
}
template <typename HVType>
void constructorWriteBarrierRangeSlow(
const GCCell *owningObj,
const GCHermesValueBase<HVType> *start,
uint32_t numHVs) {
assert(
AlignedHeapSegment::containedInSame(start, start + numHVs) &&
reinterpret_cast<const char *>(start + numHVs) <
AlignedHeapSegmentBase::storageEnd(owningObj) &&
"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.
// 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);
AlignedHeapSegmentBase::cardTableCovering(owningObj)
->dirtyCardsForAddressRange(start, start + numHVs);
}

template <typename HVType>
Expand Down
5 changes: 3 additions & 2 deletions include/hermes/VM/HermesValue-inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ inline GCHermesValueBase<HVType> *GCHermesValueBase<HVType>::uninitialized_copy(
GCHermesValueBase<HVType> *first,
GCHermesValueBase<HVType> *last,
GCHermesValueBase<HVType> *result,
GC &gc) {
GC &gc,
const GCCell *owningObj) {
#ifndef NDEBUG
uintptr_t fromFirst = reinterpret_cast<uintptr_t>(first),
fromLast = reinterpret_cast<uintptr_t>(last);
Expand All @@ -194,7 +195,7 @@ inline GCHermesValueBase<HVType> *GCHermesValueBase<HVType>::uninitialized_copy(
"Uninitialized range cannot overlap with an initialized one.");
#endif

gc.constructorWriteBarrierRange(result, last - first);
gc.constructorWriteBarrierRange(owningObj, result, last - first);
// memcpy is fine for an uninitialized copy.
std::memcpy(
reinterpret_cast<void *>(result), first, (last - first) * sizeof(HVType));
Expand Down
3 changes: 2 additions & 1 deletion include/hermes/VM/HermesValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,8 @@ class GCHermesValueBase final : public HVType {
GCHermesValueBase<HVType> *first,
GCHermesValueBase<HVType> *last,
GCHermesValueBase<HVType> *result,
GC &gc);
GC &gc,
const GCCell *owningObj);

/// Copies a range of values and performs a write barrier on each.
template <typename InputIt, typename OutputIt>
Expand Down
1 change: 1 addition & 0 deletions include/hermes/VM/MallocGC.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ class MallocGC final : public GCBase {
void writeBarrierRange(const GCHermesValueBase<HVType> *, uint32_t) {}
template <typename HVType>
void constructorWriteBarrierRange(
const GCCell *,
const GCHermesValueBase<HVType> *,
uint32_t) {}
template <typename HVType>
Expand Down
3 changes: 2 additions & 1 deletion lib/VM/ArrayStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ ExecutionStatus ArrayStorageBase<HVType>::reallocateToLarger(
{
GCHVType *from = self->data() + fromFirst;
GCHVType *to = newSelf->data() + toFirst;
GCHVType::uninitialized_copy(from, from + copySize, to, runtime.getHeap());
GCHVType::uninitialized_copy(
from, from + copySize, to, runtime.getHeap(), newSelf);
}

// Initialize the elements before the first copied element.
Expand Down
14 changes: 12 additions & 2 deletions lib/VM/GCBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,11 @@ bool GCBase::shouldSanitizeHandles() {
runtimeGCDispatch([&](auto *gc) { gc->name(arg1, arg2); }); \
}

#define GCBASE_BARRIER_3(name, type1, type2, type3) \
void GCBase::name(type1 arg1, type2 arg2, type3 arg3) { \
runtimeGCDispatch([&](auto *gc) { gc->name(arg1, arg2, arg3); }); \
}

GCBASE_BARRIER_2(writeBarrier, const GCHermesValue *, HermesValue);
GCBASE_BARRIER_2(writeBarrier, const GCSmallHermesValue *, SmallHermesValue);
GCBASE_BARRIER_2(writeBarrier, const GCPointerBase *, const GCCell *);
Expand All @@ -979,9 +984,14 @@ GCBASE_BARRIER_2(
const GCCell *);
GCBASE_BARRIER_2(writeBarrierRange, const GCHermesValue *, uint32_t);
GCBASE_BARRIER_2(writeBarrierRange, const GCSmallHermesValue *, uint32_t);
GCBASE_BARRIER_2(constructorWriteBarrierRange, const GCHermesValue *, uint32_t);
GCBASE_BARRIER_2(
GCBASE_BARRIER_3(
constructorWriteBarrierRange,
const GCCell *,
const GCHermesValue *,
uint32_t);
GCBASE_BARRIER_3(
constructorWriteBarrierRange,
const GCCell *,
const GCSmallHermesValue *,
uint32_t);
GCBASE_BARRIER_1(snapshotWriteBarrier, const GCHermesValue *);
Expand Down
3 changes: 2 additions & 1 deletion lib/VM/SegmentedArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ ExecutionStatus SegmentedArrayBase<HVType>::growRight(
self->inlineStorage(),
self->inlineStorage() + numSlotsUsed,
newSegmentedArray->inlineStorage(),
runtime.getHeap());
runtime.getHeap(),
newSegmentedArray.get());
// Set the size of the new array to be the same as the old array's size.
newSegmentedArray->numSlotsUsed_.store(
numSlotsUsed, std::memory_order_release);
Expand Down

0 comments on commit ad792f6

Please sign in to comment.