Skip to content

perf: read a view's reservation tips in one batch - #1100

Merged
REPPL merged 4 commits into
mainfrom
fix/claim-tip-batch
Oct 18, 2026
Merged

REPPL merged 4 commits into
mainfrom
fix/claim-tip-batch

Conversation

@REPPL

@REPPL REPPL commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

What this changes

A view read each reservation tip in three git processes: its name (for-each-ref), its size (cat-file -s) and its body (cat-file commit). A drain views on every move, so it paid three per reservation per move. The view now reads the ordinary tips, commits within the object bound, in three processes however many there are:

  1. one for-each-ref listing the reservation refs with their objects and types;
  2. one cat-file --batch-check for the commits' sizes;
  3. one cat-file --batch for their bodies.

Each runs through claim's storedGit (history as stored, replace refs off) and is capped. Nothing is kept beyond the call: every ref is resolved to its commit afresh each time.

A trust boundary

The tips come from origin, written by whoever can push to the reservation namespace, so the batch is read as untrusted and is a shortcut only.

  • What the batch accepts: a body is taken from it only under the exact name and size the check read. A tip it does not answer that way is read on its own, exactly as before, with every bound and refusal that read applies. That covers a missing object, a non-commit, a tag, an object past the bound, and an answer out of shape.
  • Bad answers stay contained: the cut stops at the first answer it cannot read, so one bad tip never shifts another's body under its name, and a missing object costs only itself.
  • Ref names: only refs the view has already vetted reach the batch, matched exactly.
  • Memory: one batch reads at most 1 MiB of commit bodies, about 126 tips at the object bound. A tip past that is read on its own, so a view's memory stays flat however many reservation refs a writer pushes.

A bug the review caught. Round one of review found that the batch handed the parser the bytes as git sent them, while storedGit trims trailing whitespace. A writer could push a reservation whose message was the message cap plus its newline, or ended in blanks. The view would then read it as unreadable, with no holder or footprint, while every act read it as a live hold. The batch now trims exactly as storedGit does (the second commit).

Effect

One drain test in internal/core/implement/loop:

git processes
Before #1099 1,107
After #1099 1,019
After this change 830

A view of four tips now runs 2 cat-file processes instead of 8.

Evidence

  • Parser: TestTheCommitBatchIsCutOnlyWhereItReadsAsAsked cuts crafted answers: a missing object mid-batch, a wrong size or type, an answer out of order, a short read, a body without its newline, and bytes past the end.
  • Equivalence: TestABatchedViewReadsEveryTipAsItsOwnReadWould and TestABatchedReadAgreesWithItsOwnReadAtEveryBound read each tip both ways and hold them equal. The tips include an oversized commit between two readable ones; messages either side of the message cap, raw and trimmed; one ending in blanks; commits of exactly the object bound and one past it; two refs on one commit; a blob; and a tag. They also assert which tips the batch itself answered, so a silent fallback cannot pass. The second fails without the trim, and also with a trim of newlines alone.
  • Budget: TestATipPastTheBatchBudgetIsReadOnItsOwn holds tips past the budget to their own read. It fails if the budget is ignored.
  • Process count: TestAViewReadsItsTipsInAFixedNumberOfProcesses pins the view's process count.
  • Suites: claim, peers, loop and CLI pass.
  • Preflight: make preflight passed on this commit (receipt minted).
  • Review: Opus and Sonnet security reviewers on frozen copies, briefed as parsing at a trust boundary. Round one: STOP on the trim divergence, which each confirmed with a probe. Round two: both LAND, with their probes re-run, and a budget probe of 140 and 330 tips, every tip matching its own read.

Resolves: iss-2610180545134593

Assisted-by: Claude:claude-opus-5-5

REPPL added 4 commits October 18, 2026 13:16
A view read each reservation tip in three git processes, its name, its size
and its body, so a drain, which views on every move, paid three per
reservation per move. The view now reads the ordinary tips, commits within
the object bound, in three processes however many there are: one listing of
the reservation refs with their objects and types, one `cat-file
--batch-check` for the commits' sizes, and one `cat-file --batch` for their
bodies, each read as stored and capped.

