diff --git a/JSTests/stress/immutable-properties-string-object.js b/JSTests/stress/immutable-properties-string-object.js new file mode 100644 index 0000000000000..15d8579860e7e --- /dev/null +++ b/JSTests/stress/immutable-properties-string-object.js @@ -0,0 +1,178 @@ +// The optimizing compilers turn a StringObject into its string without a call where they can check that nobody has touched the object, +// which they do by its structure. A StringObject whose properties were made immutable passes that check too: the operations below +// give the same results for it, and a site that sees one is not reoptimized for it. + +function shouldBe(actual, expected, message) { + if (actual !== expected) + throw new Error((message ? message + ": " : "") + "expected " + String(expected) + " but got " + String(actual)); +} + +// Enough calls for a site to be compiled, and then to exit often enough to be reoptimized if it is going to be. +let hot = Math.ceil(testLoopCount / 4); + +let operations = { + add: "object + suffix", + addLeft: "suffix + object", + template: "`${object}${suffix}`", + stringCall: "String(object) + suffix", + toStringCall: "object.toString() + suffix", + valueOfCall: "object.valueOf() + suffix", + charAt: "object.charAt(1) + suffix", + index: "object[1] + suffix", + length: "object.length + suffix", + equals: "(object == 'abc') + suffix", + concat: "object.concat(suffix)", +}; + +// (Functions with the same source share what the compilers have learnt about it, exits included: each site gets a source of its own.) +let sites = 0; +function makeSite(source) { + let site = new Function("object", "suffix", "return " + source + "; // site " + ++sites); + noInline(site); + return site; +} + +function run(site, pick, expected, label) { + for (let i = 0; i < hot; ++i) + shouldBe(site(pick(i), "-" + (i & 3)), expected(i), label); +} + +// The realm's first such object appears when sites are hot already. Their code is thrown away, once, and no exit is held against them. +{ + let hotSites = Object.values(operations).map(makeSite), ordinary = new String("abc"); + let results = hotSites.map(site => { run(site, () => ordinary, i => site(ordinary, "-" + (i & 3))); return site(ordinary, "!"); }); + let immutable = $vm.makePropertiesImmutable(new String("abc")); + hotSites.forEach((site, index) => { + shouldBe(site(immutable, "!"), results[index]); + run(site, i => i & 1 ? immutable : ordinary, i => site(ordinary, "-" + (i & 3))); + }); +} + +// A call of a method looks the method up by the object's structure first, and a site learns a second structure as it does for any +// other object, by being reoptimized if it was compiled too early. Those are here for their results. +let looksUpAMethod = new Set(["toStringCall", "valueOfCall", "charAt", "concat"]); + +for (let [name, source] of Object.entries(operations)) { + let reference = makeSite(source); + let expected = i => reference(new String("abc"), "-" + (i & 3)); + + // 1. The site is hot on ordinary objects, and then sees objects with immutable properties. + let later = makeSite(source), ordinary = new String("abc"); + run(later, () => ordinary, expected, name + ", ordinary"); + let immutable = $vm.makePropertiesImmutable(new String("abc")); + run(later, () => immutable, expected, name + ", immutable"); + run(later, i => i & 1 ? immutable : ordinary, expected, name + ", both"); + + // 2. The site sees both from the start. + let fromTheStart = makeSite(source); + run(fromTheStart, i => i & 1 ? immutable : ordinary, expected, name + ", both from the start"); + + // 3. The same number of calls with ordinary objects alone: whatever reoptimization this configuration causes by itself. + let control = makeSite(source); + for (let round = 0; round < 3; ++round) + run(control, () => ordinary, expected, name + ", control"); + + if (!looksUpAMethod.has(name)) { + shouldBe(reoptimizationRetryCount(later) <= reoptimizationRetryCount(control), true, name + ": reoptimized for an object with immutable properties, which it saw later"); + shouldBe(reoptimizationRetryCount(fromTheStart) <= reoptimizationRetryCount(control), true, name + ": reoptimized for an object with immutable properties"); + } + shouldBe($vm.hasImmutableProperties(immutable), true); +} + +// What the check is for still holds. An object that was touched first does not pass it, immutable properties or not. +{ + let site = makeSite("object + suffix"); + let ordinary = new String("abc"); + let withOwnToString = new String("abc"); + withOwnToString.toString = () => "own toString"; + withOwnToString.valueOf = () => "own valueOf"; + $vm.makePropertiesImmutable(withOwnToString); + let withOtherPrototype = Object.setPrototypeOf(new String("abc"), { __proto__: String.prototype, valueOf() { return "inherited valueOf"; } }); + $vm.makePropertiesImmutable(withOtherPrototype); + class Derived extends String { valueOf() { return "derived valueOf"; } } + let derived = $vm.makePropertiesImmutable(new Derived("abc")); + for (let i = 0; i < hot; ++i) { + shouldBe(site(ordinary, "!"), "abc!"); + shouldBe(site(withOwnToString, "!"), "own valueOf!"); + shouldBe(site(withOtherPrototype, "!"), "inherited valueOf!"); + shouldBe(site(derived, "!"), "derived valueOf!"); + } +} + +// Each realm has its own. +{ + let other = $vm.createGlobalObject(); + let site = makeSite("object + suffix"); + let objects = [new String("abc"), $vm.makePropertiesImmutable(new String("abc")), new other.String("abc"), $vm.makePropertiesImmutable(new other.String("abc"))]; + for (let i = 0; i < hot; ++i) + shouldBe(site(objects[i & 3], "!"), "abc!"); +} + +// The object comes from an inlined function of another realm: it is that realm's String.prototype that counts, which the site, in this +// realm, does not watch. Such an object, immutable properties or not, is not taken for one of this realm's. (What produces the object +// has to be in the other realm's code once that is inlined: a call that is not inlined itself, and a load from an object.) +for (let change of [object => object, object => $vm.makePropertiesImmutable(object)]) { + noInline(change); + let other = $vm.createGlobalObject(); + change(new other.String("abc")); + let holder = { object: change(new other.String("abc")) }; + let fresh = other.Function("change", "return change(new String('abc')); // site " + ++sites); + let existing = other.Function("holder", "return holder.object; // site " + ++sites); + let addFresh = new Function("make", "change", "suffix", "return make(change) + suffix; // site " + ++sites); + let addExisting = new Function("make", "holder", "suffix", "return make(holder) + suffix; // site " + ++sites); + noInline(addFresh); + noInline(addExisting); + for (let i = 0; i < hot; ++i) { + shouldBe(addFresh(fresh, change, "!"), "abc!"); + shouldBe(addExisting(existing, holder, "!"), "abc!"); + } + other.String.prototype.valueOf = other.Function("return 'replaced';"); + for (let i = 0; i < hot; ++i) { + shouldBe(addFresh(fresh, change, "!"), "replaced!"); + shouldBe(addExisting(existing, holder, "!"), "replaced!"); + } +} + +// The structure is kept. A realm's only such object is gone, and what a collected Structure would have left is taken by others: a new +// such object gets the structure that compiled code checks for. +{ + let other = $vm.createGlobalObject(); + let makeAndDrop = () => { $vm.makePropertiesImmutable(new other.String("abc")); }; + let overwriteTheStack = depth => depth ? overwriteTheStack(depth - 1) + 1 : 0; + noInline(makeAndDrop); + noInline(overwriteTheStack); + makeAndDrop(); + overwriteTheStack(200); + let shapes = []; + for (let round = 0; round < 3; ++round) { + fullGC(); + for (let i = 0; i < 500; ++i) + shapes.push({ ["property" + round + "_" + i]: i }); + } + let makeSiteThere = () => { + let site = other.Function("object", "suffix", "return object + suffix; // site " + ++sites); + noInline(site); + return site; + }; + let site = makeSiteThere(), control = makeSiteThere(); + for (let i = 0; i < hot * 3; ++i) { + shouldBe(site($vm.makePropertiesImmutable(new other.String("abc")), "!"), "abc!"); + shouldBe(control(new other.String("abc"), "!"), "abc!"); + } + shouldBe(reoptimizationRetryCount(site) <= reoptimizationRetryCount(control), true, "reoptimized after a collection"); +} + +// A change to String.prototype reaches such an object, hot, as it reaches any other. (Last: no site of this realm that is compiled after +// this gets the fast conversion, whatever is put back.) +{ + let site = makeSite("object + suffix"); + let immutable = $vm.makePropertiesImmutable(new String("abc")); + for (let i = 0; i < hot; ++i) + shouldBe(site(immutable, "!"), "abc!"); + let valueOf = String.prototype.valueOf; + String.prototype.valueOf = function () { return "replaced"; }; + for (let i = 0; i < hot; ++i) + shouldBe(site(immutable, "!"), "replaced!"); + String.prototype.valueOf = valueOf; + shouldBe(site(immutable, "!"), "abc!"); +} diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp index f4ac7c3b620f7..4e71d048eba84 100644 --- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp @@ -4006,8 +4006,19 @@ class FixupPhase : public Phase { // The realm has to be the one canOptimizeStringObjectAccess() was asked about, which is the one whose String.prototype this code // watches. The child's own origin can be in another realm, when it comes from an inlined function. + JSGlobalObject* globalObject = m_graph.globalObjectFor(origin.semantic); StructureSet set; - set.add(m_graph.globalObjectFor(origin.semantic)->stringObjectStructure()); + set.add(globalObject->stringObjectStructure()); + // A StringObject with that structure whose properties were made immutable has no more properties of its own, and the same + // prototype. While no such structure exists, the check is for the one structure, and this code goes when one appears. + InlineWatchpointSet& immutablePropertiesWatchpointSet = globalObject->stringObjectImmutablePropertiesWatchpointSet(); + if (immutablePropertiesWatchpointSet.isStillValid()) { + m_graph.freeze(globalObject); + m_graph.watchpoints().addLazily(immutablePropertiesWatchpointSet); + } else { + WTF::loadLoadFence(); + set.add(globalObject->stringObjectStructureWithImmutableProperties()); + } if (useKind == StringOrStringObjectUse) set.add(vm().stringStructure.get()); diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp index d4cadc8068ae3..fd318e45599b2 100644 --- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp +++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp @@ -2879,6 +2879,18 @@ void JSGlobalObject::clearStructureCache(VM& vm) m_structureCacheClearedWatchpointSet.fireAll(vm, "Clearing StructureCache"); } +// Compiled code recognizes an object nobody has touched by one of this realm's original structures. The structure such an object gets +// when its properties are made immutable says as much about it, so it is kept here, alive, for the compilers to accept as well. +// Code that was compiled when there was none is thrown away, which, unlike an exit, does not count against the code's site. +void JSGlobalObject::didMakePropertiesImmutable(VM& vm, Structure* oldStructure, Structure* newStructure) +{ + if (oldStructure != stringObjectStructure() || m_stringObjectStructureWithImmutableProperties) + return; + m_stringObjectStructureWithImmutableProperties.set(vm, this, newStructure); + WTF::storeStoreFence(); + m_stringObjectImmutablePropertiesWatchpointSet.fireAll(vm, "A StringObject's properties were made immutable"); +} + void JSGlobalObject::haveABadTime(VM& vm) { ASSERT(&vm == &this->vm()); @@ -3203,6 +3215,7 @@ void JSGlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) thisObject->m_promiseCapabilityObjectStructure.visit(visitor); thisObject->m_promiseAllSettledFulfilledResultStructure.visit(visitor); thisObject->m_promiseAllSettledRejectedResultStructure.visit(visitor); + visitor.append(thisObject->m_stringObjectStructureWithImmutableProperties); visitor.append(thisObject->m_regExpMatchesArrayStructure); visitor.append(thisObject->m_regExpMatchesArrayWithIndicesStructure); visitor.append(thisObject->m_regExpMatchesIndicesArrayStructure); diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h index f8d47bd27003e..57a40935e8341 100644 --- a/Source/JavaScriptCore/runtime/JSGlobalObject.h +++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h @@ -438,6 +438,7 @@ class JSGlobalObject : public JSSegmentedVariableObject { WriteBarrierStructureID m_mapIteratorStructure; WriteBarrierStructureID m_setIteratorStructure; WriteBarrierStructureID m_wrapForValidIteratorStructure; + WriteBarrierStructureID m_stringObjectStructureWithImmutableProperties; WriteBarrierStructureID m_regExpMatchesArrayStructure; WriteBarrierStructureID m_regExpMatchesArrayWithIndicesStructure; WriteBarrierStructureID m_regExpMatchesIndicesArrayStructure; @@ -584,6 +585,7 @@ class JSGlobalObject : public JSSegmentedVariableObject { InlineWatchpointSet m_numberToStringWatchpointSet { IsWatched }; InlineWatchpointSet m_stringToStringWatchpointSet { IsWatched }; InlineWatchpointSet m_stringValueOfWatchpointSet { IsWatched }; + InlineWatchpointSet m_stringObjectImmutablePropertiesWatchpointSet { IsWatched }; InlineWatchpointSet m_objectPrototypeValueOfWatchpointSet { IsWatched }; InlineWatchpointSet m_arrayPrototypeValueOfWatchpointSet { IsWatched }; InlineWatchpointSet m_structureCacheClearedWatchpointSet { IsWatched }; @@ -646,6 +648,8 @@ class JSGlobalObject : public JSSegmentedVariableObject { InlineWatchpointSet& arraySymbolToPrimitiveWatchpointSet() LIFETIME_BOUND { return m_arraySymbolToPrimitiveWatchpointSet; } InlineWatchpointSet& stringToStringWatchpointSet() LIFETIME_BOUND { return m_stringToStringWatchpointSet; } InlineWatchpointSet& stringValueOfWatchpointSet() LIFETIME_BOUND { return m_stringValueOfWatchpointSet; } + // Valid until stringObjectStructureWithImmutableProperties() is set. + InlineWatchpointSet& stringObjectImmutablePropertiesWatchpointSet() LIFETIME_BOUND { return m_stringObjectImmutablePropertiesWatchpointSet; } InlineWatchpointSet& objectPrototypeValueOfWatchpointSet() LIFETIME_BOUND { return m_objectPrototypeValueOfWatchpointSet; } InlineWatchpointSet& arrayPrototypeValueOfWatchpointSet() LIFETIME_BOUND { return m_arrayPrototypeValueOfWatchpointSet; } InlineWatchpointSet& regExpPrimordialPropertiesWatchpointSet() LIFETIME_BOUND { return m_regExpPrimordialPropertiesWatchpointSet; } @@ -1062,6 +1066,8 @@ class JSGlobalObject : public JSSegmentedVariableObject { Structure* setIteratorStructure() const { return m_setIteratorStructure.get(); } Structure* wrapForValidIteratorStructure() const { return m_wrapForValidIteratorStructure.get(); } Structure* stringObjectStructure() const { return m_stringObjectStructure.get(); } + // What JSObject::makePropertiesImmutable() makes of stringObjectStructure(). Null until it has: see didMakePropertiesImmutable(). + Structure* stringObjectStructureWithImmutableProperties() const { return m_stringObjectStructureWithImmutableProperties.get(); } Structure* symbolObjectStructure() const { return m_symbolObjectStructure.get(); } Structure* iteratorResultObjectStructure() const { return m_iteratorResultObjectStructure.get(this); } Structure* iteratorResultObjectStructureConcurrently() const { return m_iteratorResultObjectStructure.getConcurrently(); } @@ -1263,6 +1269,7 @@ class JSGlobalObject : public JSSegmentedVariableObject { } void haveABadTime(VM&); + void didMakePropertiesImmutable(VM&, Structure* oldStructure, Structure* newStructure); void notifyArrayBufferDetaching(); diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp index d328710aa3498..62fe531a38ddf 100644 --- a/Source/JavaScriptCore/runtime/JSObject.cpp +++ b/Source/JavaScriptCore/runtime/JSObject.cpp @@ -3133,6 +3133,8 @@ bool JSObject::makePropertiesImmutable(VM& vm) if (copyOnWriteButterfly) nukeStructureAndSetButterfly(vm, oldStructureID, copyOnWriteButterfly->toButterfly()); setStructure(vm, newStructure); + if (JSGlobalObject* realm = oldStructure->realm()) + realm->didMakePropertiesImmutable(vm, oldStructure, newStructure); if (mayBePrototype()) [[unlikely]] vm.invalidateStructureChainIntegrity(VM::StructureChainIntegrityEvent::Change); return true;