perf: avoid redundant retains for numeric array reads - #404
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
vinikjkkj
added a commit
to vinikjkkj/scriptc
that referenced
this pull request
Sep 25, 2026
A byte loop paid for its receiver on every single iteration. For for (let i = 0; i < dst.length; i++) dst[i] = (dst[i] ^ src[i]) & 0xff; both lanes emitted a retain/release pair around the length test, around each of the two reads, and around the write -- ten refcount calls per iteration on the LLVM lane, fifteen retain/release statements on the C lane -- to protect a value that the binding in scope already owned for the whole loop. Evaluate the receiver as a BORROW when it is a direct, unboxed binding and every operand evaluated after it is provably unable to overwrite that binding. The owner keeps it alive; the emitter stops taking a reference of its own. JS evaluation order is untouched -- this only decides who holds the reference while the access happens. The stability test (ir/analysis.ts, shared by both lanes so they cannot reach different conclusions from the same IR) is a small WHITELIST, not a blacklist: literals, plain reads, arithmetic, logical and ternary forms, and the three non-allocating bytes reads. Anything else -- a call, a dyn operation, an await, anything that can run user code -- answers false and the caller keeps the owned snapshot. assignExpr is checked against the receiver by name and recursed into, because an assignment nested under a ternary still produces the index while overwriting the receiver. incDec needs no check: it is numeric-only and can never write a bytes binding. Getting this wrong conservatively costs a retain/release pair; getting it wrong permissively is a use-after-free, so the default is false. Boxed and captured bindings are refused outright -- a capture box is shared, any other capture can replace its contents underneath a borrow, and the read through the box is itself a +1. A TDZ global is refused because its read has to run the guard. A borrow is NOT the existing immortal temp, and the distinction is the whole safety argument. An immortal is OWNED: a +1 on a value whose rc is SIZE_MAX, so its release is a runtime no-op and may be skipped. A borrow was never +1, so releasing it would really decrement. It therefore stays off the release frame entirely rather than joining it with a skip flag. A consumer that later tried to take ownership of one hits moveTemp's "not found in any frame" emitter bug -- a loud compile-time failure, which is the correct outcome for an unsound borrow rather than something to soften. On a 4 KiB x 4000-pass elementwise XOR -- the shape the transport and crypto paths are made of -- median wall clock, 10 alternating samples per arm, x86_64-windows-gnu, zig 0.16.0, outputs byte-identical within and across lanes: LLVM 315.4 ms -> 136.6 ms (2.31x) C 194.4 ms -> 99.9 ms (1.95x) That is a bytes-saturated loop and the numbers belong to it, not to any whole program. The C lane stays ahead because it still inlines the element access (scr_bytes_get_inl) while the LLVM lane calls @scr_bytes_get_fast and @scr_bytes_len. Closing that is the next change, and it needs the struct offsets the layout guard now pins. Ported from vercel-labs/scriptc 53e2f08 (vercel-labs#97) and 82b2417 (vercel-labs#404), rewritten against our emitters: upstream split expr-bytes.ts and expr-containers.ts out of the monolithic files after our fork point, so none of their hunks apply -- only the idea and the stability rule.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Avoid redundant retain/release calls around numeric-array reads in C and LLVM when the receiver's binding guarantees its lifetime through index evaluation. Uncertain cases keep their existing ownership handling.