Skip to content

Commit

Permalink
Cache the object's HC for use in GetNextPName
Browse files Browse the repository at this point in the history 
Summary:
The first step of speeding up `for-in` iteration over objects
is to avoid doing unnecessary membership checks every iteration.
This is done by also caching the object's own HiddenClass
(we're already storing the rest of the prototype chain)
along with the number of properties it has.

Then on each iteration we can do some simple math and check that the
HiddenClass hasn't changed instead of running `findProperty`.

Reviewed By: tmikov

Differential Revision: D67292849

fbshipit-source-id: a1807d0d1bb5ef1a6bf318bb32e8088ab2bc578b
  • Loading branch information
@avp @facebook-github-bot
avp authored and facebook-github-bot committed Jan 31, 2025
1 parent 45203e1 commit 1a5a235
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 6 deletions.
21 changes: 21 additions & 0 deletions lib/VM/Interpreter-slowpaths.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ ExecutionStatus Interpreter::caseGetNextPName(
PinnedValue<> tmp;
PinnedValue<JSObject> propObj;
PinnedValue<SymbolID> tmpPropNameStorage;
PinnedValue<HiddenClass> cachedClass;
} lv;
LocalsRAII lraii{runtime, &lv};

Expand All @@ -371,11 +372,31 @@ ExecutionStatus Interpreter::caseGetNextPName(
auto arr = Handle<BigStorage>::vmcast(&O2REG(GetNextPName));
uint32_t idx = O4REG(GetNextPName).getNumber();
uint32_t size = O5REG(GetNextPName).getNumber();

// If there's a class at index 2, it means we have a cached class.
uint32_t startIdx = 0;
uint32_t numObjProps = 0;
if (LLVM_LIKELY(size > 2)) {
lv.cachedClass = dyn_vmcast<HiddenClass>(arr->at(runtime, 2));
if (lv.cachedClass.get()) {
startIdx = arr->at(runtime, 0).getNumberAs<uint32_t>();
numObjProps = arr->at(runtime, 1).getNumberAs<uint32_t>();
}
}

MutableHandle<JSObject> propObj{lv.propObj};
MutableHandle<SymbolID> tmpPropNameStorage{lv.tmpPropNameStorage};
// Loop until we find a property which is present.
while (LLVM_LIKELY(idx < size)) {
lv.tmp = arr->at(runtime, idx);
// If there's no caching, lv.cachedClass is nullptr and the comparison will
// fail.
if (LLVM_LIKELY(size > 0) && idx - startIdx < numObjProps &&
LLVM_LIKELY(lv.cachedClass.get() == obj->getClass(runtime))) {
// Cached.
propObj = obj;
break;
}
if (lv.tmp->isSymbol()) {
// NOTE: This call is safe because we immediately discard desc,
// so it can't outlive the SymbolID.
Expand Down
52 changes: 46 additions & 6 deletions lib/VM/JSObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3080,6 +3080,8 @@ namespace {
/// All string keys are added as Symbols, to aid lookup in GetNextPName.
/// All number keys are added as numbers.
/// Only enumerable properties are included.
/// Write the number of object properties for the obj to index 1 of \p arr
/// (does not include proto properties).
/// Returns the index after the last property added.
CallResult<uint32_t> appendAllPropertyKeys(
Handle<JSObject> obj,
Expand Down Expand Up @@ -3130,7 +3132,7 @@ CallResult<uint32_t> appendAllPropertyKeys(
}
}
}; // end of lambda expression
while (lv.head.get()) {
for (bool first = true; lv.head.get(); first = false) {
GCScope gcScope(runtime);

// enumerableProps will contain all enumerable own properties from obj.
Expand All @@ -3145,6 +3147,13 @@ CallResult<uint32_t> appendAllPropertyKeys(
}
auto enumerableProps = *cr;
auto marker = gcScope.createMarker();
if (first && beginIndex > 0) {
arr->set(
runtime,
1,
HermesValue::encodeTrustedNumberValue(
enumerableProps->getEndIndex()));
}
for (unsigned i = 0, e = enumerableProps->getEndIndex(); i < e; ++i) {
gcScope.flushToMarker(marker);
lv.prop = enumerableProps->at(runtime, i).unboxToHV(runtime);
Expand Down Expand Up @@ -3221,8 +3230,11 @@ CallResult<uint32_t> appendAllPropertyKeys(
}

/// Adds the hidden classes of the prototype chain of obj to arr,
/// starting with the prototype of obj at index 0, etc., and
/// starting with the class of obj at index 2, etc., and
/// terminates with null.
/// Index 0 will contain the end index after this is complete.
/// Index 1 will contain the number of properties pushed in obj.
/// Index 2 will contain the class of obj.
///
/// \param obj The object whose prototype chain should be output
/// \param[out] arr The array where the classes will be appended. This
Expand All @@ -3232,15 +3244,31 @@ ExecutionStatus setProtoClasses(
Handle<JSObject> obj,
MutableHandle<BigStorage> &arr) {
// Layout of a JSArray stored in the for-in cache:
// [class(proto(obj)), class(proto(proto(obj))), ..., null, prop0, prop1, ...]
// [numProtos, numObjProps, class(obj),
// class(proto(obj)), class(proto(proto(obj))), ..., null,
// prop0, prop1, ...]
// prop0 is at index numProtos.
// There are numObjProps properties on obj before the proto properties start.

if (!obj->shouldCacheForIn(runtime)) {
arr->clear(runtime);
return ExecutionStatus::RETURNED;
}
MutableHandle<JSObject> head(runtime, obj->getParent(runtime));
MutableHandle<JSObject> head(runtime, *obj);
MutableHandle<> clazz(runtime);
GCScopeMarkerRAII marker{runtime};
// Push entries 0 and 1 (we'll fill them in later).
clazz = HermesValue::encodeTrustedNumberValue(0);
if (LLVM_UNLIKELY(
BigStorage::push_back(arr, runtime, clazz) ==
ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
if (LLVM_UNLIKELY(
BigStorage::push_back(arr, runtime, clazz) ==
ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
while (head.get()) {
if (!head->shouldCacheForIn(runtime)) {
arr->clear(runtime);
Expand All @@ -3262,11 +3290,22 @@ ExecutionStatus setProtoClasses(
marker.flush();
}
clazz = HermesValue::encodeNullValue();
return BigStorage::push_back(arr, runtime, clazz);
if (LLVM_UNLIKELY(
BigStorage::push_back(arr, runtime, clazz) ==
ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}

arr->set(
runtime, 0, HermesValue::encodeTrustedNumberValue(arr->size(runtime)));
return ExecutionStatus::RETURNED;
}

/// Verifies that the classes of obj's prototype chain still matches those
/// previously prefixed to arr by setProtoClasses.
/// Assumes that the first elements are the count of HiddenClasses and the count
/// of the object's own properties, and element 2 is is the object's own
/// HiddenClass, so it doesn't check those.
///
/// \param obj The object whose prototype chain should be verified
/// \param arr Array previously populated by setProtoClasses
Expand All @@ -3277,7 +3316,8 @@ uint32_t matchesProtoClasses(
Handle<JSObject> obj,
Handle<BigStorage> arr) {
MutableHandle<JSObject> head(runtime, obj->getParent(runtime));
uint32_t i = 0;
// Skip the counts and object's own class.
uint32_t i = 3;
while (head.get()) {
HermesValue protoCls = arr->at(runtime, i++);
if (protoCls.isNull() || protoCls.getObject() != head->getClass(runtime) ||
Expand Down
20 changes: 20 additions & 0 deletions lib/VM/StaticH.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1865,16 +1865,36 @@ extern "C" SHLegacyValue _sh_ljs_get_next_pname_rjs(
PinnedValue<> tmp;
PinnedValue<JSObject> propObj;
PinnedValue<SymbolID> tmpPropNameStorage;
PinnedValue<HiddenClass> cachedClass;
} lv;
LocalsRAII lraii{runtime, &lv};
GCScopeMarkerRAII marker{runtime};
uint32_t idx = toPHV(indexVal)->getNumber();
uint32_t size = toPHV(sizeVal)->getNumber();
// If there's a class at index 2, it means we have a cached class.
uint32_t startIdx = 0;
uint32_t numObjProps = 0;
if (LLVM_LIKELY(size > 2)) {
lv.cachedClass = dyn_vmcast<HiddenClass>(arr->at(runtime, 2));
if (lv.cachedClass.get()) {
startIdx = arr->at(runtime, 0).getNumberAs<uint32_t>();
numObjProps = arr->at(runtime, 1).getNumberAs<uint32_t>();
}
}

MutableHandle<JSObject> propObj{lv.propObj};
MutableHandle<SymbolID> tmpPropNameStorage{lv.tmpPropNameStorage};
// Loop until we find a property which is present.
while (idx < size) {
lv.tmp = arr->at(runtime, idx);
// If there's no caching, lv.cachedClass is nullptr and the comparison
// will fail.
if (LLVM_LIKELY(size > 0) && idx - startIdx < numObjProps &&
LLVM_LIKELY(lv.cachedClass.get() == obj->getClass(runtime))) {
// Cached.
propObj = obj;
break;
}
if (lv.tmp->isSymbol()) {
// NOTE: This call is safe because we immediately discard desc,
// so it can't outlive the SymbolID.
Expand Down

0 comments on commit 1a5a235

Please sign in to comment.