Skip to content

Commit 3cc4f8b

Browse files
sosukesuzukisosukesuzuki
committed
[JSC] LLInt and Baseline JIT op_to_this should have a fast path for non-final objects
https://bugs.webkit.org/show_bug.cgi?id=327644 Reviewed by Yusuke Suzuki. In LLInt and Baseline JIT, op_to_this took the slow path on every call unless |this| was a final object with the cached structure. So a function using |this| made a C++ call each time when its receiver was an array, a function or a Map, and for any receiver once it had seen two structures. This patch adds two fast paths. 1. |this| has the cached structure, whatever its type. The slow path now caches a structure only when |this| is an object and ToThis returned it as it is. 2. The status is ToThisConflicted and |this| is an object that is not a JSScope. ToThis returns such an object as it is. This path updates the value profile since the type of |this| can change from call to call. Test: JSTests/stress/to-this-with-receivers-that-are-not-final-objects.js * JSTests/stress/to-this-with-receivers-that-are-not-final-objects.js: Added. (shouldBe): (sloppyThis): (strictThis): (sloppyType): (strictType): (fn): (gen): (callFromScope.inner): (callFromScope): (callInWith): (withObject.method): (withObject.strictMethod): (monomorphicArray): (monomorphicFunction): (named): * Source/JavaScriptCore/jit/JITOpcodes.cpp: (JSC::JIT::emit_op_to_this): * Source/JavaScriptCore/llint/LowLevelInterpreter64.asm: * Source/JavaScriptCore/runtime/CommonSlowPaths.cpp: (JSC::JSC_DEFINE_COMMON_SLOW_PATH): Canonical link: https://commits.webkit.org/323899@main
1 parent b854058 commit 3cc4f8b

4 files changed

Lines changed: 115 additions & 7 deletions

File tree

‎JSTests/stress/to-this-with-receivers-that-are-not-final-objects.js‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Semantics of |this| conversion for receivers of many kinds, monomorphic and mixed at one function.
2+
function shouldBe(actual, expected, message) {
3+
if (actual !== expected)
4+
throw new Error(message + ": expected " + String(expected) + " but got " + String(actual));
5+
}
6+
7+
function sloppyThis() { return this; }
8+
function strictThis() { "use strict"; return this; }
9+
function sloppyType() { return typeof this; }
10+
function strictType() { "use strict"; return typeof this; }
11+
noInline(sloppyThis);
12+
noInline(strictThis);
13+
noInline(sloppyType);
14+
noInline(strictType);
15+
16+
var globalThisValue = this;
17+
var array = [1, 2];
18+
var fn = function () { };
19+
var map = new Map;
20+
var plain = { a: 1 };
21+
var plain2 = { b: 1 };
22+
var symbol = Symbol("s");
23+
var date = new Date(0);
24+
var regexp = /a/;
25+
var typed = new Uint8Array(2);
26+
function* gen() { yield 1; }
27+
var generator = gen();
28+
var proxy = new Proxy({}, {});
29+
var receivers = [array, fn, map, plain, plain2, date, regexp, typed, generator, proxy];
30+
31+
function callFromScope() {
32+
// An unqualified call: the callee must not see a scope object as |this|.
33+
let captured = 1;
34+
function inner() { return captured; }
35+
return [sloppyThis(), strictThis(), inner()];
36+
}
37+
noInline(callFromScope);
38+
39+
function callInWith(object) {
40+
with (object) {
41+
return [method(), strictMethod()];
42+
}
43+
}
44+
noInline(callInWith);
45+
var withObject = {
46+
method() { return this; },
47+
strictMethod() { "use strict"; return this; },
48+
};
49+
50+
function monomorphicArray() { "use strict"; return this.length; }
51+
function monomorphicFunction() { return this.name; }
52+
function named() { }
53+
noInline(monomorphicArray);
54+
noInline(monomorphicFunction);
55+
56+
for (var i = 0; i < testLoopCount; ++i) {
57+
for (var receiver of receivers) {
58+
shouldBe(sloppyThis.call(receiver), receiver, "sloppy object receiver");
59+
shouldBe(strictThis.call(receiver), receiver, "strict object receiver");
60+
}
61+
62+
shouldBe(strictThis.call("str"), "str", "strict string");
63+
shouldBe(strictType.call("str"), "string", "strict string type");
64+
shouldBe(sloppyType.call("str"), "object", "sloppy string type");
65+
shouldBe(String(sloppyThis.call("str")), "str", "sloppy string wrapper");
66+
shouldBe(sloppyThis.call("str") instanceof String, true, "sloppy string is wrapped");
67+
68+
shouldBe(strictThis.call(symbol), symbol, "strict symbol");
69+
shouldBe(sloppyType.call(symbol), "object", "sloppy symbol type");
70+
shouldBe(strictThis.call(10n), 10n, "strict bigint");
71+
shouldBe(sloppyType.call(10n), "object", "sloppy bigint type");
72+
shouldBe(strictThis.call(1.5), 1.5, "strict number");
73+
shouldBe(sloppyType.call(1.5), "object", "sloppy number type");
74+
shouldBe(strictThis.call(true), true, "strict boolean");
75+
shouldBe(sloppyType.call(false), "object", "sloppy boolean type");
76+
77+
shouldBe(strictThis.call(undefined), undefined, "strict undefined");
78+
shouldBe(strictThis.call(null), null, "strict null");
79+
shouldBe(sloppyThis.call(undefined), globalThisValue, "sloppy undefined");
80+
shouldBe(sloppyThis.call(null), globalThisValue, "sloppy null");
81+
shouldBe(sloppyThis.call(globalThisValue), globalThisValue, "sloppy global");
82+
shouldBe(strictThis.call(globalThisValue), globalThisValue, "strict global");
83+
84+
var fromScope = callFromScope();
85+
shouldBe(fromScope[0], globalThisValue, "sloppy call from a scope");
86+
shouldBe(fromScope[1], undefined, "strict call from a scope");
87+
shouldBe(fromScope[2], 1, "inner call");
88+
89+
var fromWith = callInWith(withObject);
90+
shouldBe(fromWith[0], withObject, "sloppy call in with");
91+
shouldBe(fromWith[1], withObject, "strict call in with");
92+
93+
shouldBe(monomorphicArray.call(array), 2, "array receiver only");
94+
shouldBe(monomorphicFunction.call(named), "named", "function receiver only");
95+
}

‎Source/JavaScriptCore/jit/JITOpcodes.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,9 +1592,14 @@ void JIT::emit_op_to_this(const JSInstruction* currentInstruction)
15921592

15931593
emitJumpSlowCaseIfNotJSCell(regT0, srcDst);
15941594

1595-
addSlowCase(branchIfNotType(regT0, FinalObjectType));
15961595
load32FromMetadata(bytecode, OpToThis::Metadata::offsetOfCachedStructureID(), regT2);
1597-
addSlowCase(branch32(NotEqual, Address(regT0, JSCell::structureIDOffset()), regT2));
1596+
Jump done = branch32(Equal, Address(regT0, JSCell::structureIDOffset()), regT2);
1597+
load32FromMetadata(bytecode, OpToThis::Metadata::offsetOfToThisStatus(), regT2);
1598+
addSlowCase(branch32(NotEqual, regT2, TrustedImm32(ToThisConflicted)));
1599+
addSlowCase(branchIfNotObject(regT0));
1600+
addSlowCase(branchIfType(regT0, JSTypeRange { JSType(FirstScopeType), JSType(LastScopeType) }));
1601+
emitValueProfilingSite(bytecode, regT0);
1602+
done.link(this);
15981603
}
15991604

16001605
void JIT::emit_op_create_this(const JSInstruction* currentInstruction)

‎Source/JavaScriptCore/llint/LowLevelInterpreter64.asm‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -899,11 +899,19 @@ llintOpWithMetadata(op_to_this, OpToThis, macro (size, get, dispatch, metadata,
899899
get(m_srcDst, t0)
900900
loadq [cfr, t0, 8], t0
901901
btqnz t0, notCellMask, .opToThisSlow
902-
bbneq JSCell::m_type[t0], FinalObjectType, .opToThisSlow
903902
loadi JSCell::m_structureID[t0], t1
904903
metadata(t2, t3)
905-
loadi OpToThis::Metadata::m_cachedStructureID[t2], t2
906-
bineq t1, t2, .opToThisSlow
904+
loadi OpToThis::Metadata::m_cachedStructureID[t2], t3
905+
bineq t1, t3, .opToThisPolymorphic
906+
dispatch()
907+
908+
.opToThisPolymorphic:
909+
bineq OpToThis::Metadata::m_toThisStatus[t2], constexpr ToThisConflicted, .opToThisSlow
910+
loadb JSCell::m_type[t0], t1
911+
bib t1, ObjectType, .opToThisSlow
912+
subi constexpr FirstScopeType, t1
913+
bibeq t1, constexpr (LastScopeType - FirstScopeType), .opToThisSlow
914+
valueProfile(size, OpToThis, m_valueProfile, t0, t1)
907915
dispatch()
908916

909917
.opToThisSlow:

‎Source/JavaScriptCore/runtime/CommonSlowPaths.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,8 @@ JSC_DEFINE_COMMON_SLOW_PATH(slow_path_to_this)
288288
auto bytecode = pc->as<OpToThis>();
289289
auto& metadata = bytecode.metadata(codeBlock);
290290
JSValue v1 = GET(bytecode.m_srcDst).jsValue();
291-
if (v1.isCell()) {
291+
auto value = v1.toThis(globalObject, bytecode.m_ecmaMode);
292+
if (v1.isObject() && value == v1) {
292293
StructureID myStructureID = v1.asCell()->structureID();
293294
StructureID otherStructureID = metadata.m_cachedStructureID;
294295
if (myStructureID != otherStructureID) {
@@ -307,7 +308,6 @@ JSC_DEFINE_COMMON_SLOW_PATH(slow_path_to_this)
307308
// different object that still has the same structure on the fast path since it'll produce
308309
// the same SpeculatedType. Therefore, we don't need to worry about value profiling on the
309310
// fast path.
310-
auto value = v1.toThis(globalObject, bytecode.m_ecmaMode);
311311
RETURN_WITH_PROFILING_CUSTOM(bytecode.m_srcDst, value, PROFILE_VALUE(value));
312312
}
313313

0 commit comments

Comments
 (0)