The tips come from origin, written by whoever can push to the reservation
namespace, so the batch is read as untrusted and is a shortcut only. A body
is taken from it only under the name and size the check read; a tip it does
not answer that way (missing, not a commit, past the bound, answered out of
shape) is read on its own exactly as before, with every bound and refusal
that read applies. The cut stops at the first answer it cannot read, so one
bad tip never shifts another's body under its name, and a missing object
costs only itself. Nothing outlives the call: each ref is resolved afresh.

Tests cut crafted batch answers (a missing object mid-batch, a wrong size or
type, an answer out of order, a short read, a body without its newline),
hold a batched read of a mixed set (an oversized tip between two readable
ones, a malformed one, a blob) equal to each tip read on its own, and pin a
view of four tips at two cat-file processes, from eight. One drain test falls
from 1,019 git processes to 830, and from 1,107 before the default-branch
change it builds on.

Resolves: iss-2610180545134593

Assisted-by: Claude:claude-opus-5-5
Both reviewers of the batched view found it parted from a tip read on its
own: storedGit hands a commit over with trailing whitespace trimmed, and
the batch handed parseTip the bytes as git sent them. A writer to the
reservation namespace could push a reservation whose message was the
message cap plus its newline, or ended in blanks, and the view would read
it as unreadable, with no holder or footprint, while every act read it as
a live hold. A batched body is now trimmed exactly as storedGit trims it.

One batch also reads at most 1 MiB of commit bodies; a tip past that is
read on its own, so the memory a view holds stays flat however many
reservation refs a writer pushes.

A test reads, both ways and held equal, the tips where the two could part:
messages either side of the cap, one ending in blanks, commits of exactly
the object bound and one past it, two refs on one commit and a tag as a
tip, and asserts which of them the batch itself answered. It fails at the
cap plus its newline without the trim, and at the blanks with a trim of
newlines alone.

Refs: iss-2610180545134593

Assisted-by: Claude:claude-opus-5-5
Both reviewers of the batched view asked for the budget fallback to be
pinned, not only probed. The budget is now a parameter below
readSeenTips, which passes the 1 MiB constant, and a test with room for
two of five tips holds the first two in the batch and every tip, inside
the budget or past it, equal to its read on its own. The budget comment
names the count it allows at the object bound, about 126.

Refs: iss-2610180545134593

Assisted-by: Claude:claude-opus-5-5
Takes in the default-branch change's test-comment fix, so the batched view
builds on that change as it is to land.

Assisted-by: Claude:claude-opus-5-5
@REPPL
REPPL enabled auto-merge October 18, 2026 16:00
@REPPL
REPPL added this pull request to the merge queue Oct 18, 2026
Merged via the queue into main with commit 0fccc78 Oct 18, 2026
14 checks passed
@REPPL
REPPL deleted the fix/claim-tip-batch branch October 18, 2026 17:10
@REPPL

REPPL commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

Merge-group times after this change and #1099 (2026-10-18):

Run ubuntu leg its race step macOS leg its race pass
#1095 (before both) 18.3 min — 43.9 min 41.9 min
#1099 23.3 min 20.8 min 49.4 min 47.4 min
#1100 (this) 23.1 min 20.6 min 44.8 min 42.4 min

The two changes do not show on CI. The ubuntu race step is at the top of its earlier 14 to 20.6 minutes, its package times sum to about the same CPU as before (4,243 and 4,293 seconds against 4,254), and the loop package's race time is inside its earlier range. The cut in git processes (one drain test, 1,107 to 830) is real where it was measured, but smaller than the difference between runners. The resolved record's grounds named exactly this as what would show them wrong, so the record now says so.

The macOS leg is above 42 minutes on both runs, and #1099's came within 0.6 minutes of the 50-minute warning, so its headroom is captured as iss-2610181711260494 (minor), with the nine-run spread of 35.2 to 49.4 minutes.

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