[JSC] A StringObject with immutable properties keeps the compilers' fast conversion to a string

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!");
+}

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());
 

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);

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();
 

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;