Skip to content

Commit 752e7e8

Browse files
committed
[JSC] Convert HasOwnProperty on the current for-in property name to EnumeratorHasOwnProperty
https://bugs.webkit.org/show_bug.cgi?id=318528 Reviewed by Yusuke Suzuki. Object.prototype.hasOwnProperty.call(o, p) is a common guard inside `for (p in o)` (eslint's no-prototype-builtins rewrites o.hasOwnProperty(p) into this form). However, op_enumerator_has_own_property is not emitted for it: the recognition happens only in the parser, which matches the exact syntactic form `o.hasOwnProperty(p)`, so the .call spelling (and Object.hasOwn) falls back to the generic HasOwnProperty node that hashes the key on every iteration. This patch additionally converts HasOwnProperty to EnumeratorHasOwnProperty in DFG strength reduction, when the key is the EnumeratorNextUpdatePropertyName of an enumeration whose GetPropertyEnumerator base is the same node as the object. This makes the guard a structure ID comparison regardless of spelling. Baseline Patched object-has-own-for-in-loop 4.7828+-0.0548 ^ 4.0638+-0.0909 ^ definitely 1.1769x faster has-own-property-call-for-in-loop 12.0470+-0.2778 ^ 9.5166+-0.2442 ^ definitely 1.2659x faster Tests: JSTests/microbenchmarks/has-own-property-call-for-in-loop.js JSTests/stress/for-in-has-own-property-call-edge-cases.js JSTests/stress/for-in-has-own-property-call.js * JSTests/microbenchmarks/has-own-property-call-for-in-loop.js: Added. (assert): (test1.count): (test1): * JSTests/stress/for-in-has-own-property-call-edge-cases.js: Added. (assert): (oracle): (deleteCurrent): (deleteCurrentHasOwn): (clobber): (guardAfterClobber): (differential): (reassign): (nested): (proxyGuard): (proxyThrow): (polyBase): (makeLarge): (i.assert.guardAfterClobber): (i.assert): (i.p.string_appeared_here.Object.setPrototypeOf): (i.assert.nested): (i.assert.polyBase): (i.assert.polyBase.Object.freeze): * JSTests/stress/for-in-has-own-property-call.js: Added. (assert): (countOwn): (countHasOwn): (crossCheck): (deleteDuring): (countIndexed): * Source/JavaScriptCore/dfg/DFGNode.cpp: (JSC::DFG::Node::convertToEnumeratorHasOwnProperty): * Source/JavaScriptCore/dfg/DFGNode.h: * Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp: (JSC::DFG::StrengthReductionPhase::handleNode): Canonical link: https://commits.webkit.org/316538@main
1 parent c1dbbbe commit 752e7e8

6 files changed

