Skip to content

perf(go): own a Vec<u8>'s storage while only the translation reaches it - #643

Merged
kixelated merged 14 commits into
mainfrom
quest/m4/go/speed
Nov 7, 2026
Merged

kixelated merged 14 commits into
mainfrom
quest/m4/go/speed

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

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

  • Own storage, told by its shape. A Vec<u8> grows through bytesPush/bytesExtend (go/runtime/rt.go): into a power-of-two buffer from a sync.Pool per size, or a new one from the runtime's growslice, 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 through slices.Grow, and is trimmed off the shape.
  • Handover at root functions (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's Vec<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 (a Box's pointee, a Vec'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 &mut arguments reach, clipping own storage and every byte slice to its length, and zeroing variants an enum does not hold. The way back runs in a defer, so a panic Go recovers still leaves nothing own in what Go keeps.
  • Only roots are exported: internal functions' Go names start lowercase, so Buf_fill can no longer be called past the handover.
  • Vacated slots are zeroed: Vec::pop, truncate, and clear of non-byte elements zero what they vacate, as slices.Delete does, so Go code reading a Vec<Vec<u8>> past its length sees nothing moved out.
  • Refused, in the signature of a root that reaches byte storage: a function pointer or trait object, since Go code could pass one that keeps what the translation hands it, or reads a &mut place mid-call; a raw pointer, which could see that handing in copied what it points into (ptr::eq against a copied Box pointee 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 &mut field), which the translation could move out of what the exit walk reaches. So the exit walk follows only top-level &mut arguments, beside the result's.
  • fixtures/hand passes byte storage every way a signature can, differentially tested against native Rust, and tests/hand/hand_test.go holds 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/op fell 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:

  • ✅ Keep clipping as decided. The miss of the 2x bar on two workloads, stream/video/lite-06/encode at 2.01x wall time and stream/audio/lite-07/encode at 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, bytesEscape copying 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.
Workload main Clip (shipped) Copy (rejected)
stream/video/lite-06/encode 2.78x, 3.00x 1.89x, 2.01x 1.17x, 1.52x
stream/video/lite-06/decode 2.56x, 2.74x 1.53x, 1.73x 1.33x, 1.83x
stream/audio/lite-06/encode 3.08x, 3.03x 1.96x, 1.97x 1.38x, 1.67x
stream/audio/lite-06/decode 2.75x, 2.81x 1.37x, 1.68x 1.50x, 1.89x
stream/video/lite-07/encode 2.70x, 2.92x 1.39x, 1.47x 1.14x, 1.42x
stream/video/lite-07/decode 2.56x, 2.92x 1.36x, 1.54x 1.16x, 1.44x
stream/audio/lite-07/encode 3.49x, 3.42x 2.07x, 2.03x 1.34x, 1.56x
stream/audio/lite-07/decode 2.94x, 2.89x 1.41x, 1.70x 1.63x, 1.99x

Ratios 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 to bytesEscape, 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):

  • ✅ Narrow the handover. A root whose signature reaches byte storage refuses a reference inside an argument below its outermost type and any raw pointer, each with its span and a fixtures/reject case (popped, same). This fixes round 6's two findings: a &mut popped out of a Vec<&mut Vec<u8>> escaping the exit walk, and entry copies breaking ptr::eq against a raw pointer. The exit walk's separate Back mode 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 &mut target on entry (more code for shapes nothing uses); merging as is (both shapes unsound).
  • ✅ Park the Go epic until a consumer asks. Speed met its bar (the table below). No consumer has asked for pure Go, and the cgo exporter stays moq's Go route, so the epic, coverage.md, and the other Go quests (runtime-names, enums, trait-impls, alias-methods) don't start until one needs it; then first weigh moq compiled to WASM running inside Go on a pure-Go runtime such as wazero (no cgo, plain 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 unblocked still lists them, but quest waiting shows 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; deleting go/ (the translator and measurements are kept).

Earlier decisions, from the quest's Plan, that this PR implements (2026-11-07, moq-dev/rust2#609):

  • ✅ The translation owns a 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).
  • ✅ Escape hides spare capacity by clipping (rejected: zeroing [len:cap] lazily at escape, measuring both, a wrapper buffer type in the Go API).
  • ✅ The bar is unchanged: every stream/ workload within 2x of cgo.

The epic (quest/m4/go/README.md) records the result and the park.

Impact

  • Public API: a 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's append to it copies. A Go slice passed in may come back with a byte less capacity, or as a copy when full. A Box Go 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 a pub type's methods as Type_method, are no longer exported. A pub function 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.toml drops excludes of crate-private items, which rust2-ffi refuses since moq-dev/rust2#607; just go bench failed on main without it.

Caused by

  • moq-dev/rust2#607 - made a config exclude that names nothing exported an error, which go/bench/ffi.toml's four excludes of crate-private items now are.

