LibJS: Optimize keys deduplication in get_object_property_iterator()
Some checks are pending
CI / macOS, arm64, Sanitizer, Clang (push) Waiting to run
CI / Linux, x86_64, Fuzzers, Clang (push) Waiting to run
CI / Linux, x86_64, Sanitizer, GNU (push) Waiting to run
CI / Linux, x86_64, Sanitizer, Clang (push) Waiting to run
Package the js repl as a binary artifact / Linux, arm64 (push) Waiting to run
Package the js repl as a binary artifact / macOS, arm64 (push) Waiting to run
Package the js repl as a binary artifact / Linux, x86_64 (push) Waiting to run
Run test262 and test-wasm / run_and_update_results (push) Waiting to run
Lint Code / lint (push) Waiting to run
Label PRs with merge conflicts / auto-labeler (push) Waiting to run
Push notes / build (push) Waiting to run

This change implements a part responsible for this invariant in a more
efficient way:
"Enumerating the properties of the target object includes enumerating
properties of its prototype, and the prototype of the prototype, and so
on, recursively; but a property of a prototype is not processed if it
has the same name as a property that has already been processed by the
iterator's next method."

Previously we inserted `(key, enumerable)` pairs into an
`OrderedHashTable`. That always built and maintained a hash table, even
when no prototype-level filtering was needed.

Now we:
- Collect only enumerable keys into `Vector<PropertyKey>`.
- Track `seen_non_enumerable_properties` so a non-enumerable own
  property still shadows prototype properties with the same name.
- Lazily materialize `HashTable<PropertyKey>` only if we encounter an
  enumerable property on a prototype and must check for duplicates. In
  the common case materialization is avoided, because default Object or
  Array prototype properties are non-enumerable.
This commit is contained in:
Aliaksandr Kalenik 2025-09-20 19:16:31 +02:00 committed by Alexander Kalenik
commit 1d41cf10f4
Notes: github-actions[bot] 2025-09-21 13:07:33 +00:00

View file

@ -42,32 +42,6 @@
#include <LibJS/Runtime/ValueInlines.h>
#include <LibJS/SourceTextModule.h>
namespace JS {
struct PropertyKeyAndEnumerableFlag {
JS::PropertyKey key;
bool enumerable { false };
};
}
namespace AK {
template<>
struct Traits<JS::PropertyKeyAndEnumerableFlag> : public DefaultTraits<JS::PropertyKeyAndEnumerableFlag> {
static unsigned hash(JS::PropertyKeyAndEnumerableFlag const& entry)
{
return Traits<JS::PropertyKey>::hash(entry.key);
}
static bool equals(JS::PropertyKeyAndEnumerableFlag const& a, JS::PropertyKeyAndEnumerableFlag const& b)
{
return Traits<JS::PropertyKey>::equals(a.key, b.key);
}
};
}
namespace JS::Bytecode {
bool g_dump_bytecode = false;
@ -1794,17 +1768,17 @@ public:
ScopeGuard remove_first = [&] { ++m_iterator; };
// If the property is deleted, don't include it (invariant no. 2)
if (!TRY(m_object->has_property(entry.key)))
if (!TRY(m_object->has_property(entry)))
continue;
done = false;
value = entry.key.to_value(vm);
value = entry.to_value(vm);
return {};
}
}
private:
PropertyNameIterator(JS::Realm& realm, GC::Ref<Object> object, OrderedHashTable<PropertyKeyAndEnumerableFlag> properties)
PropertyNameIterator(JS::Realm& realm, GC::Ref<Object> object, Vector<PropertyKey> properties)
: Object(realm, nullptr)
, m_object(object)
, m_properties(move(properties))
@ -1819,7 +1793,7 @@ private:
}
GC::Ref<Object> m_object;
OrderedHashTable<PropertyKeyAndEnumerableFlag> m_properties;
Vector<PropertyKey> m_properties;
decltype(m_properties.begin()) m_iterator;
};
@ -1855,31 +1829,43 @@ inline ThrowCompletionOr<Value> get_object_property_iterator(Interpreter& interp
}
seen_objects.clear_with_capacity();
OrderedHashTable<PropertyKeyAndEnumerableFlag> properties;
Vector<PropertyKey> properties;
properties.ensure_capacity(estimated_properties_count);
HashTable<PropertyKey> seen_non_enumerable_properties;
Optional<HashTable<PropertyKey>> seen_properties;
auto ensure_seen_properties = [&] {
if (seen_properties.has_value())
return;
seen_properties = HashTable<PropertyKey> {};
seen_properties->ensure_capacity(properties.size());
for (auto const& property : properties)
seen_properties->set(property);
};
// Collect all keys immediately (invariant no. 5)
bool in_prototype_chain = false;
for (auto object_to_check = GC::Ptr { object.ptr() }; object_to_check && !seen_objects.contains(*object_to_check); object_to_check = TRY(object_to_check->internal_get_prototype_of())) {
seen_objects.set(*object_to_check);
TRY(object_to_check->for_each_own_property_with_enumerability([&](PropertyKey const& property_key, bool enumerable) -> ThrowCompletionOr<void> {
// NOTE: If there is a non-enumerable property higher up the prototype chain with the same key,
// we mustn't include this property even if it's enumerable (invariant no. 5 and 6)
// This is achieved with the PropertyKeyAndEnumerableFlag struct, which doesn't consider
// the enumerable flag when comparing keys.
PropertyKeyAndEnumerableFlag new_entry {
.key = property_key,
.enumerable = false,
};
if (properties.contains(new_entry))
return {};
new_entry.enumerable = enumerable;
properties.set(move(new_entry), AK::HashSetExistingEntryBehavior::Keep);
if (!enumerable)
seen_non_enumerable_properties.set(property_key);
if (in_prototype_chain && enumerable) {
if (seen_non_enumerable_properties.contains(property_key))
return {};
ensure_seen_properties();
if (seen_properties->contains(property_key))
return {};
}
if (enumerable)
properties.append(property_key);
if (seen_properties.has_value())
seen_properties->set(property_key);
return {};
}));
in_prototype_chain = true;
}
properties.remove_all_matching([&](auto& key) { return !key.enumerable; });
auto iterator = interpreter.realm().create<PropertyNameIterator>(interpreter.realm(), object, move(properties));
return vm.heap().allocate<IteratorRecord>(iterator, js_undefined(), false);