From e4f1d0d03cfd798a436ec249c1527be77e7c3028 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 23:07:59 +0000 Subject: [PATCH] Object.seal / Object.freeze: take the JSObject fast path for JSArray 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. --- .../object-seal-freeze-large-array.js | 10 + .../object-seal-freeze-array-fast-path.js | 217 ++++++++++++++++++ Source/JavaScriptCore/runtime/JSObject.cpp | 36 ++- .../runtime/ObjectConstructor.cpp | 17 +- .../runtime/SparseArrayValueMap.cpp | 20 ++ .../runtime/SparseArrayValueMap.h | 3 + 6 files changed, 299 insertions(+), 4 deletions(-) create mode 100644 JSTests/microbenchmarks/object-seal-freeze-large-array.js create mode 100644 JSTests/stress/object-seal-freeze-array-fast-path.js diff --git a/JSTests/microbenchmarks/object-seal-freeze-large-array.js b/JSTests/microbenchmarks/object-seal-freeze-large-array.js new file mode 100644 index 0000000000000..46be7aecf5070 --- /dev/null +++ b/JSTests/microbenchmarks/object-seal-freeze-large-array.js @@ -0,0 +1,10 @@ +for (let i = 0; i < 2_000; i++) { + let a = new Array(i); + a.fill(i); + Object.seal(a); +} +for (let i = 0; i < 2_000; i++) { + let a = new Array(i); + a.fill(i); + Object.freeze(a); +} diff --git a/JSTests/stress/object-seal-freeze-array-fast-path.js b/JSTests/stress/object-seal-freeze-array-fast-path.js new file mode 100644 index 0000000000000..ce8d8f658a336 --- /dev/null +++ b/JSTests/stress/object-seal-freeze-array-fast-path.js @@ -0,0 +1,217 @@ +// Object.seal / Object.freeze fast path on JSArray and JSFinalObject with +// indexed properties should produce the same observable state as the generic +// SetIntegrityLevel loop. + +function shouldBe(actual, expected, msg) { + if (actual !== expected) + throw new Error((msg ? msg + ": " : "") + "expected " + expected + ", got " + actual); +} +function shouldThrow(fn, msg) { + let threw = false; + try { fn(); } catch { threw = true; } + if (!threw) + throw new Error((msg ? msg + ": " : "") + "expected throw"); +} +function desc(o, k) { return Object.getOwnPropertyDescriptor(o, k); } + +// seal: dense array +{ + let a = [1, 2, 3, 4, 5]; + Object.seal(a); + shouldBe(Object.isSealed(a), true, "sealed"); + shouldBe(Object.isFrozen(a), false, "not frozen"); + shouldBe(desc(a, 0).configurable, false, "elem !configurable"); + shouldBe(desc(a, 0).writable, true, "elem writable"); + shouldBe(desc(a, 0).enumerable, true, "elem enumerable"); + shouldBe(desc(a, "length").writable, true, "length writable after seal"); + a[2] = 99; + shouldBe(a[2], 99, "write ok"); +} + +// seal after preventExtensions +{ + let a = [1, 2, 3]; + Object.preventExtensions(a); + shouldBe(Object.isSealed(a), false, "preventExtensions is not sealed"); + Object.seal(a); + shouldBe(Object.isSealed(a), true); + shouldBe(desc(a, 1).configurable, false); +} + +// seal with indexed accessor +{ + let a = [1, 2, 3]; + let v = 0; + Object.defineProperty(a, 1, { get: () => v, set: x => v = x, configurable: true }); + Object.seal(a); + shouldBe(desc(a, 1).configurable, false); + shouldBe(typeof desc(a, 1).get, "function"); + a[1] = 7; + shouldBe(v, 7, "setter still runs"); +} + +// freeze: dense array +{ + let a = [1, 2, 3, 4, 5]; + Object.freeze(a); + shouldBe(Object.isFrozen(a), true, "frozen"); + shouldBe(desc(a, 0).writable, false, "elem !writable"); + shouldBe(desc(a, 0).configurable, false, "elem !configurable"); + shouldBe(desc(a, "length").writable, false, "length !writable"); +} + +// freeze: empty array freezes length +{ + let a = []; + Object.freeze(a); + shouldBe(Object.isFrozen(a), true); + shouldBe(desc(a, "length").writable, false, "empty array length frozen"); +} + +// freeze with indexed accessor: ReadOnly is not set on accessors +{ + let a = [1, 2, 3]; + let v = 0; + Object.defineProperty(a, 1, { get: () => v, set: x => v = x, configurable: true }); + Object.freeze(a); + shouldBe(desc(a, 1).configurable, false); + shouldBe("writable" in desc(a, 1), false, "accessor has no writable"); + a[1] = 7; + shouldBe(v, 7, "setter still runs on frozen accessor"); + shouldBe(desc(a, 0).writable, false); +} + +// freeze after seal +{ + let a = [1, 2, 3]; + Object.seal(a); + Object.freeze(a); + shouldBe(Object.isFrozen(a), true); + shouldBe(desc(a, 0).writable, false); +} + +// seal/freeze holey array keeps the hole +{ + let a = [1, , 3]; + Object.seal(a); + shouldBe(1 in a, false, "hole survives seal"); + + let b = [1, , 3]; + Object.freeze(b); + shouldBe(1 in b, false, "hole survives freeze"); +} + +// JSFinalObject with indexed properties +{ + let o = { 0: "a", 1: "b", x: 1 }; + Object.seal(o); + shouldBe(Object.isSealed(o), true); + shouldBe(desc(o, 0).configurable, false); + shouldBe(desc(o, 0).writable, true); + shouldBe(desc(o, "x").configurable, false); + o[0] = "A"; + shouldBe(o[0], "A"); +} +{ + let o = { 0: "a", 1: "b", x: 1 }; + Object.freeze(o); + shouldBe(Object.isFrozen(o), true); + shouldBe(desc(o, 0).writable, false); + shouldBe(desc(o, "x").writable, false); +} + +// DerivedArrayType takes the generic loop (canFastSetIntegrityLevel only +// matches exact ArrayType). Pins the invariant: if the gate is ever widened +// to inherits(), this still has to produce the same descriptors. +{ + class A extends Array {} + let a = A.of(1, 2, 3); + Object.freeze(a); + shouldBe(Object.isFrozen(a), true, "derived frozen"); + shouldBe(desc(a, 0).writable, false, "derived elem !writable"); + shouldBe(desc(a, 0).configurable, false, "derived elem !configurable"); + shouldBe(desc(a, "length").writable, false, "derived length frozen"); +} +{ + class A extends Array {} + let a = A.of(1, 2, 3); + Object.seal(a); + shouldBe(Object.isSealed(a), true, "derived sealed"); + shouldBe(desc(a, 0).configurable, false, "derived elem !configurable"); + shouldBe(desc(a, 0).writable, true, "derived elem writable"); + shouldBe(desc(a, "length").writable, true, "derived length writable after seal"); +} + +// Proxy still sees the generic loop +{ + let calls = []; + let target = [1, 2, 3]; + let p = new Proxy(target, { + preventExtensions(t) { calls.push("pe"); return Reflect.preventExtensions(t); }, + ownKeys(t) { calls.push("ok"); return Reflect.ownKeys(t); }, + defineProperty(t, k, d) { calls.push("d:" + String(k)); return Reflect.defineProperty(t, k, d); }, + }); + Object.seal(p); + shouldBe(calls.indexOf("pe") >= 0, true, "preventExtensions trap"); + shouldBe(calls.indexOf("ok") >= 0, true, "ownKeys trap"); + shouldBe(calls.indexOf("d:0") >= 0, true, "defineProperty trap per element"); +} + +// strict-mode write/delete semantics +(function () { + "use strict"; + let a = Object.seal([1, 2, 3]); + shouldThrow(() => { delete a[0]; }, "delete on sealed elem throws"); + shouldThrow(() => { a[3] = 4; }, "add on sealed array throws"); + shouldThrow(() => { a.length = 1; }, "shrink past non-configurable elem throws"); + + let b = Object.freeze([1, 2, 3]); + shouldThrow(() => { b[0] = 9; }, "write on frozen elem throws"); + shouldThrow(() => { b.length = 0; }, "length write on frozen throws"); +})(); + +// Frozen object on the prototype chain: indexed writes through a child must +// respect the inherited ReadOnly entry. This exercises the +// notifyPresenceOfIndexedAccessors() call in the fast JSObject::freeze path. +(function () { + "use strict"; + // Array prototype frozen before becoming a prototype. + let p = Object.freeze([1, 2, 3]); + let o = Object.create(p); + shouldThrow(() => { o[0] = 99; }, "write to inherited frozen index throws (array proto)"); + shouldBe(Object.getOwnPropertyDescriptor(o, 0), undefined, "no own 0 after rejected write"); + shouldBe(o[0], 1, "inherited value intact"); + // Index past the frozen length is not inherited, so it defines an own property. + o[5] = 5; + shouldBe(o[5], 5); + + // Plain object prototype with indexed properties. + let pp = Object.freeze({ 0: "a", 1: "b" }); + let oo = Object.create(pp); + shouldThrow(() => { oo[0] = "x"; }, "write to inherited frozen index throws (object proto)"); + shouldBe(oo[0], "a"); + + // Object already used as a prototype at freeze time. + let q = [4, 5, 6]; + let child = Object.create(q); + Object.freeze(q); + shouldThrow(() => { child[1] = 0; }, "write to inherited frozen index throws (proto frozen after)"); + shouldBe(child[1], 5); + + // Seal does not affect writability, so the child write shadows the proto. + let s = Object.seal([7, 8, 9]); + let c = Object.create(s); + c[0] = 77; + shouldBe(c[0], 77, "write through sealed proto shadows"); + shouldBe(s[0], 7); +})(); + +// repeated application is idempotent +{ + let a = [1, 2, 3]; + Object.seal(a); + Object.seal(a); + Object.freeze(a); + Object.freeze(a); + shouldBe(Object.isFrozen(a), true); +} diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp index d3167e9b63310..061e76a95b0bb 100644 --- a/Source/JavaScriptCore/runtime/JSObject.cpp +++ b/Source/JavaScriptCore/runtime/JSObject.cpp @@ -2865,10 +2865,18 @@ JSString* JSObject::toString(JSGlobalObject* globalObject) const void JSObject::seal(VM& vm) { - if (isSealed(vm)) + // Structure::isSealed only consults the named-property table, so it cannot + // short-circuit when indexed properties are present. + if (!hasIndexedProperties(indexingType()) && isSealed(vm)) return; materializeLazyOwnProperties(vm); enterDictionaryIndexingMode(vm); + if (hasAnyArrayStorage(indexingType())) { + if (SparseArrayValueMap* map = butterfly()->arrayStorage()->m_sparseMap.get()) + map->seal(); + } + if (isSealed(vm)) + return; { Structure* oldStructure = structure(); DeferredStructureTransitionWatchpointFire deferred(vm, oldStructure); @@ -2878,10 +2886,28 @@ void JSObject::seal(VM& vm) void JSObject::freeze(VM& vm) { - if (isFrozen(vm)) + bool isArray = isJSArray(this); + if (!isArray && !hasIndexedProperties(indexingType()) && isFrozen(vm)) return; materializeLazyOwnProperties(vm); enterDictionaryIndexingMode(vm); + if (hasAnyArrayStorage(indexingType())) { + if (SparseArrayValueMap* map = butterfly()->arrayStorage()->m_sparseMap.get()) { + map->freeze(); + // defineOwnIndexedProperty would have called this for each ReadOnly + // index; the prototype-chain indexed-put fast paths consult the + // MayHaveIndexedAccessors bit it sets. + if (!map->isEmpty()) + notifyPresenceOfIndexedAccessors(vm); + // JSArray "length" is not in the PropertyTable; freeze it here so + // that JSArray::isLengthWritable reports false like the generic + // SetIntegrityLevel loop would. + if (isArray) + map->setLengthIsReadOnly(); + } + } + if (isFrozen(vm)) + return; { Structure* oldStructure = structure(); DeferredStructureTransitionWatchpointFire deferred(vm, oldStructure); @@ -2894,6 +2920,12 @@ void JSObject::materializeLazyOwnProperties(VM& vm) if (!structure()->typeInfo().overridesGetOwnSpecialPropertyNames()) return; + // JSArray's only special own property is "length", which is always virtual + // (never reified onto the PropertyTable) and already DontDelete; enumerating + // own names here would just atomize every index for no effect. + if (isJSArray(this)) + return; + // Force reifying lazy properties. Special properties (e.g. function "length" / "name", // or "arguments" / "caller") are materialized onto the object as a side effect of // enumerating them via getOwnPropertyNames, so the call below is what does the reification. diff --git a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp index 676609f45f725..ce1d87f4d98e1 100644 --- a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp +++ b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp @@ -1133,12 +1133,25 @@ bool testIntegrityLevel(JSGlobalObject* globalObject, VM& vm, JSObject* object) return true; } +// True when sealing / freezing |object| via the generic SetIntegrityLevel loop +// is not observable and the result is equivalent to JSObject::seal / freeze, +// which applies attributes to the PropertyTable and SparseArrayValueMap in bulk +// without allocating a property-name array or dispatching through the method +// table per element. This is the dominant cost for large arrays. +static ALWAYS_INLINE bool canFastSetIntegrityLevel(JSObject* object) +{ + // JSFinalObject and (exact) JSArray use ordinary [[PreventExtensions]] and + // [[DefineOwnProperty]] and have no lazy own properties that must be reified + // via getOwnPropertyNames. JSArray's "length" is handled separately. + return is(object) || isJSArray(object); +} + JSObject* objectConstructorSeal(JSGlobalObject* globalObject, JSObject* object) { VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - if (is(object) && !hasIndexedProperties(object->indexingType())) { + if (canFastSetIntegrityLevel(object)) { object->seal(vm); return object; } @@ -1171,7 +1184,7 @@ JSObject* objectConstructorFreeze(JSGlobalObject* globalObject, JSObject* object VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - if (is(object) && !hasIndexedProperties(object->indexingType())) { + if (canFastSetIntegrityLevel(object)) { object->freeze(vm); return object; } diff --git a/Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp b/Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp index 42591b93f88d6..1220837de9d41 100644 --- a/Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp +++ b/Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp @@ -148,6 +148,26 @@ JSValue SparseArrayValueMap::getConcurrently(unsigned i) return iterator->getConcurrently(); } +void SparseArrayValueMap::seal() +{ + for (auto it = m_set.begin(); it != m_set.end(); ++it) { + SparseArrayEntry& entry = entryFor(it); + entry.forceSet(this, entry.attributes() | static_cast(PropertyAttribute::DontDelete)); + } +} + +void SparseArrayValueMap::freeze() +{ + for (auto it = m_set.begin(); it != m_set.end(); ++it) { + SparseArrayEntry& entry = entryFor(it); + unsigned attributes = entry.attributes(); + if (attributes & PropertyAttribute::Accessor) + entry.forceSet(this, attributes | static_cast(PropertyAttribute::DontDelete)); + else + entry.forceSet(this, attributes | static_cast(PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly)); + } +} + void SparseArrayEntry::forceSet(SparseArrayValueMap* map, unsigned attributes) { // FIXME: We can expand this for non x86 environments. Currently, loading ReadOnly | DontDelete property diff --git a/Source/JavaScriptCore/runtime/SparseArrayValueMap.h b/Source/JavaScriptCore/runtime/SparseArrayValueMap.h index f30cb70aadaf5..4088cd17836ed 100644 --- a/Source/JavaScriptCore/runtime/SparseArrayValueMap.h +++ b/Source/JavaScriptCore/runtime/SparseArrayValueMap.h @@ -183,6 +183,9 @@ class SparseArrayValueMap final : public JSCell { m_flags = static_cast(m_flags | HasAnyKindOfGetterSetterProperties); } + void seal(); + void freeze(); + static SparseArrayEntry& entryFor(const_iterator it) { return const_cast(*it); } // These methods may mutate the contents of the map