Choices made without asking

  • Own storage covers Vec<u8> only, the codec's and String's storage; other element types keep slices.Grow, zeroed, without recycling.
  • Ownership is told by shape rather than a table of owned buffers, which would add a lock and map operations to every grow and leak entries for buffers dropped inside the translation. Not measured.
  • Go-held storage the translation grows stays Go's: zeroed growth, trimmed off the own shape, never clipped, so a Go loop calling Encode(&buf) grows buf as doubling would, not by copying it whole each call.
  • Containers an argument owns are copied on the way in rather than tracked, so Go code holding a Box or a Vec'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 only BenchRun(&[u8]).
  • The pools are sync.Pools, which the collector empties, so idle buffers are not held past two cycles.

Follow-ups

None.

Validation

  • just go bench: TestAgree passes 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 only ts/ and quest/m3.
  • fixtures/reject has the new refusals (popped, same, and compared's raw pointer); fixtures/hand's iter crashed translation with expected type for param #0 before its fix.
  • The ratios above were measured before round 5's fixes and the narrowing; those change only roots that own a container, take a full slice, or are now refused, and the bench's 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 review ran from a local branch of another name and printed record skipped.

  • Reviewer: Codex, gpt-6.1-sol, xhigh effort
  • Session: 01a1b6be-af9f-7212-8fb8-6ca5b6965c31, round 3, started with --new after the design change
  • Head: 36a27d7, then a conflict-free merge of origin/main as 7e26e6a
  • Base: origin/main at 132d0ad
  • Score: 10/10, The 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.
  • Findings: none.
  • Earlier rounds of this session:
    • Round 1, at 1c5827f: fixed, high, 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's iter covers it.
    • Round 2, at 6242af2: fixed, high, go/src/lower/hand.rs:64: [P1] Traverse byte storage inside iterator results; fixtures/hand's iter_nested covers it.
  • The previous session (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)

Quest-Claim: c686a62dbe6127f6
@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

kixelated and others added 3 commits November 7, 2026 04:29
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>
@kixelated kixelated changed the title Match cgo on the crate perf(go): own a Vec<u8>'s storage while only the translation reaches it Nov 7, 2026
kixelated and others added 4 commits November 7, 2026 06:38
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
@kixelated

Copy link
Copy Markdown
Collaborator Author

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 go/fixtures/reject case:

  • a raw pointer anywhere in its signature, so no argument can see that handing in copied a Box's pointee or a full slice (round 6's ptr::eq(&*b, p) finding);
  • a reference anywhere in an argument but its outermost type, such as Vec<&mut Vec<u8>>, &mut &mut Vec<u8>, or a by-value struct's &mut field, so the translation cannot move a &mut out of what handing back walks (round 6's refs.pop() finding).

With nested references refused, the only places of Go's the translation can write are behind a top-level &mut argument, so the exit walk is the ordinary Out walk of those arguments, and the separate Back mode and the walks' raw-pointer arms are removed. Results are still walked whole after the call, which no later mutation can escape.

Rejected: keeping the general walks and recording each &mut target on entry (more code for shapes nothing uses), and merging as is (both shapes unsound). The next review starts a new session with quest review --new.

(Written by Claude Opus 5.5 with high effort)

kixelated and others added 6 commits November 7, 2026 08:00
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>
@kixelated
kixelated marked this pull request as ready for review November 7, 2026 16:38
@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e4c0270f-448b-4d5a-8fd0-7de5cb207874
Commits

Reviewing files that changed from the base of the PR and between 132d0ad and 7e26e6a.

Files ignored due to path filters: 1

  • go/Cargo.lock is excluded by !**/*.lock
Files selected for processing: 21

  • go/Cargo.toml
  • go/README.md
  • go/bench/ffi.toml
  • go/fixtures/hand/Cargo.toml
  • go/fixtures/hand/src/lib.rs
  • go/fixtures/reject/expected.txt
  • go/fixtures/reject/src/lib.rs
  • go/runtime/rt.go
  • go/runtime/rt_fmt.go
  • go/src/lower/hand.rs
  • go/src/lower/library.rs
  • go/src/lower/mod.rs
  • go/tests/fixtures.rs
  • go/tests/hand/hand_test.go
  • quest/m4/go/README.md
  • quest/m4/go/alias-methods.md
  • quest/m4/go/coverage.md
  • quest/m4/go/enums.md
  • quest/m4/go/runtime-names.md
  • quest/m4/go/speed.md
  • quest/m4/go/trait-impls.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))

Comment thread go/src/lower/hand.rs
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}) }}")),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@kixelated
kixelated added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit 7a0578a Nov 7, 2026
7 checks passed
@kixelated
kixelated deleted the quest/m4/go/speed branch November 7, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant