Skip to content

Commit

Permalink
Pass the pointer of owning object in ctor of GCHermesValueBase (faceb…
Browse files Browse the repository at this point in the history
…ook#1508)

Summary: Pull Request resolved: facebook#1508

Differential Revision: D62222238
  • Loading branch information
lavenzg authored and facebook-github-bot committed Oct 21, 2024
1 parent 8c37d41 commit b2894ee
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 25 deletions.
2 changes: 1 addition & 1 deletion include/hermes/VM/ArrayStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class ArrayStorageBase final
assert(sz < capacity());
// Use the constructor of GCHermesValue to use the correct write barrier
// for uninitialized memory.
new (&data()[sz]) GCHVType(value, runtime.getHeap());
new (&data()[sz]) GCHVType(value, runtime.getHeap(), this);
size_.store(sz + 1, std::memory_order_release);
}

Expand Down
6 changes: 4 additions & 2 deletions include/hermes/VM/Callable.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ class Environment final
getSlots(),
getSlots() + size,
HermesValue::encodeUndefinedValue(),
runtime.getHeap());
runtime.getHeap(),
this);
}

/// Create an environment using the given function to retrieve the parent
Expand Down Expand Up @@ -369,7 +370,8 @@ Environment::Environment(
getSlots(),
getSlots() + size,
HermesValue::encodeUndefinedValue(),
runtime.getHeap());
runtime.getHeap(),
this);
}

