Skip to content

Commit

Permalink
Add owning cell to parameters of GCHermesValueBase::set()
Browse files Browse the repository at this point in the history
Differential Revision: D62196480
  • Loading branch information
lavenzg authored and facebook-github-bot committed Sep 5, 2024
1 parent 1d373b0 commit 8b9fb55
Show file tree
Hide file tree
Showing 12 changed files with 73 additions and 43 deletions.
2 changes: 1 addition & 1 deletion include/hermes/VM/ArrayStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class ArrayStorageBase final
template <Inline inl = Inline::No>
void set(size_type index, HVType val, GC &gc) {
assert(index < size() && "index out of range");
data()[index].set(val, gc);
data()[index].set(val, gc, this);
}

/// \return the element at index \p index
Expand Down
23 changes: 15 additions & 8 deletions include/hermes/VM/HermesValue-inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,15 @@ GCHermesValueBase<HVType>::GCHermesValueBase(HVType hv, GC &gc, std::nullptr_t)

template <typename HVType>
template <typename NeedsBarriers>
inline void GCHermesValueBase<HVType>::set(HVType hv, GC &gc) {
inline void
GCHermesValueBase<HVType>::set(HVType hv, GC &gc, const GCCell *cell) {
if (hv.isPointer()) {
HERMES_SLOW_ASSERT(
gc.validPointer(hv.getPointer(gc.getPointerBase())) &&
"Setting an invalid pointer into a GCHermesValue");
}
assert(NeedsBarriers::value || !gc.needsWriteBarrier(this, hv));
(void)cell;
if (NeedsBarriers::value)
gc.writeBarrier(this, hv);
HVType::setNoBarrier(hv);
Expand All @@ -81,10 +83,11 @@ inline void GCHermesValueBase<HVType>::fill(
InputIt start,
InputIt end,
HVType fill,
GC &gc) {
GC &gc,
const GCCell *cell) {
if (fill.isPointer()) {
for (auto cur = start; cur != end; ++cur) {
cur->set(fill, gc);
cur->set(fill, gc, cell);
}
} else {
for (auto cur = start; cur != end; ++cur) {
Expand Down Expand Up @@ -120,15 +123,16 @@ inline OutputIt GCHermesValueBase<HVType>::copy(
InputIt first,
InputIt last,
OutputIt result,
GC &gc) {
GC &gc,
const GCCell *cell) {
#if !defined(HERMESVM_GC_HADES) && !defined(HERMESVM_GC_RUNTIME)
static_assert(
!std::is_same<InputIt, GCHermesValueBase *>::value ||
!std::is_same<OutputIt, GCHermesValueBase *>::value,
"Pointer arguments must invoke pointer overload.");
#endif
for (; first != last; ++first, (void)++result) {
result->set(*first, gc);
result->set(*first, gc, cell);
}
return result;
}
Expand Down Expand Up @@ -160,13 +164,15 @@ inline GCHermesValueBase<HVType> *GCHermesValueBase<HVType>::copy(
GCHermesValueBase<HVType> *first,
GCHermesValueBase<HVType> *last,
GCHermesValueBase<HVType> *result,
GC &gc) {
GC &gc,
const GCCell *cell) {
// We must use "raw" function such as memmove here, rather than a
// function like std::copy (or copy_backward) that respects
// constructors and operator=. For HermesValue, those require the
// contents not to contain pointers. The range write barrier
// before the copies ensure that sufficient barriers are
// performed.
(void)cell;
gc.writeBarrierRange(result, last - first);
std::memmove(
reinterpret_cast<void *>(result),
Expand Down Expand Up @@ -208,9 +214,10 @@ inline OutputIt GCHermesValueBase<HVType>::copy_backward(
InputIt first,
InputIt last,
OutputIt result,
GC &gc) {
GC &gc,
const GCCell *cell) {
while (first != last) {
(--result)->set(*--last, gc);
(--result)->set(*--last, gc, cell);
}
return result;
}
Expand Down
29 changes: 20 additions & 9 deletions include/hermes/VM/HermesValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -531,10 +531,11 @@ class GCHermesValueBase final : public HVType {
GCHermesValueBase(HVType hv, GC &gc, std::nullptr_t);
GCHermesValueBase(const HVType &) = delete;

/// The HermesValue \p hv may be an object pointer. Assign the
/// value, and perform any necessary write barriers.
/// The HermesValue \p hv may be an object pointer. Assign the value, and
/// perform any necessary write barriers. \p cell is the object that contains
/// this GCHermesValueBase. It's needed by the write barrier.
template <typename NeedsBarriers = std::true_type>
inline void set(HVType hv, GC &gc);
inline void set(HVType hv, GC &gc, const GCCell *cell);

/// The HermesValue \p hv must not be an object pointer. Assign the
/// value.
Expand All @@ -552,7 +553,8 @@ class GCHermesValueBase final : public HVType {
/// value \p fill. If the fill value is an object pointer, must
/// provide a non-null \p gc argument, to perform write barriers.
template <typename InputIt>
static inline void fill(InputIt first, InputIt last, HVType fill, GC &gc);
static inline void
fill(InputIt first, InputIt last, HVType fill, GC &gc, const GCCell *cell);

/// Same as \p fill except the range expressed by [\p first, \p last) has not
/// been previously initialized. Cannot use this on previously initialized
Expand All @@ -563,8 +565,12 @@ class GCHermesValueBase final : public HVType {

/// Copies a range of values and performs a write barrier on each.
template <typename InputIt, typename OutputIt>
static inline OutputIt
copy(InputIt first, InputIt last, OutputIt result, GC &gc);
static inline OutputIt copy(
InputIt first,
InputIt last,
OutputIt result,
GC &gc,
const GCCell *cell);

/// Same as \p copy, but the range [result, result + (last - first)) has not
/// been previously initialized. Cannot use this on previously initialized
Expand All @@ -579,7 +585,8 @@ class GCHermesValueBase final : public HVType {
GCHermesValueBase<HVType> *first,
GCHermesValueBase<HVType> *last,
GCHermesValueBase<HVType> *result,
GC &gc);
GC &gc,
const GCCell *cell);
#endif

/// Same as \p uninitialized_copy, but specialized for raw pointers. This is
Expand All @@ -595,8 +602,12 @@ class GCHermesValueBase final : public HVType {

/// Copies a range of values and performs a write barrier on each.
template <typename InputIt, typename OutputIt>
static inline OutputIt
copy_backward(InputIt first, InputIt last, OutputIt result, GC &gc);
static inline OutputIt copy_backward(
InputIt first,
InputIt last,
OutputIt result,
GC &gc,
const GCCell *cell);

/// Same as \c unreachableWriteBarrier, but for a range of values all becoming
/// unreachable.
Expand Down
4 changes: 2 additions & 2 deletions include/hermes/VM/JSObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -1736,7 +1736,7 @@ template <SlotIndex index>
inline void
JSObject::setDirectSlotValue(JSObject *self, SmallHermesValue value, GC &gc) {
static_assert(index < DIRECT_PROPERTY_SLOTS, "Must be a direct property");
self->directProps()[index].set(value, gc);
self->directProps()[index].set(value, gc, self);
}

inline SmallHermesValue JSObject::getNamedSlotValueUnsafe(
Expand Down Expand Up @@ -1839,7 +1839,7 @@ inline void JSObject::setNamedSlotValueDirectUnsafe(
// to namedSlotRef(), it is a slight performance regression, which is not
// entirely unexpected.
return self->directProps()[index].set<NeedsBarriers>(
value, runtime.getHeap());
value, runtime.getHeap(), self);
}

inline void JSObject::setNamedSlotValueIndirectUnsafe(
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/SegmentedArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ class SegmentedArrayBase final : public VariableSizeRuntimeCell,
/// Sets the element located at \p index to \p val.
template <Inline inl = Inline::No>
void set(Runtime &runtime, TotalIndex index, HVType val) {
atRef<inl>(runtime, index).set(val, runtime.getHeap());
atRef<inl>(runtime, index).set(val, runtime.getHeap(), this);
}
template <Inline inl = Inline::No>
void setNonPtr(Runtime &runtime, TotalIndex index, HVType val) {
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/StringPrimitive.h
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ class BufferedStringPrimitive final : public StringPrimitive {
Handle<ExternalStringPrimitive<T>> concatBuffer)
: StringPrimitive(length) {
concatBufferHV_.set(
HermesValue::encodeObjectValue(*concatBuffer), runtime.getHeap());
HermesValue::encodeObjectValue(*concatBuffer), runtime.getHeap(), this);
assert(
concatBuffer->contents_.size() >= length &&
"length exceeds size of concatenation buffer");
Expand Down
9 changes: 6 additions & 3 deletions lib/VM/ArrayStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,23 +185,26 @@ ExecutionStatus ArrayStorageBase<HVType>::shift(
self->data() + fromFirst,
self->data() + fromFirst + copySize,
self->data() + toFirst,
runtime.getHeap());
runtime.getHeap(),
self);
} else if (fromFirst < toFirst) {
// Copying to the right, need to copy backwards to avoid overwriting what
// is being copied.
GCHVType::copy_backward(
self->data() + fromFirst,
self->data() + fromFirst + copySize,
self->data() + toFirst + copySize,
runtime.getHeap());
runtime.getHeap(),
self);
}

// Initialize the elements which were emptied in front.
GCHVType::fill(
self->data(),
self->data() + toFirst,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
self);

// Initialize the elements between the last copied element and toLast.
if (toFirst + copySize < toLast) {
Expand Down
12 changes: 6 additions & 6 deletions lib/VM/Interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2077,16 +2077,16 @@ CallResult<HermesValue> Interpreter::interpretFunction(
}

CASE(StoreToEnvironment) {
vmcast<Environment>(O1REG(StoreToEnvironment))
->slot(ip->iStoreToEnvironment.op2)
.set(O3REG(StoreToEnvironment), runtime.getHeap());
auto *environment = vmcast<Environment>(O1REG(StoreToEnvironment));
environment->slot(ip->iStoreToEnvironment.op2)
.set(O3REG(StoreToEnvironment), runtime.getHeap(), environment);
ip = NEXTINST(StoreToEnvironment);
DISPATCH;
}
CASE(StoreToEnvironmentL) {
vmcast<Environment>(O1REG(StoreToEnvironmentL))
->slot(ip->iStoreToEnvironmentL.op2)
.set(O3REG(StoreToEnvironmentL), runtime.getHeap());
auto *environment = vmcast<Environment>(O1REG(StoreToEnvironmentL));
environment->slot(ip->iStoreToEnvironmentL.op2)
.set(O3REG(StoreToEnvironmentL), runtime.getHeap(), environment);
ip = NEXTINST(StoreToEnvironmentL);
DISPATCH;
}
Expand Down
3 changes: 2 additions & 1 deletion lib/VM/JSObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ void JSObject::allocateNewSlotStorage(
// If it is a direct property, just store the value and we are done.
if (LLVM_LIKELY(newSlotIndex < DIRECT_PROPERTY_SLOTS)) {
auto shv = SmallHermesValue::encodeHermesValue(*valueHandle, runtime);
selfHandle->directProps()[newSlotIndex].set(shv, runtime.getHeap());
selfHandle->directProps()[newSlotIndex].set(
shv, runtime.getHeap(), *selfHandle);
return;
}

Expand Down
6 changes: 3 additions & 3 deletions lib/VM/OrderedHashMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ ExecutionStatus OrderedHashMapBase<BucketType, Derived>::insert(
self->lookupInBucket(runtime, bucket, key.getHermesValue());
if (entry) {
// Element for the key already exists, update value and return.
entry->value.set(shv, runtime.getHeap());
entry->value.set(shv, runtime.getHeap(), entry);
return ExecutionStatus::RETURNED;
}
}
Expand Down Expand Up @@ -337,11 +337,11 @@ ExecutionStatus OrderedHashMapBase<BucketType, Derived>::doInsert(
// call it and set to newMapEntry one at a time.
auto newMapEntry = runtime.makeHandle(std::move(*crtRes));
auto k = SmallHermesValue::encodeHermesValue(key.getHermesValue(), runtime);
newMapEntry->key.set(k, runtime.getHeap());
newMapEntry->key.set(k, runtime.getHeap(), *newMapEntry);
if constexpr (std::is_same_v<BucketType, HashMapEntry>) {
auto v =
SmallHermesValue::encodeHermesValue(value.getHermesValue(), runtime);
newMapEntry->value.set(v, runtime.getHeap());
newMapEntry->value.set(v, runtime.getHeap(), *newMapEntry);
}

// After here, no allocation
Expand Down
17 changes: 12 additions & 5 deletions lib/VM/SegmentedArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ void SegmentedArrayBase<HVType>::allocateSegment(
"Allocating into a non-empty segment");
PseudoHandle<Segment> c = Segment::create(runtime);
self->segmentAtPossiblyUnallocated(segment)->set(
HVType::encodeObjectValue(c.get(), runtime), runtime.getHeap());
HVType::encodeObjectValue(c.get(), runtime), runtime.getHeap(), c.get());
}

template <typename HVType>
Expand Down Expand Up @@ -327,7 +327,8 @@ ExecutionStatus SegmentedArrayBase<HVType>::growLeft(
self->begin(runtime),
self->end(runtime),
newSegmentedArray->begin(runtime) + amount,
runtime.getHeap());
runtime.getHeap(),
*self);
// Assign back to self.
self = newSegmentedArray.get();
return ExecutionStatus::RETURNED;
Expand All @@ -348,13 +349,15 @@ void SegmentedArrayBase<HVType>::growLeftWithinCapacity(
self->begin(runtime),
self->end(runtime) - amount,
self->end(runtime),
runtime.getHeap());
runtime.getHeap(),
self.get());
// Fill the beginning with empty values.
GCHVType::fill(
self->begin(runtime),
self->begin(runtime) + amount,
HVType::encodeEmptyValue(),
runtime.getHeap());
runtime.getHeap(),
self.get());
}

template <typename HVType>
Expand All @@ -370,7 +373,11 @@ void SegmentedArrayBase<HVType>::shrinkLeft(
size_type amount) {
// Copy the end values leftwards to the beginning.
GCHVType::copy(
begin(runtime) + amount, end(runtime), begin(runtime), runtime.getHeap());
begin(runtime) + amount,
end(runtime),
begin(runtime),
runtime.getHeap(),
this);
// Now that all the values are moved down, fill the end with empty values.
decreaseSize(runtime, amount);
}
Expand Down
7 changes: 4 additions & 3 deletions lib/VM/StaticH.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,10 @@ extern "C" void _sh_ljs_store_to_env(
SHLegacyValue env,
SHLegacyValue val,
uint32_t index) {
vmcast<Environment>(HermesValue::fromRaw(env.raw))
->slot(index)
.set(HermesValue::fromRaw(val.raw), getRuntime(shr).getHeap());
auto *environment = vmcast<Environment>(HermesValue::fromRaw(env.raw));

environment->slot(index).set(
HermesValue::fromRaw(val.raw), getRuntime(shr).getHeap(), environment);
}

extern "C" void _sh_ljs_store_np_to_env(
Expand Down

0 comments on commit 8b9fb55

Please sign in to comment.