perf(codegen): a fixed-size allocation expression keeps the buffer-view tier (bench_int_arithmetic 395 → 149 ms)

[proggeramlug]

Follow-up to #9146, which named this as the thing it left on the table. It turned out not to be about the index at all.

The gap

is_fresh_uint8array_length_expr accepts a literal or a known-length local, but nothing built from them — so new Uint8Array(SIZE * SIZE) was never classified as a freshly allocated owned buffer. Without that classification the receiver gets no view, and with no view every element read falls back to js_uint8array_index_get_value, however well its index is proven and however inline the arithmetic around it already is.

That is why bench_int_arithmetic still paid 54 calls per pixel after #9146's interval bounds proof landed. What isolated it: changing only that benchmark's allocation to a literal length, nothing else touched, took it 395 → 168 ms with the calls gone.

Why the relaxation is the same argument, not a wider one

The predicate asks whether the allocation's size is fixed, never what it is. length_source_from_expr resolves the value later, with a FnCtx in hand, and records no constant length when it cannot — in which case the bounds proof simply declines. A sum, difference or product of literals and known-length locals is exactly as fixed as either leaf on its own.

Measurements

Mac mini, against this change's exact merge base, both binaries built in one run, interleaved, min of five, self-timed:

	base	this change	node
bench_int_arithmetic	462 ms	149 ms	62 ms

7.5× Node → 2.4×, identical checksums on both binaries.

Correctness

Both differentials from #9146 stay byte-identical to Node — eight cases around the byte read (in-bounds accumulate, OOB inside +=, OOB as a value, string concat, negative and fractional indices, Buffer receiver, mixed accumulator) and seven around the bounds proof (exact kernel shape, interval touching the last element, counter mutated in the body, out-of-range interval, one-past-the-end, negative composite index, non-constant length). The probe whose interval genuinely exceeds its buffer still declines to the checked call rather than reading out of bounds.

Note on provenance

This commit was pushed to #9146's branch shortly after that PR merged, so it never appeared under review anywhere — I found it while checking that my branches still merged cleanly against main. It is rebased onto current main here with its own changelog fragment; #9146's fragment is restored to the merged text.

https://claude.ai/code/session_012Ys25ni6VwDKE71o1NTYAT

Summary by CodeRabbit

• Performance

  • Improved Uint8Array allocations using fixed-size arithmetic expressions, helping retain efficient buffer-view optimizations.
  • Reduced benchmark runtime from 462 ms to 149 ms while preserving output checksums and bounds-checking behavior.

• Documentation

  • Added release documentation covering performance improvements and byte-equivalence verification.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 09a1872f-2438-4e6a-92a1-fd33d12fd960

📥 Commits

Reviewing files that changed from the base of the PR and between 7b5b311 and 5cde4aa.

📒 Files selected for processing (1)

• changelog.d/9181-uint8array-fixed-size-allocation-keeps-its-view.md

---

📝 Walkthrough

Walkthrough

The compiler now recognizes arithmetic expressions with fixed-length operands as known Uint8Array sizes. The changelog records the retained view optimization, benchmark results, byte equivalence, and bounds-safety verification.

Changes

Uint8Array allocation handling

Layer / File(s)	Summary
Recognize computed fixed lengths crates/perry-codegen/src/collectors/hir_facts.rs, changelog.d/9181-uint8array-fixed-size-allocation-keeps-its-view.md	Fixed-size detection now accepts Add, Sub, and Mul expressions with valid literals or known-length locals. The changelog documents the resulting optimization and verification results.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 7b5b3

The change allows fixed-size arithmetic allocation expressions to retain the optimized buffer-view path, improving performance while preserving the documented correctness behavior; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary code-generation optimization and its performance impact. The reported benchmark values differ from the detailed measurements, but the title remains directly re…
Description check	✅ Passed	The description is detailed and covers the motivation, implementation, related issue, performance measurements, and correctness validation. It does not use the template headings or include the require…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title clearly identifies the primary code-generation optimization and its performance impact. The reported benchmark values differ from the detailed measurements, but the title remains directly related to the main change.

