Repository navigation
perf: #8417 regressed iso_miss +24.8% (+27.2% instructions) while fixing #8394's quadratic concat #8486
Description
Activity
Implemented in PR #8487: #8487
I instrumented
js_string_appendbefore changing behavior. The proposed refcount failure was not the hot shape:iso_misssamples were overwhelmingly the first append to the shared empty string (dest_blen=0,capacity=0,refcount=0); frames with a second name then reachedrefcount=1,capacity=32and used the intended in-place path.The common one-name frame was instead building the complete
[name]suffix withjs_string_concat_chain, then rooting, allocating, and copying that suffix again merely to compute"" + suffix. The fix keeps #8417's lowering and reuses the suffix only when the destination is empty and the suffix is already shared. Unique sources still copy, so the change cannot create an untracked mutable alias. Tag-checked heap-string append arms use a narrow entry point that skips repeated generic pointer validation; all non-identity cases delegate to the existing append helper.Three
/usr/bin/time -lsamples per cell, exact static-wrapper build andPERRY_NO_AUTO_OPTIMIZE=1 PERRY_NO_CACHE=1:workload metric before after result iso_missinstructions, median 14.790 G 12.471 G -15.7% wall, median (range) 0.89 s (0.89-1.44) 0.75 s (0.74-1.18) ranges overlap; no wall claim peak RSS, median 31.28 MiB 31.19 MiB -0.3% #8394 fixture instructions, median 27.063 M 27.023 M -0.15% / flat wall, median (range) 0.00 s (0.00-0.37) 0.00 s (0.00-0.72) timer floor; no wall claim peak RSS, median 5.23 MiB 5.23 MiB flat This recovers about 70% of the established 11.54 G -> 14.68 G regression gap while preserving #8394's asymptotic win. The residual is the extra tag-dispatch/append stage retained by #8417; closing it completely would require a fused append-chain lowering, not another safe redundant-copy elimination.
The explicit #8394 2k/4k/8k/16k scaling probe remains 0/0/1/0 ms with correct lengths, versus the old quadratic 17/56/342/1450 ms curve.
Validation: 19/19 sweep programs byte-exact; runtime 2,606 passed / 4 ignored;
perry1,008 passed; codegen 1,113 passed; all 52 lint gates passed; exact release build passed.
#8417 fixed #8394's quadratic concat but regressed
iso_missby 24.8%Bisected on the quiet bench mini sweep. Clean adjacent-commit attribution:
iso_missinstructionsddc1cd845(parent)5f9aa1f40— #8417 "retain string accumulators in concat chains"That is +27.2% instructions, and on the mini it shows as wall time 0.6810 s -> 0.8501 s
(+24.8%), pushing
iso_missfrom 1.96x to 2.53x Node. The sample ranges are fullydisjoint (previous max 0.7080 < current min 0.8501), so this is not noise.
-Os(#8457) is not the cause — forcingPERRY_LL_SIZE_OPT=0gives +0.8% withoverlapping samples, and both arms already sit at ~14.7 G.
This is a real trade, not a mistake
#8417 did exactly what it claimed. Re-measured on the same fixture from #8394:
90x at n=16000, and the curve went from roughly quadratic to flat. That is a large,
real win on accumulator-shaped concatenation.
The problem is that
iso_missbuilds a string it usually throws away — its hot line isand the result is only read on a lookup miss. Retaining the accumulator helps when the
string is kept and hurts when it is discarded.
iso_missis the discard case, and it is therow where string concat was already ~18% of self time.
What would resolve it
Ideally the retention becomes conditional rather than unconditional — retain when the
accumulator is subsequently read, fall back when it is dead. Whether that is cheaply
decidable in
literals_vars.rsis the open question; if it is not, this is a genuinepolicy choice about which shape to favour and should be recorded as one.
Note #8417 is small (
crates/perry-codegen/src/expr/literals_vars.rs, +42/-1 plus tests),so the surface to reason about is narrow.
Process note
I merged #8417 after validating it against the #8394 scaling fixture and confirming 19/19
corpus byte-exactness — but I did not run a timing A/B across the corpus, only a
correctness one. A perf PR that changes codegen for a common operation needs the corpus
timed, not just diffed. That gap is why this took a full sweep to surface.