alkblobs/tasks/c4-borrowed-key-overloads.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
c4-borrowed-key-overloads	C-4 — borrowed-key overloads on serving paths (the as_bytes allocation nit)	completed	c1-git-object-family-impl	narrow	low	component	implementation	wave-13 review-002 keys performance

Description

Review-002 C-4 (nit, non-blocking — keep it small). Key::as_bytes() allocates (src/keys.rs:53 — returns Vec<u8>), and hot-path callers reach for it per call (src/local.rs:899 is a serving-path derive+as_bytes site). The consumer's serving loops (custom-Find pack generation, ranged serving) pay it once per object lookup — measured harmless at POC scale, which is exactly why this is a nit. Borrowed-key overloads would drop the per-call allocation on serving paths.

The borrowed shape is the task's design point. The physical encoding is tag byte || digest bytes, but the tag is not stored adjacent to the digest bytes today (Key wraps Digest; the tag is computed at encode time), so a true &self -> &[u8] return is not free. The candidate shapes:

• a caller-buffer fill (write_key_bytes(&self, out: &mut [u8]) or similar) — zero allocation, the call sites own a small stack buffer;
• a small Copy wrapper (a fixed-capacity key-bytes type with Deref<Target = [u8]>) — ergonomic at the call sites, still no heap;
• storing the tag inside Key so a borrowed slice becomes possible — an internal-representation change (the key bytes are the one-way door, not the Rust layout; pre-publish is the cheap window, but only take this if the call sites want it).

Pick per the call sites' shapes; convert the serving-path sites that do not need ownership (local.rs:899's neighborhood first; the ops pumps' key-hex sites are string-shaped, likely untouched). No behavior change; existing tests green unmodified except where the API surface moved.

Acceptance Criteria

• The borrowed overload exists (the shape decided and recorded in Notes) and the serving-path call sites that don't need ownership use it
• No behavior change; the round-trip invariant (from_bytes inverts the encoding) holds for the new shape (unit-tested)
• Gates green: cargo test (default), clippy --all-targets -- -D warnings, cargo fmt --check

References

• docs/reviews/002-first-consumer-review.md (C-4)
• src/keys.rs:53 (as_bytes); src/local.rs:899 (the serving-path site)
• /workspace/@alkdev/alkgit/docs/research/substrate-recomposition.md (upstream-ask 3 — the "non-load-bearing" judgment)

Notes

Shape decided: a small Copy wrapper — KeyBytes (src/keys.rs), a fixed-capacity [u8; 33] (tag + digest at the widest) with len, Deref<Target = [u8]>, as_slice, and slice-based Eq/Ord/Hash (the written prefix's tag-first compare is exactly the keys' total order, since tag order is the registry order and equal tags imply equal lengths). Minted by Key::key_bytes(); Key::as_bytes() now delegates to it (one encoding source, still returns the owned Vec). Not stored adjacent today, so a true &self -> &[u8] was impossible; the wrapper was the cheapest shape that deref-coerces — every &key.as_bytes() borrow temporary became &key.key_bytes() with nothing else moved, and the third candidate (storing the tag in Key, an internal-layout change) was unnecessary: the call sites are single temporaries.

Converted sites (borrow-temporaries on serving/renewal paths):

• src/store/read.rs — get/stat/read_range/has (the decode-at-boundary-then-re-encode-per-op sites, one alloc per lookup dropped)
• src/store/sweep.rs — delete, arbitrated_delete's SQL arbitration consult, delete_everywhere, and the mark side's window staging (as_bytes().to_vec() -> key_bytes().to_vec(), a double allocation dropped)
• src/ops/fetch.rs — the whole establishment get and both ranged-serving loops (fetch_establish_ranges, fetch_pump_ranges — per-range allocs dropped)
• src/ops/specs.rs — blobs/stat, blobs/have, blobs/delete
• src/ops/tokens.rs — the fleet pin-renewal cadence (ops_pin_check/ops_pin_renew)

Anchor correction (review-002's local.rs:899): in the current tree that line resolves to key_for, a #[cfg(test)] helper in local.rs that returns an owned Vec<u8> consumed by ~30 call sites (moves across awaits, Vec<Vec<u8>> comparisons) — a test helper, not a serving path; it stays by shape (ownership is what its callers want, and converting it would churn the test module for zero serving-path gain). The real serving-path anchors are the converted sites above; the sweep's test modules now exercise key_bytes as a side effect of the mechanical conversion.

Deliberately untouched: the write-path borrow-temporaries (src/store/put.rs's four kv().put/stage.commit sites; src/store/fleet.rs's joint_put/stage.commit/stage.publish) — out of the task's serving-path scope; fleet_pin_row (needs the owned Vec for PinRow) and spawn_pin_renewal (moves the bytes into the renewal task) genuinely need ownership. Ops key-hex sites are string-shaped, untouched per the task's expectation.

Summary

Review-002 C-4 landed: Key::key_bytes() + the KeyBytes Copy wrapper (fixed-capacity stack bytes, Deref<Target = [u8]>, slice-based ordering equal to the keys' total order) exported from lib.rs; as_bytes() delegates to it. All serving-path borrow temporaries converted (store read/delete paths, ops fetch/stat/have/ delete, fleet pin-renewal); per-call heap allocations dropped from those sites with zero behavior change. New unit tests: borrowed round-trip (from_bytes inverts key_bytes for all four algorithms) and borrowed-vs-typed ordering agreement. Gates: cargo test (default) green, cargo test --all-features green against the live pg harness (338+125 tests), clippy default and --all-features --all-targets -- -D warnings clean, cargo fmt --check clean, cargo doc --no-deps clean, fuzz/shared corpus replay green.