From 9e4428f730588ddf30d61ee405031ce8feff06f9 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:55:29 +0100 Subject: [PATCH 1/2] perf: name origin's default branch once per claim operation A reservation named origin's default branch three times, two git processes each: once for its fetch and twice in judge, once for the default branch's records and once to ask whether there was one. A view named it twice: for those records and for the generated files. A drain makes both on every move, and in one drain test these lookups were about a tenth of its 1,107 git processes. judge now resolves the name once, after the act's fetch, and hands it to defaultRecords; a view resolves it once and reads the generated files under it through generatedFilesAt, the exported GeneratedFiles keeping its form. The name is never carried across a fetch, which can create or move origin/HEAD, and every operation resolves it afresh, so a default branch that changes is seen at the next one. Tests pin a reservation at two lookups, one each side of its fetch, and a view at one; the drain test falls to 1,019 git processes. Refs: iss-2610180545134593 Assisted-by: Claude:claude-opus-5-5 --- internal/core/claim/calls_test.go | 56 +++++++++++++++++++++++++++++++ internal/core/claim/generated.go | 8 ++++- internal/core/claim/handback.go | 9 +++-- internal/core/claim/remote.go | 7 ++-- internal/core/claim/view.go | 7 ++-- 5 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 internal/core/claim/calls_test.go diff --git a/internal/core/claim/calls_test.go b/internal/core/claim/calls_test.go new file mode 100644 index 0000000000..090b48e2ae --- /dev/null +++ b/internal/core/claim/calls_test.go @@ -0,0 +1,56 @@ +package claim + +import ( + "strings" + "testing" +) + +// defaultLookups counts the times the traced commands resolved origin's +// default branch: originDefault traces its symbolic-ref once per resolution. +func defaultLookups(l *gitLog) int { + l.mu.Lock() + defer l.mu.Unlock() + n := 0 + for _, c := range l.cmds { + if c[0] == "symbolic-ref" && strings.HasSuffix(c[len(c)-1], "/HEAD") { + n++ + } + } + return n +} + +// TestAReservationResolvesTheDefaultBranchOnceEachSideOfItsFetch +// (iss-2610180545134593): one reservation names origin's default branch +// twice, no more. Once before its fetch, to name the branch the fetch brings +// in, and once after it, for everything that reads the branch as fetched: a +// fetch can create or move origin/HEAD, so the name is never carried across +// one. Each lookup is two git processes, and a reservation used to make five. +func TestAReservationResolvesTheDefaultBranchOnceEachSideOfItsFetch(t *testing.T) { + w := newWorld(t) + p := w.clone("1234+octo@users.noreply.github.com", "") + log := traced(t) + if _, err := p.reserve("iss-1", "commands"); err != nil { + t.Fatal(err) + } + if n := defaultLookups(log); n != 2 { + t.Errorf("one reservation resolved origin's default branch %d times; want 2, once each side of its fetch", n) + } +} + +// TestAViewResolvesTheDefaultBranchOnce: a view fetches nothing, so it names +// origin's default branch once, for the records it reads there and the +// generated files it recognises; it used to name it once for each. +func TestAViewResolvesTheDefaultBranchOnce(t *testing.T) { + w := newWorld(t) + p := w.clone("1234+octo@users.noreply.github.com", "") + if _, err := p.reserve("iss-1", "commands"); err != nil { + t.Fatal(err) + } + log := traced(t) + if _, err := View(p.dir, w.clock()); err != nil { + t.Fatal(err) + } + if n := defaultLookups(log); n != 1 { + t.Errorf("one view resolved origin's default branch %d times; want 1", n) + } +} diff --git a/internal/core/claim/generated.go b/internal/core/claim/generated.go index 954d43a02c..7fef7a7f08 100644 --- a/internal/core/claim/generated.go +++ b/internal/core/claim/generated.go @@ -56,7 +56,13 @@ func (g Generated) Is(p string) bool { // root .gitattributes); otherwise the set is empty and the reason says why // generated files were not recognised. func GeneratedFiles(root string) (Generated, string) { - ref := originDefault(root) + return generatedFilesAt(root, originDefault(root)) +} + +// generatedFilesAt is GeneratedFiles read from ref, origin's default branch as +// the caller resolved it ("" for none), so an operation that has named the +// branch once reads it under that one name. +func generatedFilesAt(root, ref string) (Generated, string) { if ref == "" { return Generated{}, "" } diff --git a/internal/core/claim/handback.go b/internal/core/claim/handback.go index c35739e0c4..7324190a55 100644 --- a/internal/core/claim/handback.go +++ b/internal/core/claim/handback.go @@ -239,11 +239,10 @@ func validBlob(s string) bool { return true } -// defaultRecords is every record origin's default branch holds as last -// fetched, by id, each with its folders and blobs; nil when this checkout -// names no default branch of origin. -func defaultRecords(root string) (map[string][]peers.RecordAt, error) { - ref := originDefault(root) +// defaultRecords is every record origin's default branch, ref as the caller +// resolved it, holds as last fetched, by id, each with its folders and blobs; +// nil when ref is "", where this checkout names no default branch of origin. +func defaultRecords(root, ref string) (map[string][]peers.RecordAt, error) { if ref == "" { return nil, nil } diff --git a/internal/core/claim/remote.go b/internal/core/claim/remote.go index 816d33411e..20eaaa60b3 100644 --- a/internal/core/claim/remote.go +++ b/internal/core/claim/remote.go @@ -604,13 +604,16 @@ func judge(p *prepared, tree string) (judged, error) { // no one edited it since, and a person's edit this copy has not // fetched reads as unchanged until it is (dcn-2610161412283450, // refined by dcn-2610161837009957). One with no blob read stands. - recs, err := defaultRecords(p.root) + // One name for the default branch, resolved after the act's fetch, + // which can create or move origin/HEAD (iss-2610180545134593). + ref := originDefault(p.root) + recs, err := defaultRecords(p.root, ref) if err != nil && p.derivedGap == "" { return judged{}, refuse("could not read whether %s is finished on the default branch: %s; %s", p.id, redacted(err), noReservation) } switch { - case originDefault(p.root) == "": + case ref == "": j.blobUnread = "this checkout names no default branch of " + Remote case err != nil: j.blobUnread = p.derivedGap + "; " + redacted(err) diff --git a/internal/core/claim/view.go b/internal/core/claim/view.go index 16af69e5b6..c4c88c0a3a 100644 --- a/internal/core/claim/view.go +++ b/internal/core/claim/view.go @@ -184,7 +184,10 @@ func viewOf(root string, now time.Time, derive bool, only func(subject string) b // subjects alone, which no record there ends. var records map[string][]peers.RecordAt anyRecord := slices.ContainsFunc(refs, func(ref string) bool { return !isRelease(strings.TrimPrefix(ref, TrackingPrefix)) }) - if ref := originDefault(root); derive && anyRecord && ref != "" { + // One name for origin's default branch per view (iss-2610180545134593): a + // view fetches nothing, so the name cannot move under it. + defaultRef := originDefault(root) + if ref := defaultRef; derive && anyRecord && ref != "" { if records, err = peers.RecordsAt(root, ref); err != nil { return snap, fmt.Errorf("reading %s: %s", gitutil.ShortRef(ref), redacted(err)) } @@ -228,7 +231,7 @@ func viewOf(root string, now time.Time, derive bool, only func(subject string) b // origin's default branch recognises none. if slices.ContainsFunc(snap.Reservations, func(r Reservation) bool { return r.State == StateLive && len(r.Footprint) > 0 }) { if derive { - snap.Generated, snap.GeneratedUnread = GeneratedFiles(root) + snap.Generated, snap.GeneratedUnread = generatedFilesAt(root, defaultRef) } else { snap.GeneratedUnread = "origin's default branch could not be read when the reservations were fetched" } From 91364815b279d6dbc0b99921b2077da12ee0b3db Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Sun, 18 Oct 2026 13:23:26 +0100 Subject: [PATCH 2/2] test: say what the default-branch call count measures Review notes: the reservation test said a reservation names the default branch twice, no more, but that holds where origin holds no other reservation; with others there, the crowding check's view names it once more after the same fetch. It also gave the old count as five, where the case it measures made three. The comment and the failure text now say both. Refs: iss-2610180545134593 Assisted-by: Claude:claude-opus-5-5 --- internal/core/claim/calls_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/internal/core/claim/calls_test.go b/internal/core/claim/calls_test.go index 090b48e2ae..b773611c46 100644 --- a/internal/core/claim/calls_test.go +++ b/internal/core/claim/calls_test.go @@ -20,11 +20,14 @@ func defaultLookups(l *gitLog) int { } // TestAReservationResolvesTheDefaultBranchOnceEachSideOfItsFetch -// (iss-2610180545134593): one reservation names origin's default branch -// twice, no more. Once before its fetch, to name the branch the fetch brings -// in, and once after it, for everything that reads the branch as fetched: a -// fetch can create or move origin/HEAD, so the name is never carried across -// one. Each lookup is two git processes, and a reservation used to make five. +// (iss-2610180545134593): a reservation where origin holds no other names +// origin's default branch twice: once before its fetch, to name the branch the +// fetch brings in, and once after it, in judge, for the default branch's +// records. A fetch can create or move origin/HEAD, so the name is never +// carried across one. Each lookup is two git processes; judge used to make +// two of its own, so this reservation made three. Where origin holds other +// reservations, the crowding check's view names the branch once more, after +// the same fetch, as a view does (TestAViewResolvesTheDefaultBranchOnce). func TestAReservationResolvesTheDefaultBranchOnceEachSideOfItsFetch(t *testing.T) { w := newWorld(t) p := w.clone("1234+octo@users.noreply.github.com", "") @@ -33,7 +36,7 @@ func TestAReservationResolvesTheDefaultBranchOnceEachSideOfItsFetch(t *testing.T t.Fatal(err) } if n := defaultLookups(log); n != 2 { - t.Errorf("one reservation resolved origin's default branch %d times; want 2, once each side of its fetch", n) + t.Errorf("a reservation with no other on origin resolved origin's default branch %d times; want 2, once each side of its fetch", n) } }