Skip to content

Commit

Permalink
Factor out logic for updating ClassFlags
Browse files Browse the repository at this point in the history
Summary:
Encapsulate the logic for updating the ClassFlags of a HiddenClass.
This is useful in conjunction with the next diff which adds an
additional class flag which needs to be updated each time there is a
property added.

Reviewed By: tmikov

Differential Revision: D42047721

fbshipit-source-id: a5a71cd9c587981801cb64d588995d95de0aa053
  • Loading branch information
fbmal7 authored and facebook-github-bot committed Jan 10, 2023
1 parent 810d587 commit e42cdb4
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 14 deletions.
16 changes: 16 additions & 0 deletions include/hermes/VM/HiddenClass.h
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,15 @@ class HiddenClass final : public GCCell {
/// Cache that contains for-in property names for objects of this class.
/// Never used in dictionary mode.
GCPointer<BigStorage> forInCache_{};

/// Computes the updated class flags for a class with flags \p flags for when
/// a property is added or updated with property flags \p pf and based on
/// whether a new index like property has been added.
/// This operation is idempotent, which means that multiple updates with the
/// same \p pf and \p addedIndexLike may be collapsed into a single update.
/// \return Updated flags that reflect the added/updated property.
static ClassFlags
computeFlags(ClassFlags flags, PropertyFlags pf, bool addedIndexLike);
};

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -640,6 +649,13 @@ inline OptValue<bool> HiddenClass::tryFindPropertyFast(
}
return llvh::None;
}
inline ClassFlags HiddenClass::computeFlags(
ClassFlags flags,
PropertyFlags pf,
bool addedIndexLike) {
flags.hasIndexLikeProperties |= addedIndexLike;
return flags;
}

} // namespace vm
} // namespace hermes
Expand Down
33 changes: 19 additions & 14 deletions lib/VM/HiddenClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,10 +379,11 @@ CallResult<std::pair<Handle<HiddenClass>, SlotIndex>> HiddenClass::addProperty(
assert(propertyFlags.isValid() && "propertyFlags must be valid");

if (LLVM_UNLIKELY(selfHandle->isDictionary())) {
if (toArrayIndex(
runtime.getIdentifierTable().getStringView(runtime, name))) {
selfHandle->flags_.hasIndexLikeProperties = true;
}
auto isIndexLike =
toArrayIndex(runtime.getIdentifierTable().getStringView(runtime, name))
.hasValue();
selfHandle->flags_ =
computeFlags(selfHandle->flags_, propertyFlags, isIndexLike);

// Allocate a new slot.
// TODO: this changes the property map, so if we want to support OOM
Expand Down Expand Up @@ -452,10 +453,11 @@ CallResult<std::pair<Handle<HiddenClass>, SlotIndex>> HiddenClass::addProperty(
// Do it.
auto childHandle = copyToNewDictionary(selfHandle, runtime);

if (toArrayIndex(
runtime.getIdentifierTable().getStringView(runtime, name))) {
childHandle->flags_.hasIndexLikeProperties = true;
}
auto isIndexLike =
toArrayIndex(runtime.getIdentifierTable().getStringView(runtime, name))
.hasValue();
childHandle->flags_ =
computeFlags(childHandle->flags_, propertyFlags, isIndexLike);

// Add the property to the child.
if (LLVM_UNLIKELY(
Expand All @@ -471,11 +473,16 @@ CallResult<std::pair<Handle<HiddenClass>, SlotIndex>> HiddenClass::addProperty(
return std::make_pair(childHandle, childHandle->numProperties_++);
}

auto isIndexLike =
toArrayIndex(runtime.getIdentifierTable().getStringView(runtime, name))
.hasValue();
auto newFlags = computeFlags(selfHandle->flags_, propertyFlags, isIndexLike);

// Allocate the child.
auto childHandle = runtime.makeHandle<HiddenClass>(
runtime.ignoreAllocationFailure(HiddenClass::create(
runtime,
selfHandle->flags_,
newFlags,
selfHandle,
name,
propertyFlags,
Expand All @@ -489,10 +496,6 @@ CallResult<std::pair<Handle<HiddenClass>, SlotIndex>> HiddenClass::addProperty(
inserted &&
"transition already exists when adding a new property to hidden class");

if (toArrayIndex(runtime.getIdentifierTable().getStringView(runtime, name))) {
childHandle->flags_.hasIndexLikeProperties = true;
}

if (selfHandle->propertyMap_) {
assert(
!DictPropertyMap::find(selfHandle->propertyMap_.get(runtime), name) &&
Expand Down Expand Up @@ -542,6 +545,7 @@ Handle<HiddenClass> HiddenClass::updateProperty(
assert(
selfHandle->propertyMap_ &&
"propertyMap must exist in dictionary mode");
selfHandle->flags_ = computeFlags(selfHandle->flags_, newFlags, false);
DictPropertyMap::getDescriptorPair(
selfHandle->propertyMap_.getNonNull(runtime), pos)
->second.flags = newFlags;
Expand Down Expand Up @@ -605,7 +609,7 @@ Handle<HiddenClass> HiddenClass::updateProperty(
auto childHandle = runtime.makeHandle<HiddenClass>(
runtime.ignoreAllocationFailure(HiddenClass::create(
runtime,
selfHandle->flags_,
computeFlags(selfHandle->flags_, newFlags, false),
selfHandle,
name,
transitionFlags,
Expand Down Expand Up @@ -755,6 +759,7 @@ Handle<HiddenClass> HiddenClass::updatePropertyFlagsWithoutTransitions(
DictPropertyMap::forEachMutablePropertyDescriptor(
mapHandle, runtime, changeFlags);
}
classHandle->flags_ = computeFlags(classHandle->flags_, flagsToSet, false);

return std::move(classHandle);
}
Expand Down

0 comments on commit e42cdb4

Please sign in to comment.