Full details: Description check

Explanation

The description is detailed and covers the motivation, implementation, related issue, performance measurements, and correctness validation. It does not use the template headings or include the required checklist and explicit build/test command status, but the substantive information is mostly complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Gate on the Linux box, commit 7b5b311c4: clean.

Lint: formatting PASS, CI-plan self-test PASS, gap-snapshot self-test PASS, parity-allowlist PASS, no ratchet ceilings raised. The 3 FAIL steps are the environmental ones (unexpanded ${{ github.* }} outside Actions; a changelog fragment is present).

Runtime suite: 2820 passed; 3 failed — the three known parallel-flaky tests, all passing single-threaded on this same commit:

gc::roots::stack_maps::decode_tests::tests::discovers_a_map_from_a_later_loaded_shared_object ... ok
r#box::release_tests::completed_activation_residue_is_bounded_not_linear ... ok
symbol::get::own_data_ic_tests::composed_symbol_field_cache_reloads_mutated_final_slot ... ok

Same set I demonstrated on unmodified origin/main while gating #9161 (three parallel runs there; run 3 failed on discovers_a_map_from_a_later_loaded_shared_object). One new data point: this run flaked with the box at load 1.0, so they are racy in parallel rather than merely load-sensitive. This diff touches one collector file (collectors/hir_facts.rs) and a changelog fragment — no runtime code.

Might be worth an issue to run these three with --test-threads=1 or serialize them; they have cost three gate runs today across two PRs.

[proggeramlug]

Merged (rebased onto main).

Verified the subject is actually live, rather than trusting the benchmark — the number moving is not by itself evidence the predicate is what moved it. IR-diffed --trace llvm for const a = new Uint8Array(SIZE * SIZE) with a summing loop, counting real call @js_uint8array_index_get_value sites (not the declare, which is present either way):

	new Uint8Array(SIZE * SIZE)	new Uint8Array(64)
main	1 call site	0
this PR	0 call sites	0

and a dynamic function f(n) { new Uint8Array(n) … } still emits 16 call sites, so the widening didn't swallow the ineligible case. That's the change doing what it says.

The one thing I checked hard is that the new Add/Sub/Mul arm bypasses guards the leaf predicate has: is_fresh_uint8array_length_literal rejects negatives and anything >= i32::MAX, but Mul(100000, 100000) has both leaves pass while the product overflows, and Sub(1, 5) is negative with both leaves fine. So the arm admits sizes the leaf deliberately excludes.

It turns out not to matter, and I confirmed rather than reasoned it: a negative or oversized length throws RangeError at the allocation, so the let never binds and no read on the view tier is ever reached. A differential probe covering SIZE*SIZE, SIZE+4, SIZE-3, the nested SIZE*SIZE-SIZE+2, negative sizes, non-constant locals in the product, aliasing (const y = x; y[3] = 42), and set/subarray matches node 26.5.1 byte-for-byte.

One divergence surfaced and it is not yours: new Uint8Array(100000 * 100000) throws RangeError on Perry where node allocates it. I isolated it to Perry's allocation cap rather than this predicate by testing the same size as a single literal (new Uint8Array(10000000000)), which fails is_fresh_uint8array_length_literal and so never reaches your new arm — it throws identically. Path-independent, pre-existing.

Validation: perry-codegen 1357 passed / 0 failed; all 60 lint gates green. bench_int_arithmetic runs in ~60 ms here, but I did not A/B the timing against main, so treat that as a measurement on a different box and not as confirmation of the 395 → 149 ms figure — the IR delta above is the part I'd stand behind.

Renamed the changelog fragment to the PR-keyed 9181- convention from changelog.d/README.md, so in-flight PRs can't collide on the filename.