Lines changed: 393 additions & 0 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
function assert(b) {
2+
if (!b)
3+
throw new Error;
4+
}
5+
6+
function test1() {
7+
function count(o) {
8+
let c = 0;
9+
for (let p in o) {
10+
if (Object.prototype.hasOwnProperty.call(o, p))
11+
c += p.length;
12+
}
13+
return c;
14+
}
15+
noInline(count);
16+
17+
const keys = ['method', 'url', 'status', 'headers', 'body', 'retries', 'timeout', 'ok'];
18+
let srcs = [];
19+
for (let j = 0; j < 16; j++) {
20+
let o = {};
21+
for (let k of keys)
22+
o[k] = k + j;
23+
srcs.push(o);
24+
}
25+
26+
let expected = 0;
27+
for (let k of keys)
28+
expected += k.length;
29+
30+
for (let i = 0; i < 1000000; ++i)
31+
assert(count(srcs[i & 15]) === expected);
32+
}
33+
test1();
Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
function assert(b, m) {
2+
if (!b)
3+
throw new Error(m);
4+
}
5+
6+
const hop = Object.prototype.hasOwnProperty;
7+
8+
function oracle(o, p) {
9+
return Reflect.getOwnPropertyDescriptor(Object(o), p) !== undefined;
10+
}
11+
noInline(oracle);
12+
13+
// The guard must observe a delete of the current key.
14+
function deleteCurrent(o) {
15+
const seen = [];
16+
for (const p in o) {
17+
delete o[p];
18+
seen.push(p + ':' + Object.prototype.hasOwnProperty.call(o, p));
19+
}
20+
return seen.join(',');
21+
}
22+
noInline(deleteCurrent);
23+
24+
function deleteCurrentHasOwn(o) {
25+
const seen = [];
26+
for (const p in o) {
27+
delete o[p];
28+
seen.push(p + ':' + Object.hasOwn(o, p));
29+
}
30+
return seen.join(',');
31+
}
32+
noInline(deleteCurrentHasOwn);
33+
34+
// A call that clobbers the world between the property name production and the guard.
35+
let clobberTarget = null;
36+
let clobberKey = '';
37+
function clobber() {
38+
if (clobberTarget)
39+
delete clobberTarget[clobberKey];
40+
}
41+
noInline(clobber);
42+
43+
function guardAfterClobber(o, victim) {
44+
const seen = [];
45+
for (const p in o) {
46+
clobberTarget = p === victim ? o : null;
47+
clobberKey = p;
48+
clobber();
49+
seen.push(p + ':' + hop.call(o, p));
50+
}
51+
clobberTarget = null;
52+
return seen.join(',');
53+
}
54+
noInline(guardAfterClobber);
55+
56+
// The guard must always agree with Reflect.getOwnPropertyDescriptor, even while
57+
// the mutator churns structures, prototypes, and upcoming keys.
58+
function differential(o, mutate) {
59+
let ones = 0;
60+
for (const p in o) {
61+
if (mutate)
62+
mutate(o, p);
63+
const got = Object.prototype.hasOwnProperty.call(o, p);
64+
assert(got === oracle(o, p), "differential mismatch for " + p);
65+
if (got)
66+
ones++;
67+
}
68+
return ones;
69+
}
70+
noInline(differential);
71+
72+
// Loop variable reassignment: q keeps the enumerated name, p does not.
73+
function reassign(o) {
74+
let n = 0;
75+
for (let p in o) {
76+
const q = p;
77+
p = 'not a property';
78+
assert(!hop.call(o, p), "reassigned key must miss");
79+
if (hop.call(o, q))
80+
n++;
81+
}
82+
return n;
83+
}
84+
noInline(reassign);
85+
86+
// Nested enumerations over the same object, guarding both loop variables.
87+
function nested(o) {
88+
let n = 0;
89+
for (const p in o) {
90+
for (const q in o) {
91+
if (hop.call(o, p))
92+
n++;
93+
if (Object.hasOwn(o, q))
94+
n += 10;
95+
}
96+
}
97+
return n;
98+
}
99+
noInline(nested);
100+
101+
// The guard on a proxy must consult the getOwnPropertyDescriptor trap every time.
102+
function proxyGuard(o, counter) {
103+
let n = 0;
104+
for (const p in o) {
105+
const before = counter.count;
106+
if (hop.call(o, p))
107+
n++;
108+
assert(counter.count > before, "guard must hit the proxy trap");
109+
}
110+
return n;
111+
}
112+
noInline(proxyGuard);
113+
114+
// A trap that starts throwing mid-enumeration must surface the exception.
115+
function proxyThrow(o, state) {
116+
const seen = [];
117+
for (const p in o) {
118+
seen.push(p + ':' + hop.call(o, p));
119+
state.throwing = true;
120+
}
121+
return seen.join(',');
122+
}
123+
noInline(proxyThrow);
124+
125+
// Polymorphic bases: plain, null-proto, frozen, large, array, string primitive.
126+
function polyBase(o) {
127+
let n = 0;
128+
for (const p in o) {
129+
if (Object.prototype.hasOwnProperty.call(o, p))
130+
n++;
131+
}
132+
return n;
133+
}
134+
noInline(polyBase);
135+
136+
const protoBase = { pa: 1, pb: 2 };
137+
138+
function makeLarge() {
139+
const o = {};
140+
for (let i = 0; i < 40; ++i)
141+
o['k' + i] = i;
142+
return o;
143+
}
144+
145+
const largeTemplate = makeLarge();
146+
147+
for (let i = 0; i < testLoopCount; ++i) {
148+
assert(deleteCurrent({ a: 1, b: 2, c: 3 }) === 'a:false,b:false,c:false', "deleteCurrent");
149+
assert(deleteCurrentHasOwn({ a: 1, b: 2, c: 3 }) === 'a:false,b:false,c:false', "deleteCurrentHasOwn");
150+
151+
assert(guardAfterClobber({ a: 1, b: 2, c: 3 }, 'b') === 'a:true,b:false,c:true', "guardAfterClobber");
152+
153+
{
154+
const o = Object.create(protoBase);
155+
o.a = 1;
156+
o.b = 2;
157+
assert(differential(o, null) === 2, "differential proto");
158+
}
159+
{
160+
const o = { a: 1, b: 2, c: 3 };
161+
assert(differential(o, (t, p) => { if (p === 'a') delete t.c; }) === 2, "differential delete upcoming");
162+
}
163+
{
164+
const o = { a: 1, b: 2 };
165+
differential(o, (t, p) => { t['n' + p] = 1; });
166+
}
167+
{
168+
const o = Object.create(protoBase);
169+
o.a = 1;
170+
differential(o, (t, p) => { if (p === 'a') Object.setPrototypeOf(t, { zz: 1 }); });
171+
}
172+
{
173+
const o = { a: 1, b: 2, c: 3, d: 4 };
174+
delete o.b;
175+
o['k' + (i & 7)] = 1;
176+
assert(differential(o, null) === 4, "differential dictionary");
177+
}
178+
179+
assert(reassign({ a: 1, b: 2, c: 3 }) === 3, "reassign");
180+
assert(nested({ a: 1, b: 2, c: 3 }) === 99, "nested");
181+
182+
{
183+
const counter = { count: 0 };
184+
const proxy = new Proxy({ a: 1, b: 2 }, {
185+
getOwnPropertyDescriptor(t, k) {
186+
counter.count++;
187+
return Reflect.getOwnPropertyDescriptor(t, k);
188+
}
189+
});
190+
assert(proxyGuard(proxy, counter) === 2, "proxyGuard");
191+
}
192+
{
193+
const state = { throwing: false };
194+
const proxy = new Proxy({ a: 1, b: 2 }, {
195+
getOwnPropertyDescriptor(t, k) {
196+
if (state.throwing)
197+
throw new Error('trap');
198+
return Reflect.getOwnPropertyDescriptor(t, k);
199+
}
200+
});
201+
let threw = false;
202+
let result = null;
203+
try {
204+
result = proxyThrow(proxy, state);
205+
} catch (e) {
206+
threw = true;
207+
assert(e.message === 'trap', "proxyThrow message");
208+
}
209+
assert(threw || result === 'a:true,b:true', "proxyThrow " + result);
210+
}
211+
212+
assert(polyBase({ a: 1, b: 2, c: 3 }) === 3, "polyBase object");
213+
assert(polyBase("abcd") === 4, "polyBase string");
214+
{
215+
const arr = [10, 20, 30];
216+
arr.named = 1;
217+
delete arr[1];
218+
assert(polyBase(arr) === 3, "polyBase array");
219+
}
220+
{
221+
const o = Object.create(null);
222+
o.x = 1;
223+
o.y = 2;
224+
assert(polyBase(o) === 2, "polyBase null proto");
225+
}
226+
assert(polyBase(Object.freeze({ f1: 1, f2: 2 })) === 2, "polyBase frozen");
227+
{
228+
const o = { ...largeTemplate };
229+
assert(polyBase(o) === 40, "polyBase large");
230+
}
231+
{
232+
const o = Object.create(protoBase);
233+
o.own = 1;
234+
assert(polyBase(o) === 1, "polyBase inherited");
235+
}
236+
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
function assert(b, m) {
2+
if (!b)
3+
throw new Error(m);
4+
}
5+
6+
const hop = Object.prototype.hasOwnProperty;
7+
8+
function countOwn(o) {
9+
let own = 0;
10+
let inherited = 0;
11+
for (const p in o) {
12+
if (Object.prototype.hasOwnProperty.call(o, p))
13+
own++;
14+
else
15+
inherited++;
16+
}
17+
return [own, inherited];
18+
}
19+
noInline(countOwn);
20+
21+
function countHasOwn(o) {
22+
let own = 0;
23+
for (const p in o) {
24+
if (Object.hasOwn(o, p))
25+
own++;
26+
}
27+
return own;
28+
}
29+
noInline(countHasOwn);
30+
31+
function crossCheck(o, other) {
32+
let n = 0;
33+
for (const p in o) {
34+
if (hop.call(other, p))
35+
n++;
36+
}
37+
return n;
38+
}
39+
noInline(crossCheck);
40+
41+
function deleteDuring(o, victim) {
42+
const seen = [];
43+
for (const p in o) {
44+
if (p === 'a')
45+
delete o[victim];
46+
seen.push(p + ':' + hop.call(o, p));
47+
}
48+
return seen.join(',');
49+
}
50+
noInline(deleteDuring);
51+
52+
function countIndexed(o) {
53+
let own = 0;
54+
for (const p in o) {
55+
if (hop.call(o, p))
56+
own++;
57+
}
58+
return own;
59+
}
60+
noInline(countIndexed);
61+
62+
const proto = { pa: 1, pb: 2 };
63+
64+
for (let i = 0; i < testLoopCount; ++i) {
65+
const o = Object.create(proto);
66+
o.a = 1;
67+
o.b = 2;
68+
o.c = 3;
69+
const [own, inherited] = countOwn(o);
70+
assert(own === 3, "own " + own);
71+
assert(inherited === 2, "inherited " + inherited);
72+
73+
const shadow = Object.create(proto);
74+
shadow.pa = 9;
75+
shadow.x = 1;
76+
const [sOwn, sInherited] = countOwn(shadow);
77+
assert(sOwn === 2, "shadow own " + sOwn);
78+
assert(sInherited === 1, "shadow inherited " + sInherited);
79+
80+
const h = Object.create(proto);
81+
h.a = 1;
82+
h.b = 2;
83+
assert(countHasOwn(h) === 2, "hasOwn");
84+
85+
assert(crossCheck({ a: 1, b: 2, c: 3 }, { a: 1, c: 3 }) === 2, "cross");
86+
87+
const d = { a: 1, b: 2, c: 3 };
88+
const r = deleteDuring(d, 'c');
89+
assert(r === 'a:true,b:true' || r === 'a:true,b:true,c:false', "delete " + r);
90+
91+
const arr = [10, 20, 30];
92+
arr.named = 1;
93+
assert(countIndexed(arr) === 4, "indexed");
94+
}

Source/JavaScriptCore/dfg/DFGNode.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,22 @@ void Node::convertToDefineAccessorProperty(Graph& graph, Edge base, Edge propert
422422
children = AdjacencyList(AdjacencyList::Variable, firstChild, 5);
423423
}
424424

425+
void Node::convertToEnumeratorHasOwnProperty(Graph& graph, Edge base, Edge propertyName, Edge index, Edge mode, Edge enumerator, ArrayMode arrayMode, unsigned enumeratorMetadata)
426+
{
427+
ASSERT(op() == HasOwnProperty);
428+
setOpAndDefaultFlags(EnumeratorHasOwnProperty);
429+
m_opInfo = arrayMode.asWord();
430+
m_opInfo2 = enumeratorMetadata;
431+
432+
unsigned firstChild = graph.m_varArgChildren.size();
433+
graph.m_varArgChildren.append(base);
434+
graph.m_varArgChildren.append(propertyName);
435+
graph.m_varArgChildren.append(index);
436+
graph.m_varArgChildren.append(mode);
437+
graph.m_varArgChildren.append(enumerator);
438+
children = AdjacencyList(AdjacencyList::Variable, firstChild, 5);
439+
}
440+
425441
void Node::convertToObjectDefinePropertyFromFields(Graph& graph, Edge target, Edge key, Edge enumerable, Edge configurable, Edge value, Edge writable, Edge getter, Edge setter)
426442
{
427443
ASSERT(op() == ObjectDefineProperty);

Source/JavaScriptCore/dfg/DFGNode.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,7 @@ struct Node {
986986
void convertToDefineAccessorProperty(Graph&, Edge base, Edge property, Edge getter, Edge setter, Edge attributes);
987987
void convertToObjectDefinePropertyFromFields(Graph&, Edge target, Edge key, Edge enumerable, Edge configurable, Edge value, Edge writable, Edge getter, Edge setter);
988988
void convertToPutByIdDirect(Graph&, Edge base, Edge value, CacheableIdentifier, ECMAMode);
989+
void convertToEnumeratorHasOwnProperty(Graph&, Edge base, Edge propertyName, Edge index, Edge mode, Edge enumerator, ArrayMode, unsigned enumeratorMetadata);
989990

990991
void convertToSetRegExpObjectLastIndex()
991992
{

0 commit comments

Comments
 (0)