/// A function produced by Function.prototype.bind(). It packages a function
Expand Down
17 changes: 12 additions & 5 deletions include/hermes/VM/HermesValue-inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,13 @@ inline PinnedHermesValue &PinnedHermesValue::operator=(PseudoHandle<T> &&hv) {

template <typename HVType>
template <typename NeedsBarriers>
GCHermesValueBase<HVType>::GCHermesValueBase(HVType hv, GC &gc) : HVType{hv} {
GCHermesValueBase<HVType>::GCHermesValueBase(
HVType hv,
GC &gc,
const GCCell *cell)
: HVType{hv} {
assert(!hv.isPointer() || hv.getPointer());
(void)cell;
if (NeedsBarriers::value)
gc.constructorWriteBarrier(this, hv);
}
Expand Down Expand Up @@ -103,11 +108,12 @@ inline void GCHermesValueBase<HVType>::uninitialized_fill(
InputIt start,
InputIt end,
HVType fill,
GC &gc) {
GC &gc,
const GCCell *cell) {
if (fill.isPointer()) {
for (auto cur = start; cur != end; ++cur) {
// Use the constructor write barrier. Assume it needs barriers.
new (&*cur) GCHermesValueBase<HVType>(fill, gc);
new (&*cur) GCHermesValueBase<HVType>(fill, gc, cell);
}
} else {
for (auto cur = start; cur != end; ++cur) {
Expand Down Expand Up @@ -141,13 +147,14 @@ inline OutputIt GCHermesValueBase<HVType>::uninitialized_copy(
InputIt first,
InputIt last,
OutputIt result,
GC &gc) {
GC &gc,
const GCCell *cell) {
static_assert(
!std::is_same<InputIt, GCHermesValueBase *>::value ||
!std::is_same<OutputIt, GCHermesValueBase *>::value,
"Pointer arguments must invoke pointer overload.");
for (; first != last; ++first, (void)++result) {
new (&*result) GCHermesValueBase<HVType>(*first, gc);
new (&*result) GCHermesValueBase<HVType>(*first, gc, cell);
}
return result;
}
Expand Down
18 changes: 13 additions & 5 deletions include/hermes/VM/HermesValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ class GCHermesValueBase final : public HVType {
GCHermesValueBase() : HVType(HVType::encodeUndefinedValue()) {}
/// Initialize a GCHermesValue from another HV. Performs a write barrier.
template <typename NeedsBarriers = std::true_type>
GCHermesValueBase(HVType hv, GC &gc);
GCHermesValueBase(HVType hv, GC &gc, const GCCell *cell);
/// Initialize a GCHermesValue from a non-pointer HV. Might perform a write
/// barrier, depending on the GC.
/// NOTE: The last parameter is unused, but acts as an overload selector.
Expand Down Expand Up @@ -564,8 +564,12 @@ class GCHermesValueBase final : public HVType {
/// been previously initialized. Cannot use this on previously initialized
/// memory, as it will use an incorrect write barrier.
template <typename InputIt>
static inline void
uninitialized_fill(InputIt first, InputIt last, HVType fill, GC &gc);
static inline void uninitialized_fill(
InputIt first,
InputIt last,
HVType fill,
GC &gc,
const GCCell *cell);

/// Copies a range of values and performs a write barrier on each.
template <typename InputIt, typename OutputIt>
Expand All @@ -576,8 +580,12 @@ class GCHermesValueBase final : public HVType {
/// been previously initialized. Cannot use this on previously initialized
/// memory, as it will use an incorrect write barrier.
template <typename InputIt, typename OutputIt>
static inline OutputIt
uninitialized_copy(InputIt first, InputIt last, OutputIt result, GC &gc);
static inline OutputIt uninitialized_copy(
InputIt first,
InputIt last,
OutputIt result,
GC &gc,
const GCCell *cell);

/// Same as \p copy, but specialized for raw pointers.
static inline GCHermesValueBase<HVType> *copy(
Expand Down
3 changes: 2 additions & 1 deletion include/hermes/VM/JSObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -1676,7 +1676,8 @@ inline T *JSObject::initDirectPropStorage(Runtime &runtime, T *self) {
self->directProps() + numOverlapSlots<T>(),
self->directProps() + DIRECT_PROPERTY_SLOTS,
SmallHermesValue::encodeUndefinedValue(),
runtime.getHeap());
runtime.getHeap(),
self);
return self;
}

Expand Down
12 changes: 8 additions & 4 deletions lib/VM/ArrayStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,17 @@ ExecutionStatus ArrayStorageBase<HVType>::reallocateToLarger(
newSelf->data(),
newSelf->data() + toFirst,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
newSelf);

// Initialize the elements after the last copied element and toLast.
if (toFirst + copySize < toLast) {
GCHVType::uninitialized_fill(
newSelf->data() + toFirst + copySize,
newSelf->data() + toLast,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
newSelf);
}

newSelf->size_.store(toLast, std::memory_order_release);
Expand Down Expand Up @@ -152,7 +154,8 @@ void ArrayStorageBase<HVType>::resizeWithinCapacity(
self->data() + sz,
self->data() + newSize,
HVType::encodeEmptyValue(),
gc);
gc,
self);
} else if (newSize < sz) {
// Execute write barriers on elements about to be conceptually changed to
// null.
Expand Down Expand Up @@ -212,7 +215,8 @@ ExecutionStatus ArrayStorageBase<HVType>::shift(
self->data() + toFirst + copySize,
self->data() + toLast,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
self);
}
if (toLast < self->size()) {
// Some elements are becoming unreachable, let the GC know.
Expand Down
27 changes: 20 additions & 7 deletions lib/VM/SegmentedArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ void SegmentedArrayBase<HVType>::Segment::setLength(
data_ + len,
data_ + newLength,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
this);
length_.store(newLength, std::memory_order_release);
} else if (newLength < len) {
// If length is decreasing a write barrier needs to be done.
Expand Down Expand Up @@ -193,8 +194,16 @@ ExecutionStatus SegmentedArrayBase<HVType>::push_back(
return ExecutionStatus::EXCEPTION;
}
const auto shv = HVType::encodeHermesValue(*value, runtime);
auto &elm = self->atRef(runtime, oldSize);
new (&elm) GCHVType(shv, runtime.getHeap());
if (oldSize < kValueToSegmentThreshold) {
auto &elm = self->inlineStorage()[oldSize];
new (&elm) GCHVType(shv, runtime.getHeap(), *self);
} else {
auto segmentNumber = toSegment(oldSize);
auto *segment = self->segmentAt(runtime, segmentNumber);
auto &elm = segment->at(toInterior(oldSize));
// elm lives in segment, which is not the same cell as SegmentedArrayBase.
new (&elm) GCHVType(shv, runtime.getHeap(), segment);
}
return ExecutionStatus::RETURNED;
}

Expand Down Expand Up @@ -395,7 +404,8 @@ void SegmentedArrayBase<HVType>::increaseSizeWithinCapacity(
inlineStorage() + currSize,
inlineStorage() + finalSize,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
this);
// Set the final size.
numSlotsUsed_.store(finalSize, std::memory_order_release);
return;
Expand All @@ -410,7 +420,8 @@ void SegmentedArrayBase<HVType>::increaseSizeWithinCapacity(
inlineStorage() + currSize,
inlineStorage() + kValueToSegmentThreshold,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
this);
}
segmentAt(runtime, segment)->setLength(runtime, segmentLength);
}
Expand Down Expand Up @@ -443,7 +454,8 @@ SegmentedArrayBase<HVType>::increaseSize(
self->inlineStorage() + currSize,
self->inlineStorage() + kValueToSegmentThreshold,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
self.get());
// Set the size to the inline storage threshold.
self->numSlotsUsed_.store(
kValueToSegmentThreshold, std::memory_order_release);
Expand All @@ -469,7 +481,8 @@ SegmentedArrayBase<HVType>::increaseSize(
self->numSlotsUsed_.load(std::memory_order_relaxed),
self->inlineStorage() + newNumSlotsUsed,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
self.get());
self->numSlotsUsed_.store(newNumSlotsUsed, std::memory_order_release);

// Allocate a handle to track the current array.
Expand Down

0 comments on commit b2894ee

Please sign in to comment.