Skip to content

Commit d210b31

Browse files
nanofuxionclaude
andcommitted
Flush the property cache for a prototype, not for every property
`js_prop_cache_invalidate` bumps a runtime-wide generation counter, so every entry in the shape-keyed property cache dies at once. It was called unconditionally on entry to `add_property`. Every object literal field, every constructor field and every expando therefore threw away the whole cache: an ExpressX request does that twenty-odd times, and every field read in between paid a miss and a refill. Only a prototype's new property can make a live entry wrong. An entry answers "a receiver of shape S finds this at depth d, slot k", and it is only ever filled from a property that was found. Adding to an ordinary object changes that object's own shape, so an entry keyed on the old one stops matching it, and no other object's layout moved. Adding to a prototype can shadow something a site already found further down the chain, which is the one case a live entry is still matched and wrong. Resize, compact, in-place shape update, prototype reassignment and a freed shape's address all keep their own invalidation at their own site. The gate needs `is_prototype` to mean what it says, and it did not. Upstream sets it only in `JS_SetPrototypeInternal`, because upstream uses it to track `Array.prototype` rather than to answer "is anyone inheriting from me" -- so `Foo.prototype` and every builtin prototype had it false. It is now set in `js_new_shape_nohash`, the one funnel every shape creation passes through, which flags an object the moment a shape names it as a prototype. That necessarily precedes any cached entry whose chain includes it, because an entry is only filled from a receiver that was actually walked. Worth: twelve warm reads sharing a loop with one four-field literal go 150.9 to 128.0 ns an iteration, which is 25.7 ns of flush interference down to 5.6. On ExpressX, 0.1 to 1.8% across seven request shapes. On benchmarks/wintertc, nothing. The last two are the honest part: ExpressX reads few fields twice on the same shape, and none of the wintertc workloads is a warm read set. It is kept for being strictly less work, not for those two rows. tests/fixtures/prop_cache_shadow.mjs guards it: warm sites, then an own property shadowing an inherited one, a nearer prototype shadowing a further one, `Object.prototype` gaining and losing one, a constructor prototype's method replaced and then shadowed, `setPrototypeOf` under a warm site, an accessor redefined as data, delete-and-regrow, and 500 iterations of ordinary object churn that must disturb none of it. Node runs the same file and agrees. Deleting the `is_prototype` line makes it fail on the `Foo.prototype` case, which is the negative control. All 53 files in third_party/quickjs/tests behave identically either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent d869d3c commit d210b31

5 files changed

Lines changed: 251 additions & 5 deletions

File tree

CMakeLists.txt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,13 @@ if(BUILD_TESTING AND SXN_BUILD_TESTS)
236236
# expected value is Node's, bar the one i32 wrap that is SX's by definition.
237237
add_test(NAME sxn-struct-sroa COMMAND sxn ${CMAKE_CURRENT_SOURCE_DIR}/tests/fixtures/struct_sroa.sx)
238238
set_tests_properties(sxn-struct-sroa PROPERTIES FAIL_REGULAR_EXPRESSION "FAIL")
239+
# The property cache is flushed when a prototype gains a property, and not
240+
# when an ordinary object does. This is the second half: a warm call site
241+
# that has already found a property, and then something that should shadow
242+
# or move it. Every answer is Node's, and `node tests/fixtures/
243+
# prop_cache_shadow.mjs` runs the same file.
244+
add_test(NAME sxn-prop-cache-shadow COMMAND sxn ${CMAKE_CURRENT_SOURCE_DIR}/tests/fixtures/prop_cache_shadow.mjs)
245+
set_tests_properties(sxn-prop-cache-shadow PROPERTIES FAIL_REGULAR_EXPRESSION "FAIL")
239246
# Reference cycles are invisible to refcounting and reclaimable only by the
240247
# cycle collector, which runs only on allocation. Sxn.gc() is the explicit
241248
# ask; this asserts a burst of 100k cyclic closures comes back.

spec/IMPLEMENTATION.md

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,61 @@ emitter by a byte. Nothing emitted it -- it is selected only for an unchecked
375375
checked -- so it had never mattered, and briefly making the path live is what
376376
surfaced it. It stays dead; the declaration is now honest.
377377

