Add fast paths for Array.prototype shift/unshift/slice/forEach/map/filter#2093
Add fast paths for Array.prototype shift/unshift/slice/forEach/map/filter#2093zhelezkov wants to merge 2 commits into
Conversation
…lter Use arrayFastPathCheck to bypass the generic per-index property protocol for ordinary JSArrays: - shift/unshift: move elements directly in the indexed storage under NoAllocScope and update the length slot directly. The fast path is guarded by arrayFastPathCheck against runtime.arrayClass, so it bails to the generic path for accessors, index-like properties on the prototype chain, read-only length, and frozen/sealed arrays. - slice: copy the elements straight out of the source storage into the new array's storage. The check runs after argument coercion, so arrays mutated by valueOf/toString side effects fall back to the generic path. - forEach/map/filter: hoist the fast path check out of the loop and read elements directly from the indexed storage via the shared readElementForCallback helper. This stays correct across callback mutations because a non-empty slot in the indexed storage is always a plain own data property (converting an element to an accessor deletes the indexed slot first), and at() re-checks bounds against the current storage every iteration; holes fall back to the generic per-index read. Microbenchmarks (M2, Release, interpreter): shift ~35x, unshift ~36x, slice ~11x, forEach/map/filter ~1.3-1.45x. push/pop and splice are unchanged (parity). New lit test covers holes, mid-iteration mutation, accessors installed by callbacks, Proxy prototypes, frozen/sealed arrays, and elements on Array.prototype. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Hi @zhelezkov! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
meta-cla
Bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Follow the same structure as pop: when the fast-path check fails for a JSArray, populate the O local from the this-arg and keep the length already read via JSArray::getLength, so the slow path skips the redundant toObject and length property read. Zero-argument unshift on a plain array now also takes the cheap length fetch. Also drop the two stale "Add a fast path for actual arrays" TODOs, which are addressed by the fast paths at the top of each function.
1 participant
Hey all,
I've been exploring the Hermes codebase to investigate some performance aspects of our React Native app, and I noticed a few potentially small but meaningful improvements around array fast-path handling.
AI usage notice: I used AI to help identify opportunities, implement the changes, review the code, and create benchmark tests. I also manually reviewed the code and validated the benchmark results to ensure the improvements are correct and meaningful.
Please let me know what I can improve/adjust to get this optimizations upstreamed into hermes!
Description
Resolves the
TODO: Implement a fast path for actual arrays.in severalArray.prototypemethods by using the existingarrayFastPathCheckto bypass the generic per-index property protocol for ordinaryJSArrays:shift/unshift: move elements directly in the indexed storage underNoAllocScopeand write the new length straight to the length slot. Guarded byarrayFastPathCheckagainstruntime.arrayClass, so accessors, index-like properties anywhere on the prototype chain, a read-onlylength, and frozen/sealed arrays all fall back to the generic path. Both entry points are wrapped inNoLeakHandleScopeto ensure the fast path allocates no handles.slice: copy elements straight from the source storage into the new array's storage. The check runs after argument coercion, so an array mutated by avalueOf/toStringside effect fails the check (element count no longer matches the capturedlen) and falls back.forEach/map/filter: hoist the fast-path check out of the loop and read each element via a new sharedreadElementForCallbackhelper that tries the indexed storage first. This remains correct even though callbacks can mutate the array mid-iteration: a non-empty slot in the indexed storage is always a plain own data property (converting an element to an accessor deletes the indexed slot first), andat()re-checks bounds against the current storage on every iteration. Holes fall back to the generic per-index read, which handles prototype-chain lookups (including Proxy prototypes installed mid-iteration).Benchmarks
Apple M2, Release build, default options (HV64, no JIT — plain interpreter, matching how Hermes ships in React Native). Minimum of 3 timed runs after warmup; identical result checksums across both binaries.
shift/unshift/sliceimprove by an order of magnitude because the fast path replaces a computed-descriptor lookup plus property get/put per element with raw indexed-storage moves.forEach/map/filterimprove less because the per-element callback invocation dominates in the interpreter.Test plan
test/hermes/array-fastpath-mutation.jscovering holes, callback-driven mutation mid-iteration (shrink/grow), accessors installed by callbacks, Proxy prototypes installed mid-iteration, frozen/sealed arrays, read-onlylength, argument-coercion side effects inslice, and index-like properties onArray.prototype.check-hermeson a Debug build: all tests pass.-DHERMESVM_HEAP_HV_MODE=HEAP_HV_BOXEDDebug build to force boxed doubles through the fast paths.