Repository navigation
perf(go): own a Vec<u8>'s storage while only the translation reaches it - #643
Conversation
Quest-Claim: c686a62dbe6127f6
The runtime grows a byte buffer it owns without zeroing it, through growslice, and recycles an outgrown one through power-of-two pools. Own storage is told by its shape: a power-of-two capacity of at least 8, never filled. A root function trims Go's storage of that shape on the way in and clips its own to its length on the way out, in its result and behind its arguments' &mut and Box, so storage Go can reach is never recycled and shows nothing past its length. A function pointer over byte storage in a root's signature is refused. The bench's ffi.toml drops excludes of crate-private items, which rust2-ffi refuses since #607. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ng past its length A root's walk cleared a Go-held Vec's slots past its length, which may be Go's own elements of a larger array. Pop, truncate, and clear zero what they vacate instead, as slices.Delete does; a Vec<u8>'s have nothing to zero. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…of how it escapes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The maintainer decided on 2026-11-07 that clipping's 1 to 4% miss of the 2x bar on two stream workloads is noise, so speed meets its bar and coverage splits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nly roots Round 5 of the review found ways own storage reaches memory Go holds: - A full Go slice of own-shaped capacity, shortened inside, took the shape; bytesEnter now copies it. - Internal functions on a pub type exported as `Type_method`, callable past the handover; they are now unexported. - A Go callback beside `&mut Vec<u8>` could read or keep storage mid-call; any function pointer or trait object in a root whose signature reaches byte storage is now refused. - Go aliases of a container the translation writes, then abandons (an outgrown array, a Box), kept own storage; a root now copies each Box pointee and Vec element array an argument owns, so a by-value Box is the translation's copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # go/README.md # go/fixtures/reject/expected.txt
|
Design change: narrow the handover, decided with the maintainer on 2026-11-07 at the round-6 design checkpoint. A root whose signature reaches byte storage now refuses, each with its span and a
With nested references refused, the only places of Go's the translation can write are behind a top-level Rejected: keeping the general walks and recording each (Written by Claude Opus 5.5 with high effort) |
A root that reaches byte storage refuses a raw pointer in its signature and a reference anywhere in an argument but its outermost type, so the exit walk only follows top-level &mut arguments and no argument sees an entry copy's address. The Back walk and raw-pointer arms go. The Go epic and its children wait until a consumer needs pure Go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ifetime Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit 7e26e6a against base 132d0ad.
One actionable finding: the output handover walk can recurse forever on an otherwise supported cyclic shared-reference result (inline P2). The narrowed argument restrictions do not cover this case.
Direction: preserve the storage-ownership boundary, but make output traversal cycle-safe or refuse unsupported cyclic output shapes explicitly. The accepted clipping/performance trade-off is not the issue here.
Verification limits: GitHub-only static inspection of allocation/pooling, entry/exit walks, root wrappers/refusals, constant lowering, and handover fixtures. No translator probes, Go tests, CI, or benchmarks were run; reported performance and passing validation were not independently reproduced.
(Written by review (OpenAI))
| let f = self.hand(inner, hand, span); | ||
| match copy { | ||
| true => body.push(format!("if *{p} != nil {{ c := **{p}; *{p} = &c; {f}(&c) }}")), | ||
| false => body.push(format!("if *{p} != nil {{ {f}(*{p}) }}")), |
There was a problem hiding this comment.
[P2] Guard the output walk against cyclic shared references
The generated Out walk follows every non-nil reference recursively without tracking visited objects. Safe Rust can return a cyclic static: struct Node { bytes: Vec<u8>, next: &'static Node }, static NODE: Node = Node { bytes: Vec::new(), next: &NODE };, and pub fn node() -> &'static Node { &NODE }. The existing constant lowering explicitly supports cyclic statics and empty Vec constants, but the new wrapper walks NODE.next forever and eventually stack-overflows before returning. The compile-time seen check only terminates type inspection; it does not stop these generated calls. The nested-reference refusal applies to arguments, not this output. Track visited referents during handover (or explicitly reject this output shape), and add a cyclic-static result regression test.

Finishes quest/m4/go/speed.md, implementing the storage ownership the maintainer settled on 2026-11-07 in moq-dev/rust2#609. Every
stream/workload now runs within 2x of cgo, the maintainer counting a 1 to 4% miss on two of them as noise, so speed meets its bar and the quest is deleted.Problem
A group's stream runs 2.6 to 3.5x slower translated than through cgo, mostly in growing the output: zeroing and copying each larger buffer, and the collector reclaiming the ones left behind. The 2026-11-07 decision: the runtime grows a
Vec's storage without zeroing and recycles outgrown buffers while only the translation reaches it, hands it to Go code clipped to its length, and never recycles it after.Approach
Vec<u8>grows throughbytesPush/bytesExtend(go/runtime/rt.go): into a power-of-two buffer from async.Poolper size, or a new one from the runtime'sgrowslice, unzeroed, giving the outgrown buffer back. A buffer is own if its capacity is a power of two of at least 8 and it is not full; own growth always leaves a byte spare, so the check costs no lookup. Go's storage of another shape grows as before, zeroed throughslices.Grow, and is trimmed off the shape.go/src/lower/hand.rs). A root whose signature reaches byte storage gets a wrapper that walks its arguments on the way in (by value,&mut,Box), taking Go'sVec<u8>s of own-shaped capacity off the shape (a byte less capacity, or a copy when full) and copying each container an argument owns (aBox's pointee, aVec's element array) so the translation never writes storage of its own where Go may still hold the container. On the way out it walks its result, behind any pointer and through a slice iterator, and what its top-level&mutarguments reach, clipping own storage and every byte slice to its length, and zeroing variants an enum does not hold. The way back runs in adefer, so a panic Go recovers still leaves nothing own in what Go keeps.Buf_fillcan no longer be called past the handover.Vec::pop,truncate, andclearof non-byte elements zero what they vacate, asslices.Deletedoes, so Go code reading aVec<Vec<u8>>past its length sees nothing moved out.&mutplace mid-call; a raw pointer, which could see that handing in copied what it points into (ptr::eqagainst a copiedBoxpointee or full slice); and a reference anywhere in an argument but its outermost type (Vec<&mut Vec<u8>>,&mut &mut Vec<u8>, a by-value struct's&mutfield), which the translation could move out of what the exit walk reaches. So the exit walk follows only top-level&mutarguments, beside the result's.fixtures/handpasses byte storage every way a signature can, differentially tested against native Rust, andtests/hand/hand_test.goholds onto it from Go: results are clipped, Go's and handed-out storage survives the pool churning after the translation outgrows it (full slices included), Go's Box and an array the translation outgrows keep what Go put there, a panic still hands back, and Go's storage grown one byte per call moves no more often than doubling. Disabling the trim, the full copy, the container copies, or the clip each fails it.The stream ratios to cgo, clipped and with the rejected copy variant, are under Decisions.
faults/opfell with ownership: video encode 62.5 to 38.1 (cgo 5.4), audio 0.8 to 0.3 (cgo 0.2). Bytes allocated per op: video 1.61 MB on main, 1.19 MB clipped, 0.72 MB copied; audio encode 25.8 KB, 16.8 KB, 9.8 KB (cgo 9.5 KB). An earlier six-round run at load up to 70 ranked them the same, audio encode clipped at 2.04 to 2.10x.What is left with clipping, from a CPU profile of audio encode: copying on each grow (22%), allocating the result Go keeps (11%), and the collector (about 27%). The result is own storage up to twice its length, allocated anew each run since it never returns to the pool, so the collector runs about twice as often as for cgo's exact result.
Decisions
Decided with the maintainer on 2026-11-07, settling the choice this PR's measurements raised:
stream/video/lite-06/encodeat 2.01x wall time andstream/audio/lite-07/encodeat 2.07x CPU and 2.03x wall time, counts as noise, so the bar is met and the epic's drop rule does not apply. Rejected: copy at escape,bytesEscapecopying own storage into an exact Go buffer and recycling it (1.1 to 1.6x CPU, 1.4 to 2.0x wall on every stream; it stays measured evidence below, not shipped); and dropping rust2-go.stream/video/lite-06/encodestream/video/lite-06/decodestream/audio/lite-06/encodestream/audio/lite-06/decodestream/video/lite-07/encodestream/video/lite-07/decodestream/audio/lite-07/encodestream/audio/lite-07/decodeRatios to cgo, CPU time then wall time, medians of ten runs alternating the variants in one
quest gate --host, host load 11 to 30 of 32 cores. The copy variant was a local change tobytesEscape, never committed, so no code is removed for it; it is not kept as a bench flag, since nothing would consume it.Decided with the maintainer on 2026-11-07, at the round-6 design checkpoint (design change):
fixtures/rejectcase (popped,same). This fixes round 6's two findings: a&mutpopped out of aVec<&mut Vec<u8>>escaping the exit walk, and entry copies breakingptr::eqagainst a raw pointer. The exit walk's separateBackmode and the walks' raw-pointer arms are removed. A reference behind another&mut(&mut &mut Vec<u8>) is refused too, since reassigning the inner one loses its target the same way, and shared references are included for one rule; nothing uses either shape. It fits m3's unsafe policy (moq-dev/rust2#649): safe Rust is the contract. Rejected: keeping the general walks, recording each&muttarget on entry (more code for shapes nothing uses); merging as is (both shapes unsound).go get; unmeasured for moq-net's I/O). Recorded in quest/m4/go/README.md's Plan. Quest has no parked state, so each of the five children carries a Waiting item (Go consumers: one needs moq in pure Go ...):quest unblockedstill lists them, butquest waitingshows the item, and /quest-spawn checks that a waiting quest's condition has cleared before starting it. Rejected: splitting coverage.md into per-construct quests now; deletinggo/(the translator and measurements are kept).Earlier decisions, from the quest's Plan, that this PR implements (2026-11-07, moq-dev/rust2#609):
Vec's storage while it stays inside the translation, and hands it over safely where it reaches Go code (rejected: skipping zeroing and recycling everywhere with rules for Go callers; dropping the translator then).[len:cap]lazily at escape, measuring both, a wrapper buffer type in the Go API).stream/workload within 2x of cgo.The epic (quest/m4/go/README.md) records the result and the park.
Impact
Vec<u8>or byte slice a root function returns, or leaves behind an argument's&mut, may have no capacity past its length, so a Go caller'sappendto it copies. A Go slice passed in may come back with a byte less capacity, or as a copy when full. ABoxGo passes by value is left as it was: the translation works on a copy, as Rust's move would leave the original unread. Internal functions, such as apubtype's methods asType_method, are no longer exported. Apubfunction whose signature reaches byte storage and holds a function pointer, trait object, raw pointer, or a reference inside an argument no longer translates. Wire: none.go/bench/ffi.tomldrops excludes of crate-private items, which rust2-ffi refuses since moq-dev/rust2#607;just go benchfailed on main without it.Caused by
excludethat names nothing exported an error, whichgo/bench/ffi.toml's four excludes of crate-private items now are.Choices made without asking
Vec<u8>only, the codec's andString's storage; other element types keepslices.Grow, zeroed, without recycling.Encode(&buf)growsbufas doubling would, not by copying it whole each call.Boxor aVec's array never sees what the translation moved, grew, or replaced in it. The codec's roots own none, so the stream ratios are unchanged: the bench calls onlyBenchRun(&[u8]).sync.Pools, which the collector empties, so idle buffers are not held past two cycles.Follow-ups
None.
Validation
just go bench:TestAgreepasses for this PR and the copy variant: the translation and cgo agree on every workload.quest gate just ci: pass, at 7e26e6a, which merges origin/main (132d0ad) into the reviewed head 36a27d7; the merge touches onlyts/andquest/m3.fixtures/rejecthas the new refusals (popped,same, andcompared's raw pointer);fixtures/hand'sitercrashed translation withexpected type for param #0before its fix.BenchRun(&[u8])is none of these, so not re-measured.Models
Size: L
Implementer: Claude Code, claude-opus-5-5, high effort
Review
Written by hand, since
quest reviewran from a local branch of another name and printedrecord skipped.01a1b6be-af9f-7212-8fb8-6ca5b6965c31, round 3, started with--newafter the design changeThe iterator escape is fixed and no remaining defects were found; focused lowering probes passed, though full tests were not rerun in the read-only environment.go/src/lower/hand.rs:82:[P1] Traverse shared-reference results when handing storage out; the Out walk now goes behind any pointer (reaches). Fixed, medium,go/src/lower/hand.rs:55:[P2] Skip the iterator's lifetime when selecting its element type;fixtures/hand'sitercovers it.go/src/lower/hand.rs:64:[P1] Traverse byte storage inside iterator results;fixtures/hand'siter_nestedcovers it.01a1b3b6-3f48-7ed2-a2cd-2762e285f8f9) ended at its round-6 checkpoint; its two findings are fixed by the narrowing.🤖 Generated with Claude Code
(Written by Claude Opus 5.5 with high effort)