378+
### The property cache was flushed by every property anyone added
379+
380+
`js_prop_cache_invalidate` bumps a runtime-wide generation counter, which
381+
invalidates every entry in the shape-keyed property cache at once. It was
382+
called unconditionally on entry to `add_property` -- so every object literal
383+
field, every constructor field and every expando threw away the whole cache.
384+
An ExpressX request builds a 13-property response, adds eight properties to a
385+
native `Request` and builds an outcome object, so it did that twenty-odd
386+
times, and every field read between them paid a miss and a refill.
387+
388+
Only a prototype's new property can make a live entry wrong. The cache answers
389+
"a receiver of shape S finds this at depth d, slot k", and it is only ever
390+
filled from a property that was *found*. Adding to an ordinary object changes
391+
that object's own shape, so an entry keyed on the old one stops matching it
392+
and nothing else moved. Adding to a prototype can shadow something a site
393+
already found further down the chain, and that is the one case where a live
394+
entry is still matched and is wrong. Every other way a property can move --
395+
resize, compact, in-place shape update, prototype reassignment, a freed
396+
shape's address -- already invalidates at its own site.
397+
398+
That gate needed one other change to be sound. `is_prototype` was set only in
399+
`JS_SetPrototypeInternal`, which leaves it false for `Foo.prototype` and for
400+
every builtin prototype: upstream uses the flag only to track
401+
`Array.prototype`, not to answer "is anyone inheriting from me". It is now set
402+
in `js_new_shape_nohash`, the single funnel every shape creation passes
403+
through, so an object is flagged the moment a shape names it as a prototype --
404+
which necessarily precedes any cached entry whose chain includes it, because
405+
an entry is only filled from a receiver that was actually walked.
406+
407+
`tests/fixtures/prop_cache_shadow.mjs` is the guard: warm call sites, then own
408+
properties shadowing inherited ones, a nearer prototype shadowing a further
409+
one, `Object.prototype` gaining and losing a property, a constructor
410+
prototype's method replaced and then shadowed, `setPrototypeOf` under a warm
411+
site, an accessor redefined as data, and 500 iterations of ordinary object
412+
churn that must disturb none of it. Node runs the same file and agrees.
413+
Removing the `is_prototype` line makes it fail on the `Foo.prototype` case,
414+
which is the one upstream leaves unflagged.
415+
416+
Worth, measured three ways:
417+
418+
| | flush every add | gated |
419+
|---|---|---|
420+
| 12 warm reads beside one 4-field literal | 150.9 ns | 128.0 |
421+
| ExpressX, per request, seven shapes | -- | 0.1 to 1.8% |
422+
| `benchmarks/wintertc` throughput | -- | unmoved |
423+
424+
The first row is the shape the cache exists for and the interference is
425+
mostly gone: 25.7 ns of it per iteration against 5.6. The other two are the
426+
honest part. ExpressX reads few fields twice on the same shape -- a fresh
427+
`Request` and a fresh response object per request -- so most of its reads
428+
would miss anyway, and the wintertc workloads are Buffer, TextEncoder,
429+
EventEmitter and JSON, none of which is a warm read set. The change is kept
430+
because it is strictly less work for the same answer, not because those two
431+
rows moved.
432+
378433
### A JIT is ruled out by platform, not by effort
379434

380435
iOS does not grant W^X/JIT entitlements to third-party apps, so a

