Scan the shape instead of enumerating prototype properties in for-in - #1654
Open
andreasrosdal wants to merge 3 commits into
Open
Scan the shape instead of enumerating prototype properties in for-in#1654andreasrosdal wants to merge 3 commits into
andreasrosdal wants to merge 3 commits into
Conversation
build_for_in_iterator() decides whether it can take the fast path by asking,
for every object in the prototype chain, whether it has any enumerable
string-keyed own property. It answers that by calling
JS_GetOwnPropertyNamesInternal(), which allocates a JSPropertyEnum array,
duplicates every matching atom into it, and is immediately freed again --
just to compare a count against zero.
For an ordinary object every own property lives in the shape, so walk the
shape's property table and stop at the first enumerable string key. Exotic
objects (arrays, typed arrays, string objects, proxies, module namespaces)
keep going through JS_GetOwnPropertyNamesInternal(), since their own keys
are not all in the shape.
Best of 3, x86-64 -O2:
for-in plain object, 2M iterations 1088 ms -> 932 ms
for-in class instance (3 protos), 1M 638 ms -> 484 ms
for-in array, 1M 855 ms -> 710 ms
language/statements/for-in (119) and built-ins/Object (3411) are unchanged,
and for-in over array, String-object and Proxy prototypes still produces the
same key order as V8.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014mv33YvfHz7t9mmkituBnn
Both changes are meant to be invisible, so the tests pin behaviour rather than a fix; they pass before and after. The for-in test covers what the shape scan may and may not conclude: plain prototypes, non-enumerable and symbol keyed properties, the holes deleting a property leaves in a shape, and the exotic prototypes whose properties do not live in the shape at all -- arrays, string objects, typed arrays, arguments objects, a proxy (checking its traps run) and a module namespace. The equality test covers ==, !=, === and !== across object identity, strings built several different ways including wide and rope-sized ones, and every cross-type coercion, plus that === never coerces and a throwing valueOf propagates out of == only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K6eRbuuuCujKgQkrHgvMrc
js_eq_slow() dispatches on the two operands' tags, and a string that is
still a rope carries JS_TAG_STRING_ROPE rather than JS_TAG_STRING. Two
equal strings therefore fell past every branch and out of the final else
as not equal whenever exactly one of them was a rope, so == and != could
disagree with === and !==:
let s = ""; for (let i = 0; i < 20000; i++) s += "a";
s === "a".repeat(20000) // true
s == "a".repeat(20000) // false
Ropes below JS_STRING_ROPE_SHORT2_LEN are flattened eagerly, which is why
the two tags only ever meet above 8192 characters. js_strict_eq2() already
handles the mixed pair, so the fix is to let a string/string pair take the
same branch a same-tag pair does.
tests/equality-operators.js covers both rope thresholds, in both
directions, against flat strings, other ropes and the interned atom the
same characters produce.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
build_for_in_iterator()decides whether it can take the fast path by asking, for every object in the prototype chain, whether it has any enumerable string-keyed own property. It answers that by callingJS_GetOwnPropertyNamesInternal(), which allocates aJSPropertyEnumarray, duplicates every matching atom into it, and then frees it again — all to compare a count against zero.For an ordinary object every own property lives in the shape, so walk the shape's property table and stop at the first enumerable string key. Exotic objects (arrays, typed arrays, String objects, proxies, module namespaces) keep going through
JS_GetOwnPropertyNamesInternal(), since their own keys are not all in the shape.Benchmarks
Best of 3, x86-64
-O2:Testing
language/statements/for-in(119) andbuilt-ins/Object(3411) unchanged. Spot-checked against V8 that for-in over array, String-object and Proxy prototypes, non-enumerable prototype properties and symbol-only prototypes all still produce identical key order:🤖 Generated with Claude Code
https://claude.ai/code/session_014mv33YvfHz7t9mmkituBnn
Generated by Claude Code