Repository navigation
Conversation
WalkthroughJavaScriptCore now applies sealing and freezing attributes to sparse indexed properties, extends array-specific integrity handling, centralizes fast-path selection, and adds stress-test coverage plus a large-array microbenchmark. ChangesArray integrity operations
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
Preview Builds
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
robobun
force-pushed
the
farm/fbe5fb68/fast-seal-freeze-arrays
branch
from
July 25, 2026 00:11
8bb57a3 to
c00301c
Compare
robobun
added a commit
to oven-sh/bun
that referenced
this pull request
Jul 25, 2026
…and JSFinalObject with indexed properties objectConstructorSeal / objectConstructorFreeze only take the JSObject::seal / JSObject::freeze fast path for a JSFinalObject with no indexed properties; every other receiver falls through to the generic SetIntegrityLevel loop, which per element allocates an Identifier, dispatches through the method table, and walks defineOwnProperty. On a 1000-element array that is three O(n) passes on top of the dictionary conversion. Teach JSObject::seal / freeze to also set the integrity attributes on the SparseArrayValueMap (new seal()/freeze() mirroring PropertyTable::seal/freeze), freeze JSArray "length" from the same place, and widen the fast-path gate to JSFinalObject || isJSArray(). JSArray has no lazily reified own properties so materializeLazyOwnProperties skips it, and Structure::isSealed/isFrozen only cover named properties so the early returns skip their short circuit when indexed properties are present. For the oven-sh/bun#6360 benchmark (2000 arrays, avg 1000 elements) this brings Object.seal from ~460ms to ~90ms and Object.freeze from ~530ms to ~100ms on x86_64; the remainder is the contiguous -> sparse map conversion that JSC uses to represent non-configurable indices.
robobun
force-pushed
the
farm/fbe5fb68/fast-seal-freeze-arrays
branch
from
July 25, 2026 04:45
c00301c to
e4f1d0d
Compare
robobun
added a commit
to oven-sh/bun
that referenced
this pull request
Jul 25, 2026
There was a problem hiding this comment.
The earlier notifyPresenceOfIndexedAccessors issue is fixed and covered by the new prototype-chain regression tests, and I didn't find further problems on this pass — but this widens a spec-equivalence fast path in core JSObject runtime code, so it's worth a human look.
What was reviewed:
JSObject::seal/freezeearly-return gating vs. indexed properties, and thatenterDictionaryIndexingModealways yields ArrayStorage + sparse map forJSArray(incl. empty arrays) sosetLengthIsReadOnlyis reached.SparseArrayValueMap::freeze()accessor-vs-data attribute handling matchesPropertyTable::freeze()and the genericSetIntegrityLeveldescriptor.canFastSetIntegrityLevelgate:isJSArrayis exact-ArrayType, soDerivedArrayType/Proxy/typed arrays still hit the observable loop.materializeLazyOwnPropertiesskippingJSArray—JSArray::getOwnSpecialPropertyNamesonly addslength, which is virtual and alreadyDontDelete.
Extended reasoning...
Overview
The PR widens the Object.seal/Object.freeze fast path from "JSFinalObject with no indexed properties" to "JSFinalObject or exact JSArray", and teaches JSObject::seal/freeze to bulk-apply DontDelete/ReadOnly to the SparseArrayValueMap in place of the per-index defineOwnProperty loop. Six files touched: JSObject.cpp, ObjectConstructor.cpp, SparseArrayValueMap.{h,cpp}, plus a stress test and a microbenchmark.
Security risks
None identified. This is a spec-equivalence optimization on ordinary object internal methods; no attacker-controlled input reaches new parsing or allocation paths, and the gate is narrowed to exact JSFinalObject/ArrayType so exotic [[DefineOwnProperty]]/[[PreventExtensions]] overrides (Proxy, typed arrays, module namespaces, DerivedArrayType) still take the generic observable loop.
Level of scrutiny
High. This is core JSC runtime code where the fast path must be observationally identical to the ECMAScript SetIntegrityLevel algorithm across every reachable object shape. The first revision already had one real bug (missing MayHaveIndexedAccessors structure bit → prototype-chain ReadOnly enforcement bypassed), now fixed with regression coverage. That kind of invariant — structure-transition side effects the old defineOwnIndexedProperty path performed implicitly — is exactly what makes this change hard to verify by inspection alone, so it warrants a human reviewer with JSC object-model expertise.
Other factors
- The follow-up commit adds
notifyPresenceOfIndexedAccessors(vm)aftermap->freeze()and regression tests for both freeze-before-prototype and freeze-after-prototype cases, plus a positive test that a sealed prototype still allows shadowing. This addresses the earlier finding fully. - I re-checked that
enterDictionaryIndexingModeforArrayClass(empty array) still allocates ArrayStorage and a sparse map, sosetLengthIsReadOnly()is reached forObject.freeze([]); the stress test asserts this. - The
isSealed(vm)/isFrozen(vm)short-circuit after the sparse-map update is sound because at that point indexed attributes have already been applied andStructure::didTransition()is true (thepreventExtensionsTransitioninsideenterDictionaryIndexingMode's callers isn't relevant here, but the storage-type transition is), so the only remaining work is the named-propertysealTransition/freezeTransition. - test262 (
built-ins/Object/{seal,freeze,isSealed,isFrozen,preventExtensions}, 279 tests) and existingJSTests/stress/*seal*|*freeze*are reported passing. - Not approving because this is a non-mechanical change to a critical, spec-sensitive runtime path that already surfaced one subtle correctness bug during review.
robobun
added a commit
to oven-sh/bun
that referenced
this pull request
Jul 25, 2026
robobun
added a commit
that referenced
this pull request
Oct 1, 2026
…ArrayStorage vector Object.preventExtensions, Object.seal, Object.freeze and a read-only "length" all went through JSObject::enterDictionaryIndexingMode, which moved every element of the object into the SparseArrayValueMap (sparse mode, vector length 0). After that every a[i] read missed the JIT vector fast paths and did a hash lookup in C++, and the freeze itself cost two butterfly conversions, a map cell and one hash insert per element. Now enterDictionaryIndexingMode keeps the elements in the vector and only switches the object to the SlowPutArrayStorage shape. LLInt and the baseline JIT never store into a SlowPutArrayStorage vector inline, and DFG / FTL now check the structure before they do, so every write reaches the C++ paths. The attributes of the elements live on the Structure: vectorElementsAreNonConfigurable (Seal), vectorElementsAreReadOnly (Freeze) and arrayLengthIsReadOnly (Freeze and the new SetArrayLengthReadOnly transition, which replaces the LengthIsReadOnly flag of the sparse map). Entries of the sparse map keep their own attributes; elements only move into the map when a single index needs attributes of its own (Object.defineProperty on an element), and they carry the structure's attributes with them. A non-extensible object therefore never has the plain ArrayStorage shape (suggestedArrayStorageTransition), which is what lets the plain ArrayStorage paths stay as they were. The C++ vector writers and readers consult the bits: putByIndex, trySetIndexQuickly, putDirectIndex and the beyond-vector-length paths, deletePropertyByIndex, defineOwnIndexedProperty, getOwnPropertySlotByIndex, the prototype-chain put intercept, JSArray::setLength / pop / push and Array.prototype.reverse. The DFG folds a read of a frozen element from the vector as it folded one from the sparse map; JSObject::freeze fences the element stores before the structure store. ArrayAllocationProfile ignores the shape of an array that became non-extensible, so one frozen array does not make the arrays its allocation site creates next SlowPutArrayStorage. The baseline indexed load and "in" ICs accept the SlowPutArrayStorage shape, whose vector they already read. Object.isSealed / isFrozen answer from the structure for an object that JSObject::seal / freeze sealed or froze. On the oven-sh/bun#44305 repro (300k arrays of 16 ints, x86_64 release jsc): freeze 970 ms -> 35-47 ms, reads from the frozen arrays 9.5x plain -> 1.0-1.2x plain. Builds on #337, which is the first commit of this branch.
robobun
added a commit
that referenced
this pull request
Oct 1, 2026
…ArrayStorage vector Object.preventExtensions, Object.seal, Object.freeze and a read-only "length" all went through JSObject::enterDictionaryIndexingMode, which moved every element of the object into the SparseArrayValueMap (sparse mode, vector length 0). After that every a[i] read missed the JIT vector fast paths and did a hash lookup in C++, and the freeze itself cost two butterfly conversions, a map cell and one hash insert per element. Now enterDictionaryIndexingMode keeps the elements in the vector and only switches the object to the SlowPutArrayStorage shape. LLInt and the baseline JIT never store into a SlowPutArrayStorage vector inline, and DFG / FTL now check the structure before they do, so every write reaches the C++ paths. The attributes of the elements live on the Structure: vectorElementsAreNonConfigurable (Seal), vectorElementsAreReadOnly (Freeze) and arrayLengthIsReadOnly (Freeze and the new SetArrayLengthReadOnly transition, which replaces the LengthIsReadOnly flag of the sparse map). Entries of the sparse map keep their own attributes; elements only move into the map when a single index needs attributes of its own (Object.defineProperty on an element), and they carry the structure's attributes with them. A non-extensible object therefore never has the plain ArrayStorage shape (suggestedArrayStorageTransition), which is what lets the plain ArrayStorage paths stay as they were. The C++ vector writers and readers consult the bits: putByIndex, trySetIndexQuickly, putDirectIndex and the beyond-vector-length paths, deletePropertyByIndex, defineOwnIndexedProperty, getOwnPropertySlotByIndex, the prototype-chain put intercept, JSArray::setLength / pop / push and Array.prototype.reverse. An array with no elements (ArrayClass, such as Array.prototype) stays blank: the bits and isStructureExtensible() cover the later writes, so freezing Array.prototype keeps the array prototype chain watchpoint. A frozen object tells the objects that inherit from it about its read-only elements (notifyPresenceOfIndexedAccessors) when it becomes a prototype, not when it is frozen, so a frozen array that is no prototype keeps the builtin fast paths. The DFG folds a read of a frozen element from the vector as it folded one from the sparse map; JSObject::freeze fences the element stores before the structure store. ArrayAllocationProfile never records the SlowPutArrayStorage shape, so one frozen array does not make the arrays its allocation site creates next SlowPutArrayStorage. The baseline indexed load and "in" ICs accept the SlowPutArrayStorage shape, whose vector they already read. Object.isSealed / isFrozen answer from the structure for an object that JSObject::seal / freeze sealed or froze. Object.seal / Object.freeze take the JSObject::seal / freeze path for an exact JSArray and for a JSFinalObject with indexed properties instead of the generic per-index SetIntegrityLevel loop (from #337, which this supersedes); JSArray has no lazily reified own properties, so materializeLazyOwnProperties skips it. On the oven-sh/bun#44305 repro (300k arrays of 16 ints, x86_64 release jsc): freeze 970 ms -> 35-47 ms, reads from the frozen arrays 9.5x plain -> 1.0-1.2x plain. Supersedes #337. The ArrayClass handling and freeze-array-prototype.js come from #622. Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Collaborator
Author
|
#752 carries this change (the objectConstructorSeal / Freeze gate widening and SparseArrayValueMap::seal / freeze) and also keeps the elements in the ArrayStorage vector instead of moving them into the sparse map, which is what made reads from frozen arrays slow. If #752 lands, this one can be closed. |
robobun
added a commit
that referenced
this pull request
Oct 1, 2026
…ArrayStorage vector Object.preventExtensions, Object.seal, Object.freeze and a read-only "length" all went through JSObject::enterDictionaryIndexingMode, which moved every element of the object into the SparseArrayValueMap (sparse mode, vector length 0). After that every a[i] read missed the JIT vector fast paths and did a hash lookup in C++, and the freeze itself cost two butterfly conversions, a map cell and one hash insert per element. Now enterDictionaryIndexingMode keeps the elements in the vector and only switches the object to the SlowPutArrayStorage shape. LLInt and the baseline JIT never store into a SlowPutArrayStorage vector inline, and DFG / FTL now check the structure before they do, so every write reaches the C++ paths. The attributes of the elements live on the Structure: vectorElementsAreNonConfigurable (Seal), vectorElementsAreReadOnly (Freeze) and arrayLengthIsReadOnly (Freeze and the new SetArrayLengthReadOnly transition, which replaces the LengthIsReadOnly flag of the sparse map). Entries of the sparse map keep their own attributes; elements only move into the map when a single index needs attributes of its own (Object.defineProperty on an element), and they carry the structure's attributes with them. A non-extensible object therefore never has the plain ArrayStorage shape (suggestedArrayStorageTransition), which is what lets the plain ArrayStorage paths stay as they were. The C++ vector writers and readers consult the bits: putByIndex, trySetIndexQuickly, putDirectIndex and the beyond-vector-length paths, deletePropertyByIndex, defineOwnIndexedProperty, getOwnPropertySlotByIndex, the prototype-chain put intercept, JSArray::setLength / pop / push and Array.prototype.reverse. An array with no elements (ArrayClass, such as Array.prototype) stays blank: the bits and isStructureExtensible() cover the later writes, so freezing Array.prototype keeps the array prototype chain watchpoint. A frozen object tells the objects that inherit from it about its read-only elements (notifyPresenceOfIndexedAccessors) when it becomes a prototype, not when it is frozen, so a frozen array that is no prototype keeps the builtin fast paths. The DFG folds a read of a frozen element from the vector as it folded one from the sparse map; JSObject::freeze fences the element stores before the structure store. ArrayAllocationProfile never records the SlowPutArrayStorage shape, so one frozen array does not make the arrays its allocation site creates next SlowPutArrayStorage. The baseline indexed load and "in" ICs accept the SlowPutArrayStorage shape, whose vector they already read. Object.isSealed / isFrozen answer from the structure for an object that JSObject::seal / freeze sealed or froze. Object.seal / Object.freeze take the JSObject::seal / freeze path for an exact JSArray and for a JSFinalObject with indexed properties instead of the generic per-index SetIntegrityLevel loop (from #337, which this supersedes); JSArray has no lazily reified own properties, so materializeLazyOwnProperties skips it. On the oven-sh/bun#44305 repro (300k arrays of 16 ints, x86_64 release jsc): freeze 970 ms -> 35-47 ms, reads from the frozen arrays 9.5x plain -> 1.0-1.2x plain. Supersedes #337. The ArrayClass handling and freeze-array-prototype.js come from #622. Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
robobun
added a commit
that referenced
this pull request
Oct 1, 2026
…ArrayStorage vector Object.preventExtensions, Object.seal, Object.freeze and a read-only "length" all went through JSObject::enterDictionaryIndexingMode, which moved every element of the object into the SparseArrayValueMap (sparse mode, vector length 0). After that every a[i] read missed the JIT vector fast paths and did a hash lookup in C++, and the freeze itself cost two butterfly conversions, a map cell and one hash insert per element. Now enterDictionaryIndexingMode keeps the elements in the vector and only switches the object to the SlowPutArrayStorage shape. LLInt and the baseline JIT never store into a SlowPutArrayStorage vector inline, and DFG / FTL now check the structure before they do, so every write reaches the C++ paths. The attributes of the elements live on the Structure: vectorElementsAreNonConfigurable (Seal), vectorElementsAreReadOnly (Freeze) and arrayLengthIsReadOnly (Freeze and the new SetArrayLengthReadOnly transition, which replaces the LengthIsReadOnly flag of the sparse map). Entries of the sparse map keep their own attributes; elements only move into the map when a single index needs attributes of its own (Object.defineProperty on an element), and they carry the structure's attributes with them. A non-extensible object therefore never has the plain ArrayStorage shape (suggestedArrayStorageTransition), which is what lets the plain ArrayStorage paths stay as they were. The C++ vector writers and readers consult the bits: putByIndex, trySetIndexQuickly, putDirectIndex and the beyond-vector-length paths, deletePropertyByIndex, defineOwnIndexedProperty, getOwnPropertySlotByIndex, the prototype-chain put intercept, JSArray::setLength / pop / push and Array.prototype.reverse. An array with no elements (ArrayClass, such as Array.prototype) stays blank: the bits and isStructureExtensible() cover the later writes, so freezing Array.prototype keeps the array prototype chain watchpoint. A frozen object tells the objects that inherit from it about its read-only elements (notifyPresenceOfIndexedAccessors) when it becomes a prototype, not when it is frozen, so a frozen array that is no prototype keeps the builtin fast paths. The DFG folds a read of a frozen element from the vector as it folded one from the sparse map; JSObject::freeze fences the element stores before the structure store. ArrayAllocationProfile never records the SlowPutArrayStorage shape, so one frozen array does not make the arrays its allocation site creates next SlowPutArrayStorage. The baseline indexed load and "in" ICs accept the SlowPutArrayStorage shape, whose vector they already read. Object.isSealed / isFrozen answer from the structure for an object that JSObject::seal / freeze sealed or froze. Object.seal / Object.freeze take the JSObject::seal / freeze path for an exact JSArray and for a JSFinalObject with indexed properties instead of the generic per-index SetIntegrityLevel loop (from #337, which this supersedes); JSArray has no lazily reified own properties, so materializeLazyOwnProperties skips it. On the oven-sh/bun#44305 repro (300k arrays of 16 ints, x86_64 release jsc): freeze 970 ms -> 35-47 ms, reads from the frozen arrays 9.5x plain -> 1.0-1.2x plain. Supersedes #337. The ArrayClass handling and freeze-array-prototype.js come from #622. Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
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
1 participant
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.
Problem
objectConstructorSeal/objectConstructorFreezeonly take theJSObject::seal/JSObject::freezefast path for aJSFinalObjectwith no indexed properties. Every other receiver, including everyJSArray, falls through to the genericSetIntegrityLevelloop:On a dense N-element array that is three O(n) passes on top of the contiguous→sparse conversion, each going through
Identifierallocation and method-table dispatch.For oven-sh/bun#6360 (2000 arrays, avg 1000 elements)
Object.sealtakes ~460ms andObject.freeze~530ms vs ~3ms in V8.Change
SparseArrayValueMap::seal()/freeze(): bulk-ORDontDelete(andReadOnlyfor data entries on freeze) onto every entry's attributes, mirroringPropertyTable::seal()/freeze().JSObject::seal/freeze: afterenterDictionaryIndexingMode, apply the bulk attribute update to the sparse map;freezealso setsLengthIsReadOnlyon the map forJSArraysoisLengthWritable()reports false the same way the genericSetIntegrityLevelloop would. The early-return skips its short-circuit when indexed properties are present becauseStructure::isSealed/isFrozenonly consult the named-property table.materializeLazyOwnProperties: skipJSArray, whose only special own property islength, which is never reified onto the PropertyTable; enumerating own names there just atomizes every index for no effect.objectConstructorSeal/objectConstructorFreeze: widen the fast-path gate tois<JSFinalObject>() || isJSArray().isJSArraymatches exactArrayType, soDerivedArrayTypesubclasses and every other receiver with overridden[[DefineOwnProperty]]/[[PreventExtensions]]still take the generic observable loop.Results (x86_64, release)
2000 iterations, array size
ifilled withi:Object.seal(array)Object.freeze(array)Object.preventExtensions(array)The remaining ~90ms is
enterDictionaryIndexingModeitself (contiguous →ArrayStoragevector →SparseArrayValueMap). JSC represents non-configurable indexed elements via the sparse map and has no sealed/frozen contiguous elements kind like V8, so the conversion is still required; a further ~2x is available by folding the two conversions into one (the "horribly inefficient" note inenterDictionaryIndexingMode).Tests
JSTests/stress/object-seal-freeze-array-fast-path.jscovers: descriptor attributes after seal/freeze on dense / holey / accessor-bearing / named-prop-bearing arrays and plain objects,lengthwritability, seal-after-preventExtensions, freeze-after-seal, idempotence, strict-mode throw semantics, and that aProxytarget still observes thepreventExtensions/ownKeys/definePropertytraps.JSTests/microbenchmarks/object-seal-freeze-large-array.jsis the sealing/freezing loop for the perf dashboard.built-ins/Object/{seal,freeze,isSealed,isFrozen,preventExtensions}and the existingJSTests/stress/*seal*,*freeze*,*frozen*pass.