spec/PERFORMANCE.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ tagged `arcsx:` in `third_party/quickjs`), roughly in order of payoff:
6464
pass allocated 7-8 blocks; recycling them is what took Buffer 83->36 ms
6565
and TextEncoder 65->23.5 ms in a single change, and cut the pause
6666
benchmark's total time from 1.1 s to 0.41 s.
67+
- **The property cache, flushed only by a prototype.** Adding a property to
68+
any object at all used to invalidate every entry in the shape-keyed
69+
property cache, so an object literal beside a warm read set threw the reads
70+
away on every iteration. Only a prototype's new property can shadow
71+
something a call site already found, so only that invalidates now. Twelve
72+
warm reads sharing a loop with one four-field literal go 150.9 -> 128.0 ns
73+
an iteration; the interference the flush caused was 25.7 ns of that and is
74+
now 5.6. The rows in this document do not move -- none of these workloads
75+
is a warm read set -- and `spec/IMPLEMENTATION.md` has the soundness
76+
argument and the fixture that guards it.
6777
- **Pinned core-type shapes.** QuickJS interns the empty shape behind
6878
`new Foo()` in a runtime-wide table, but nothing holds a reference to it,
6979
so a loop that allocates and drops one object per iteration destroys the
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// The property cache is invalidated when a prototype gains a property, and
2+
// not when an ordinary object does. These are the shapes where the second
3+
// half could go wrong: a site that has already cached where it found a
4+
// property, and then something adds one that should shadow it, or moves it.
5+
//
6+
// Every answer here is the language's, and none of them depends on a cache.
7+
8+
let failures = 0;
9+
function check(name, actual, expected) {
10+
if (!Object.is(actual, expected)) {
11+
console.log("FAIL", name, "expected", expected, "got", actual);
12+
failures += 1;
13+
}
14+
}
15+
// One call site per case, so each has its own cache entry, and each is warmed
16+
// before the mutation it is meant to notice.
17+
const warm = (f, x, n) => { for (let i = 0; i < n; i++) f(x); };
18+
19+
// --- a new own property shadows an inherited one -------------------------
20+
{
21+
const proto = { m: "proto" };
22+
const o = Object.create(proto);
23+
const read = (x) => x.m;
24+
warm(read, o, 200);
25+
check("inherited before", read(o), "proto");
26+
o.m = "own";
27+
check("own shadows inherited", read(o), "own");
28+
}
29+
30+
// --- a nearer prototype shadows a further one ----------------------------
31+
{
32+
const far = { m: "far" };
33+
const near = Object.create(far);
34+
const o = Object.create(near);
35+
const read = (x) => x.m;
36+
warm(read, o, 200);
37+
check("found at depth 2", read(o), "far");
38+
near.m = "near";
39+
check("depth 1 shadows depth 2", read(o), "near");
40+
}
41+
42+
// --- Object.prototype, reached from a plain object -----------------------
43+
{
44+
const o = { a: 1 };
45+
const read = (x) => x.zz_probe;
46+
check("absent before", read(o), undefined);
47+
warm(read, o, 200);
48+
Object.prototype.zz_probe = "proto";
49+
check("added to Object.prototype", read(o), "proto");
50+
const mid = { zz_probe: "own" };
51+
check("own beats Object.prototype", read(mid), "own");
52+
delete Object.prototype.zz_probe;
53+
check("deleted again", read(o), undefined);
54+
}
55+
56+
// --- a constructor prototype gaining a method ----------------------------
57+
{
58+
function Foo() { this.a = 1; }
59+
Foo.prototype.m = function () { return "one"; };
60+
const f = new Foo();
61+
const call = (x) => x.m();
62+
warm(call, f, 200);
63+
check("prototype method", call(f), "one");
64+
Foo.prototype.m = function () { return "two"; };
65+
check("replaced on the prototype", call(f), "two");
66+
f.m = function () { return "own"; };
67+
check("own method shadows it", call(f), "own");
68+
}
69+
70+
// --- a fresh object assigned as a prototype after the site is warm -------
71+
{
72+
function Bar() {}
73+
const b = new Bar();
74+
const read = (x) => x.k;
75+
Object.prototype.k = "object";
76+
warm(read, b, 200);
77+
check("from Object.prototype", read(b), "object");
78+
Bar.prototype.k = "bar";
79+
check("Bar.prototype shadows it", read(b), "bar");
80+
delete Object.prototype.k;
81+
check("still Bar's", read(b), "bar");
82+
}
83+
84+
// --- setPrototypeOf under a warm site ------------------------------------
85+
{
86+
const a = { m: "a" }, c = { m: "c" };
87+
const o = Object.create(a);
88+
const read = (x) => x.m;
89+
warm(read, o, 200);
90+
check("through a", read(o), "a");
91+
Object.setPrototypeOf(o, c);
92+
check("through c", read(o), "c");
93+
}
94+
95+
// --- ordinary objects growing, which must not disturb anything ----------
96+
{
97+
const proto = { m: "proto" };
98+
const read = (x) => x.m;
99+
const first = Object.create(proto);
100+
warm(read, first, 200);
101+
let last = null;
102+
for (let i = 0; i < 500; i++) {
103+
// Each of these adds four properties to an ordinary object, which is
104+
// what used to flush the cache 2000 times over this loop.
105+
const churn = { w: i, x: i, y: i, z: i };
106+
if (churn.w !== i) { failures += 1; }
107+
last = Object.create(proto);
108+
last.own = i;
109+
if (read(last) !== "proto") { failures += 1; }
110+
}
111+
check("still found through the prototype", read(first), "proto");
112+
check("and on the last one built", read(last), "proto");
113+
proto.m = "changed";
114+
check("changing the prototype is still seen", read(first), "changed");
115+
}
116+
117+
// --- accessors and non-writables on a prototype --------------------------
118+
{
119+
const proto = {};
120+
Object.defineProperty(proto, "g", { get() { return "getter"; }, configurable: true });
121+
const o = Object.create(proto);
122+
const read = (x) => x.g;
123+
warm(read, o, 200);
124+
check("inherited getter", read(o), "getter");
125+
Object.defineProperty(proto, "g", { value: "data", configurable: true });
126+
check("redefined as data", read(o), "data");
127+
}
128+
129+
// --- delete, resize and compaction --------------------------------------
130+
{
131+
const o = {};
132+
for (let i = 0; i < 40; i++) o["p" + i] = i;
133+
const read = (x) => x.p39;
134+
warm(read, o, 200);
135+
check("last property", read(o), 39);
136+
for (let i = 0; i < 39; i++) delete o["p" + i];
137+
check("survives 39 deletes", read(o), 39);
138+
for (let i = 0; i < 40; i++) o["q" + i] = i;
139+
check("survives regrowth", read(o), 39);
140+
}
141+
142+
if (failures !== 0) throw new Error(failures + " property-cache checks failed");
143+
console.log("property cache: all checks passed");

