|
| 1 | +# Bisect: the 2026-08-13 broad regression `0a21611fe` → `843ef621f` |
| 2 | + |
| 3 | +**Verdict: `4784d5da7` — #7983, "#6759 C3 rung 1 — make the shape word uniform".** |
| 4 | +Single commit, isolated against its own parent. Fixed in PR #8010. |
| 5 | + |
| 6 | +Not #7997 (the aarch64 SVE prologue decoder) and not #7994 (per-thread |
| 7 | +prototype addresses) — both landed *after* the regression was already fully |
| 8 | +present and cost nothing measurable on this corpus. |
| 9 | + |
| 10 | +## Instrument |
| 11 | + |
| 12 | +Wall clock on the dev box cannot resolve this (load 30–200 all session), so |
| 13 | +everything here is **instructions retired** (`/usr/bin/time -l`, best-of-3, |
| 14 | +exit-checked, output `cmp`-ed against `m0810/expected/`). Every hop rebuilt |
| 15 | +`-p perry -p perry-runtime-static -p perry-stdlib-static` into ONE target dir |
| 16 | +with the `.a` mtimes checked. Corpus binaries went to `$HOME/bisect-bins/<sha>/` |
| 17 | +with the SAME basenames in DIFFERENT directories, per the protocol's `cmp` trap. |
| 18 | + |
| 19 | +The instruction delta tracks the reported wall-clock delta row for row, which is |
| 20 | +what says the regression is **work-bound**. |
| 21 | + |
| 22 | +## The five measured arms (instructions retired, best-of-3) |
| 23 | + |
| 24 | +| bench | `0a21611fe` good | `23a8aad31` parent | `4784d5da7` #7983 | `843ef621f` tip | `1b53332f8` main | **#8010 fixed** | |
| 25 | +|---|--:|--:|--:|--:|--:|--:| |
| 26 | +| cycles | 1,788,433,524 | 1,788,778,236 | **2,759,621,970** | 2,758,613,331 | 2,758,649,326 | **1,788,432,369** | |
| 27 | +| deeplist | 891,705,830 | 898,511,111 | **1,304,689,118** | 1,297,094,745 | 1,303,183,101 | **888,880,890** | |
| 28 | +| interp | 11,619,044,867 | 11,616,644,282 | **14,908,419,553** | 14,890,479,866 | 14,893,088,080 | **11,615,285,338** | |
| 29 | +| pipeline | 2,584,827,137 | 2,582,965,931 | **3,199,913,725** | 3,192,369,325 | 3,193,275,738 | **2,582,793,215** | |
| 30 | +| iso_miss | 14,201,035,818 | 14,205,407,941 | **17,464,543,037** | 17,444,768,294 | 17,459,670,264 | **14,205,774,080** | |
| 31 | +| churn | 3,092,382,790 | 3,091,040,177 | 3,127,189,885 | 3,122,830,475 | 3,123,870,835 | 3,088,954,190 | |
| 32 | +| retain | 1,903,836,211 | 1,929,069,359 | 1,933,154,142 | 1,901,604,111 | 1,913,067,161 | 1,901,657,815 | |
| 33 | +| asyncpipe | 1,205,121,699 | 1,206,723,078 | 1,233,483,901 | 1,231,925,146 | 1,234,911,466 | 1,206,410,762 | |
| 34 | +| fib40 | 3,837,818,438 | 3,838,189,564 | 3,839,712,991 | 3,835,231,921 | 3,836,450,763 | 3,838,170,517 | |
| 35 | + |
| 36 | +The whole regression appears **at #7983 and nothing after it adds any** — the |
| 37 | +`4784d5da7` column already equals the tip. `cycles` fixed matches the good |
| 38 | +endpoint to **1,155 instructions out of 1.79e9 (0.00006%)**. |
| 39 | + |
| 40 | +## Mechanism |
| 41 | + |
| 42 | +The split is **by receiver kind, not program size**: `cycles` (`class Cell`), |
| 43 | +`deeplist` (`class LNode`), `interp`, `pipeline`, `iso_miss` regressed; |
| 44 | +`churn` and `retain` (`type … = { … }` object literals) did not. |
| 45 | + |
| 46 | +The emitted read PIC (`expr/property_get/generic_dispatch.rs`) derives its whole |
| 47 | +cache token from the header shape word: |
| 48 | + |
| 49 | +``` |
| 50 | +is_stamp = (parent_class_id - 0x8000_0000) u< 0x4000_0000 |
| 51 | +token = is_stamp ? (parent_class_id | 1<<62) : keys_array |
| 52 | +``` |
| 53 | + |
| 54 | +and its own comment states the premise: *"Everything else (class instances, |
| 55 | +unstamped receivers) keeps the keys-pointer compare."* |
| 56 | + |
| 57 | +Rung 1 broke that premise **halfway**. It stamps a class instance — but LAZILY, |
| 58 | +at the first by-name resolve — while **codegen INLINE-allocates `new C(…)` and |
| 59 | +stores a literal `0` into that word**, never calling |
| 60 | +`js_object_alloc_class_with_keys` at all. So one shape's population splits, and |
| 61 | +at any site reading a field of a freshly allocated instance: |
| 62 | + |
| 63 | +1. instance #1 misses, is stamped, primes the **id** token; |
| 64 | +2. instance #2 is newborn, computes the **keys-pointer** token → miss; |
| 65 | +3. the handler stamps #2 and re-primes the same id (ids are per keys-array); |
| 66 | +4. instance #3 is newborn → miss. Forever. Hit rate **0%**. |
| 67 | + |
| 68 | +### The single-build proof |
| 69 | + |
| 70 | +Three programs, one compiler, differing only in how many read passes they make |
| 71 | +over the same 3,000,000-instance array: |
| 72 | + |
| 73 | +| program | instructions | delta | per read | |
| 74 | +|---|--:|--:|--:| |
| 75 | +| build array only | 2,564,930,818 | — | — | |
| 76 | +| build + 1 read pass | 2,695,683,246 | **130,752,428** | **43.6** | |
| 77 | +| build + 2 read passes | 2,742,277,736 | **46,594,490** | **15.5** | |
| 78 | + |
| 79 | +Pass 1 sees each instance NEWBORN; pass 2 sees the SAME instances already |
| 80 | +stamped. **2.8×, and the only difference is the stamp.** |
| 81 | + |
| 82 | +## The fix that did NOT work (kept because measuring it is what found the truth) |
| 83 | + |
| 84 | +The first attempt birth-stamped in `js_object_alloc_class_with_keys` / |
| 85 | +`js_object_alloc_class_dynamic_parent`, reading the memoized |
| 86 | +`ShapeCacheEntry::runtime_shape_id`. It measured **zero recovery** — `cycles` |
| 87 | +2,759,257,399, unchanged. `--trace llvm` on `cycles.ts` then showed why: the |
| 88 | +`new Cell(…)` site is a bump-pointer allocation emitting |
| 89 | +`store i64 8589934592` (parent_class_id 0 ‖ field_count 2) directly; the runtime |
| 90 | +allocator is declared but never called. Worse, birth-stamping only the runtime |
| 91 | +path would have created a NEW split for any class allocated both ways. |
| 92 | + |
| 93 | +★ **A fix whose subject never runs looks exactly like a fix that didn't help.** |
| 94 | +The `.a` mtimes moved, the binaries differed, the unit test passed, and the |
| 95 | +change was still inert on the hot path. |
| 96 | + |
| 97 | +## The fix as landed (PR #8010) |
| 98 | + |
| 99 | +`shapes::shape_word_is_stampable` restores the `class_id == 0` discriminant in |
| 100 | +ONE named place, on both the read and write side; `object_shape_stamp` and |
| 101 | +`stamp_object_shape` route through it. The population is uniform again (nobody |
| 102 | +stamped) and every row returns to the good endpoint. |
| 103 | + |
| 104 | +Rung 1's structural content stays: the helper trio, the `RegExpHeader`-alias |
| 105 | +hole it closed (the old gate never excluded a RegExp — its offset 8 reads as |
| 106 | +`class_id == 0`), and the delete/re-mint machinery. |
| 107 | + |
| 108 | +**Rung 2 belongs in codegen** (PR #8009): the inline allocation must store the |
| 109 | +class's ShapeId where it stores `0`. When that lands, |
| 110 | +`shape_word_is_stampable` → `shape_word_is_writable` and the population is |
| 111 | +uniform the other way. The gate test is written to pass in BOTH uniform states, |
| 112 | +so it does not change with the flip. |
| 113 | + |
| 114 | +## Question 2 — churn / retain / asyncpipe were flat; the mini's +13/+13/+27% is not this commit |
| 115 | + |
| 116 | +Peak RSS and GC collection counts are BOTH load-independent, so this is valid on |
| 117 | +a busy box. `PERRY_GC_DIAG=1` **and** `PERRY_GC_TRACE=1` (DIAG alone prints |
| 118 | +nothing); positive control — the printer emits 445 lines and 22 |
| 119 | +`collection_kind":"minor"` for `cycles`. |
| 120 | + |
| 121 | +| bench | quantity | `0a21611fe` | `1b53332f8` main | Δ | |
| 122 | +|---|---|--:|--:|--:| |
| 123 | +| churn | instructions | 3,092,462,749 | 3,124,640,479 | +1.0% | |
| 124 | +| | peak RSS (KB) | 24,976 | 24,992 | +0.06% | |
| 125 | +| | minors / fulls | 88 / 0 | 88 / 0 | identical | |
| 126 | +| retain | instructions | 1,936,036,952 | 1,931,641,207 | −0.2% | |
| 127 | +| | peak RSS (KB) | 254,800 | 254,816 | +0.006% | |
| 128 | +| | minors / fulls | 4 / 0 | 4 / 0 | identical | |
| 129 | +| asyncpipe | instructions | 1,206,387,962 | 1,231,635,504 | +2.1% | |
| 130 | +| | peak RSS (KB) | 38,832 | 39,168 | +0.9% | |
| 131 | +| | minors / fulls | 1 / 0 | 1 / 0 | identical | |
| 132 | +| **cycles** (control) | instructions | 1,790,911,190 | 2,759,606,611 | **+54%** | |
| 133 | +| | peak RSS (KB) | 24,816 | 24,800 | −0.06% | |
| 134 | +| | minors / fulls | 22 / 0 | 22 / 0 | identical | |
| 135 | + |
| 136 | +* **GC-scheduling explanation: refuted.** Collection counts are identical on all |
| 137 | + four programs, zero full collections anywhere. Nothing was rescheduled. |
| 138 | +* **Locality/footprint explanation: refuted.** Peak RSS is flat to ≤0.9%. |
| 139 | +* **Work: flat** for churn (+1.0%) and retain (−0.2%). |
| 140 | + |
| 141 | +★ Note the control row: `cycles` regressed **54%** with RSS and collection counts |
| 142 | +**also flat**. So flat RSS/counts can only REFUTE the scheduling and footprint |
| 143 | +explanations — they can never confirm "nothing changed". The positive statement |
| 144 | +for churn/retain is the instruction count, and it is flat. |
| 145 | + |
| 146 | +With no work added, no collection rescheduled and no footprint change, there is |
| 147 | +no mechanism left for a 13% wall-clock move on churn or retain: **those two rows |
| 148 | +are mini-side variance.** They should regain their node wins on the next sweep — |
| 149 | +the fix leaves them 1.1% and 0.6% BELOW main. |
| 150 | + |
| 151 | +`asyncpipe` is the one row with a real attributable cost: **+2.1%**, not +27%, |
| 152 | +and the fix returns it exactly to the good endpoint (1,206,410,762 vs |
| 153 | +1,205,121,699). The remaining ~25 points are either mini variance or **idle/ |
| 154 | +parked time**, which neither instructions retired nor cycles elapsed can observe |
| 155 | +for an async program — that can only be settled on the quiet mini, and it is now |
| 156 | +moot. |
| 157 | + |
| 158 | +## Validation |
| 159 | + |
| 160 | +* `cargo test -p perry-runtime --lib` (`RUST_TEST_THREADS=1`): **2278 pass, 0 fail**. |
| 161 | +* Sabotage-verified with the fix **committed first**; restored and **REBUILT** |
| 162 | + (`Compiling perry-runtime` = 1) before re-confirming green. |
| 163 | +* `iso_miss` canary prints `checksum 437840 misses 0`, including under |
| 164 | + `PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 |
| 165 | + PERRY_GC_VERIFY_EVACUATION=1`; `cycles`/`deeplist`/`interp` byte-identical |
| 166 | + under the same knobs. |
| 167 | +* Whole probe corpus output-verified against `m0810/expected/` at every arm. |
| 168 | +* Gap suite, class/property/shape/delete/proto slice (70 tests), graded against |
| 169 | + `test-parity/gap_snapshot.json`: **68 pass, 1 expected-fail, 0 unexpected.** |
| 170 | + The 70th (`test_gap_3527_http_ctor_prototype`) is #7999's tokio link guard |
| 171 | + refusing an ad-hoc build set without `-p perry-ext-http` — an artefact of |
| 172 | + compiling outside the harness, not a behavioural difference. |
| 173 | + |
| 174 | +★ The full 554-test gap suite could NOT be run on this host: the harness stalls |
| 175 | +at 0% CPU after ~10-25 tests, on both a full run and a 1/8 shard, while the same |
| 176 | +compile finishes in seconds standalone. The box was at **8.6 GiB free** (the |
| 177 | +protocol wants >= 25 GiB for parity runs) and under heavy contention from other |
| 178 | +sessions. That is a host constraint, not a property of this change; CI runs the |
| 179 | +full suite on the PR. |
| 180 | + |
| 181 | +## Bisect hygiene notes |
| 182 | + |
| 183 | +* `git status` cannot see a stale `.a`. Every hop verified `libperry_runtime.a` |
| 184 | + and `perry` mtimes moved after the checkout. |
| 185 | +* The `d456b411e` dirty-but-corroborating sweep recorded in |
| 186 | + MEASUREMENT-PROTOCOL.md held up: everything up to and including #7981 is flat. |
0 commit comments