third_party/quickjs/quickjs.c

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,9 +1270,12 @@ struct JSShape {
12701270
- A hit requires an exact (pc, shape) match *and* a matching global
12711271
generation stamp, so a stale entry can never be used.
12721272
- `gen` is bumped by js_prop_cache_invalidate() at every point that can
1273-
change where a property lives: adding, deleting, resizing, compacting,
1274-
in-place shape updates, prototype changes, and shape frees (the last
1275-
also rules out a freed shape's address being reused by a new one).
1273+
change where a property lives: deleting, resizing, compacting, in-place
1274+
shape updates, prototype changes, shape frees (which also rules out a
1275+
freed shape's address being reused by a new one), and adding a property
1276+
to an object that is somebody's prototype. Adding to an ordinary object
1277+
needs no bump: its own shape changes, so an entry keyed on the old shape
1278+
stops matching it, and nothing else moved. See add_property.
12761279
- Only the *location* is cached, never a value, so ordinary writes to an
12771280
existing slot need no invalidation: the value is always read fresh.
12781281
Walking `depth` prototypes from a live receiver is safe because
@@ -6758,8 +6761,20 @@ static inline JSShape *js_new_shape_nohash(JSContext *ctx, JSObject *proto,
67586761
sh = get_shape_from_alloc(sh_alloc, hash_size);
67596762
JS_REF_COUNT(sh) = 1;
67606763
add_gc_object(rt, &sh->header, JS_GC_OBJ_TYPE_SHAPE);
6761-
if (proto)
6764+
if (proto) {
67626765
js_dup(JS_MKPTR(JS_TAG_OBJECT, proto));
6766+
/* arcsx: every shape creation passes through here, so this is where
6767+
an object becomes reachable as somebody's prototype. Upstream sets
6768+
is_prototype only in JS_SetPrototypeInternal, which leaves it false
6769+
for `Foo.prototype` and for every builtin prototype -- fine while
6770+
the flag only tracked Array.prototype, but add_property now uses it
6771+
to decide whether a property being added can shadow one a call site
6772+
has already cached deeper in a chain. Flagged here, the answer is
6773+
always yes when it might be: a cached entry can only name a chain
6774+
that some real receiver walked, and creating that receiver's shape
6775+
ran this line first. */
6776+
proto->is_prototype = true;
6777+
}
67636778
sh->proto = proto;
67646779
/* prop_hash_mask must be set before prop_hash_end(sh) is used, as the hash
67656780
location depends on it in the merged-header layout. */
@@ -11518,10 +11533,26 @@ JSValue JS_GetPropertyStr(JSContext *ctx, JSValueConst this_obj,
1151811533
static JSProperty *add_property(JSContext *ctx,
1151911534
JSObject *p, JSAtom prop, int prop_flags)
1152011535
{
11521-
js_prop_cache_invalidate(ctx->rt); /* arcsx: shape gains a property */
1152211536
JSShape *sh, *new_sh;
1152311537

1152411538
if (unlikely(p->is_prototype)) {
11539+
/* arcsx: only a prototype's new property can invalidate a cached
11540+
lookup. The cache answers "a receiver of shape S finds this at
11541+
depth d, slot k", and it is only ever filled from a property that
11542+
was found. Adding to an ordinary object changes that object's own
11543+
shape, so an entry keyed on the old one stops matching it and no
11544+
other object's layout moved; adding to a prototype can shadow
11545+
something a site already found further down the chain, which is the
11546+
one case a live entry could still be matched and be wrong. Every
11547+
other way a property can move -- resize, compact, in-place shape
11548+
update, prototype reassignment, a freed shape's address -- keeps
11549+
its own invalidation at its own site.
11550+
11551+
This is the difference between flushing the runtime's whole
11552+
property cache on every object literal, constructor field and
11553+
expando, and flushing it when a prototype gains a method.
11554+
spec/PERFORMANCE.md has what it was worth. */
11555+
js_prop_cache_invalidate(ctx->rt);
1152511556
/* track addition of small integer properties to
1152611557
Array.prototype and Object.prototype */
1152711558
if (unlikely((p == JS_VALUE_GET_OBJ(ctx->class_proto[JS_CLASS_ARRAY]) ||

0 commit comments

Comments
 (0)