diff --git a/cache_options.go b/cache_options.go index 03bf3ae..51159b3 100644 --- a/cache_options.go +++ b/cache_options.go @@ -9,3 +9,17 @@ func WithHunkDedupBudget(bytes int) ScannerOption { hs.hunkDedupBudget = bytes } } + +// WithHunkDedupRetainedBudget bounds the hunk bytes a deduplicating scan +// holds between a worker computing them and the consumer callback seeing +// them. The sequencer stops handing out new pairs while the charge is above +// this budget, so the bound trades scan throughput for peak memory. The +// default of 512 MiB keeps every worker busy through a multi-megabyte blob +// on large hosts. Values below 1 MiB are raised to 1 MiB so a single large +// hunk can always be admitted. +func WithHunkDedupRetainedBudget(bytes int) ScannerOption { + bytes = max(bytes, 1<<20) + return func(hs *HistoryScanner) { + hs.dedupLimits.retainedBytesCap = int64(bytes) + } +} diff --git a/commit_enum.go b/commit_enum.go new file mode 100644 index 0000000..e43ba22 --- /dev/null +++ b/commit_enum.go @@ -0,0 +1,405 @@ +// commit_enum.go +// +// Pack-order commit enumeration for the ref-walk commit load. +// +// A reachable walk over the commit DAG discovers each commit only after +// inflating its child, so its critical path is the first-parent chain: one +// header inflation after another, however many workers run. On the rails +// history that chain held the sixteen-worker walk to 176 ms at a quarter of +// its CPU capacity. The pack index already names every object, so the +// commits stored whole in the packs are read here in parallel, index range +// by index range, and the reachable walk then runs over them in memory, +// touching the store only for commits the enumeration did not cover: +// delta-encoded commits (under 1% of rails), loose commits, and tags. + +package objstore + +import ( + "runtime" + "sync" + "sync/atomic" +) + +// enumChunk is how many index entries an enumeration worker claims at a +// time; a contiguous range keeps its oidTable and entry reads sequential. +const enumChunk = 2048 + +// enumeratePackedCommits parses every commit object stored whole (not as a +// delta) in the store's packs and returns their commitInfos in no particular +// order. Objects that fail to inflate or parse are left out rather than +// reported: an unreachable object cannot fail a scan, and a reachable one is +// read again, with its error, by the walk that follows. +func (s *store) enumeratePackedCommits(numWorkers int) []commitInfo { + type packRange struct { + pf *idxFile + start int + } + var ranges []packRange + for _, pf := range s.packs { + for start := 0; start < len(pf.entries); start += enumChunk { + ranges = append(ranges, packRange{pf, start}) + } + } + if len(ranges) == 0 { + return nil + } + numWorkers = max(1, min(numWorkers, len(ranges))) + + var ( + next atomic.Int64 + mu sync.Mutex + results [][]commitInfo + wg sync.WaitGroup + ) + for range numWorkers { + wg.Add(1) + go func() { + defer wg.Done() + var out []commitInfo + scratch := getDeltaScratch(maxOneShotCommitBytes) + defer putDeltaScratch(scratch) + for { + i := int(next.Add(1) - 1) + if i >= len(ranges) { + break + } + r := ranges[i] + end := min(r.start+enumChunk, len(r.pf.entries)) + for j := r.start; j < end; j++ { + off := r.pf.entries[j].offset + var hdr [32]byte + n, _ := r.pf.pack.ReadAt(hdr[:], int64(off)) + typ, size, hdrLen := parseObjectHeaderUnsafe(hdr[:n]) + if hdrLen <= 0 || typ != ObjCommit || size > maxOneShotCommitBytes { + continue + } + buf := scratch.buf[:size] + if err := inflateExact(r.pf.pack, int64(off)+int64(hdrLen), buf); err != nil { + continue + } + // parseCommitInfoFromHeader stops at the blank line + // that ends the header, so the message is never read. + info, err := parseCommitInfoFromHeader(r.pf.oidTable[j], buf) + if err != nil { + continue + } + out = append(out, info) + } + } + mu.Lock() + results = append(results, out) + mu.Unlock() + }() + } + wg.Wait() + + total := 0 + for _, r := range results { + total += len(r) + } + all := make([]commitInfo, 0, total) + for _, r := range results { + all = append(all, r...) + } + return all +} + +// enumShards is the number of OID-prefix shards the enumerated-commit index +// is split into so that it can be built and queried in parallel; a power of +// two. +const enumShards = 16 + +// enumReadQueue bounds the uncovered OIDs queued for the reader pool and +// the finished reads waiting to be folded back into the walk. Uncovered +// commits are rare (delta-encoded commits, tags, loose commits), and the walk +// keeps going over the enumerated commits while they are read. +const enumReadQueue = 256 + +// enumIndex maps enumerated commit OIDs to their positions, sharded by the +// leading OID bits. +type enumIndex [enumShards]map[Hash]int32 + +func (ix *enumIndex) lookup(oid Hash) (int32, bool) { + i, ok := ix[int(oid[0])>>4][oid] + return i, ok +} + +// loadCommitsReachable returns every commit reachable from tips, in no +// particular order, with each commit's parents resolved to positions in the +// result (-1 for a parent absent from the store). Commits the pack +// enumeration covered are followed in memory over their parent indexes; the +// walk reads the rest through walkOne, in parallel batches, with the same +// handling of tags, stale refs and shallow boundaries as the ref walk. +func (hs *HistoryScanner) loadCommitsReachable(tips []Hash) ([]commitInfo, commitParents, error) { + numWorkers := runtime.NumCPU() + enumerated := hs.store.enumeratePackedCommits(numWorkers) + + var ix enumIndex + for sh := range enumShards { + ix[sh] = make(map[Hash]int32, len(enumerated)/enumShards+1) + } + var wg sync.WaitGroup + for sh := range enumShards { + wg.Add(1) + go func() { + defer wg.Done() + m := ix[sh] + for i := range enumerated { + if oid := &enumerated[i].OID; int(oid[0])>>4 == sh { + m[*oid] = int32(i) + } + } + }() + } + wg.Wait() + + // parentIdx holds each enumerated commit's parents as indexes into + // enumerated, -1 for a parent the enumeration did not cover, in one + // flat array: parentIdx[parentStart[i]:parentStart[i+1]]. + parentStart := make([]int32, len(enumerated)+1) + for i := range enumerated { + parentStart[i+1] = parentStart[i] + int32(len(enumerated[i].ParentOIDs)) + } + parentIdx := make([]int32, parentStart[len(enumerated)]) + var next atomic.Int64 + const resolveChunk = 4096 + for range min(numWorkers, len(enumerated)/resolveChunk+1) { + wg.Add(1) + go func() { + defer wg.Done() + for { + start := int(next.Add(resolveChunk) - resolveChunk) + if start >= len(enumerated) { + return + } + for i := start; i < min(start+resolveChunk, len(enumerated)); i++ { + dst := parentIdx[parentStart[i]:parentStart[i+1]] + for j, p := range enumerated[i].ParentOIDs { + if k, ok := ix.lookup(p); ok { + dst[j] = k + } else { + dst[j] = -1 + } + } + } + } + }() + } + wg.Wait() + + visited := make([]bool, len(enumerated)) + seenOther := make(map[Hash]struct{}) + // The walk itself touches only the int32 arrays above, which fit the + // cache, and records the enumerated commits it reaches as visitOrder; + // the result is assembled from it afterwards, in parallel. others + // collects the store-read commits in the order their reads were + // consumed; they follow the enumerated commits in the result. + visitOrder := make([]int32, 0, len(enumerated)) + var others []commitInfo + var stack []int32 // enumerated commits to visit + var classify func(Hash) + + // Uncovered OIDs go to a reader pool through readReq while the walk + // continues over the enumerated commits; each read comes back on + // readRes with the commit (or nothing, for a stale ref or a tag whose + // target is pushed) and the OIDs to classify next. inFlight counts + // reads requested and not yet consumed. + type read struct { + infos []commitInfo + pushes []Hash + err error + } + readReq := make(chan Hash, enumReadQueue) + readRes := make(chan read, enumReadQueue) + readers := min(numWorkers, enumReadQueue) + for range readers { + wg.Add(1) + go func() { + defer wg.Done() + for oid := range readReq { + var r read + hs.walkOne(oid, + func(info commitInfo) error { r.infos = append(r.infos, info); return nil }, + func(next Hash) { r.pushes = append(r.pushes, next) }, + func(err error) { + if r.err == nil { + r.err = err + } + }) + readRes <- r + } + }() + } + defer func() { + close(readReq) + wg.Wait() + }() + inFlight := 0 + var firstErr error + // consume folds one finished read into the walk. + consume := func(r read) { + inFlight-- + if r.err != nil && firstErr == nil { + firstErr = r.err + } + others = append(others, r.infos...) + for _, oid := range r.pushes { + classify(oid) + } + } + // request hands an uncovered OID to the readers, consuming finished + // reads while the queue is full so the request never deadlocks against + // a reader blocked on a full result channel. + request := func(oid Hash) { + for { + select { + case readReq <- oid: + inFlight++ + return + case r := <-readRes: + consume(r) + } + } + } + // classify routes an OID to the in-memory stack or the readers. + classify = func(oid Hash) { + if i, ok := ix.lookup(oid); ok { + if !visited[i] { + visited[i] = true + stack = append(stack, i) + } + return + } + if _, ok := seenOther[oid]; ok { + return + } + seenOther[oid] = struct{}{} + request(oid) + } + // assemble builds the result from visitOrder and others and resolves + // every result's parents to result positions: pos maps an enumerated + // commit to its position, otherPos a store-read one. The scatter and + // the resolution run in parallel chunks; the maps are read-only here. + assemble := func() ([]commitInfo, commitParents) { + ne := len(visitOrder) + out := make([]commitInfo, ne+len(others)) + copy(out[ne:], others) + pos := make([]int32, len(enumerated)) + otherPos := make(map[Hash]int32, len(others)) + for q := range others { + otherPos[others[q].OID] = int32(ne + q) + } + // The reader pool is still registered on wg until the deferred + // close, so the chunk workers use their own group. + const chunk = 4096 + var next atomic.Int64 + var cw sync.WaitGroup + for range min(numWorkers, ne/chunk+1) { + cw.Add(1) + go func() { + defer cw.Done() + for { + lo := int(next.Add(chunk) - chunk) + if lo >= ne { + return + } + for p := lo; p < min(lo+chunk, ne); p++ { + i := visitOrder[p] + out[p] = enumerated[i] + pos[i] = int32(p) + } + } + }() + } + cw.Wait() + start := make([]int32, len(out)+1) + for i := range out { + start[i+1] = start[i] + int32(len(out[i].ParentOIDs)) + } + idx := make([]int32, start[len(out)]) + next.Store(0) + for range min(numWorkers, len(out)/chunk+1) { + cw.Add(1) + go func() { + defer cw.Done() + for { + lo := int(next.Add(chunk) - chunk) + if lo >= len(out) { + return + } + for i := lo; i < min(lo+chunk, len(out)); i++ { + dst := idx[start[i]:start[i+1]] + if i < ne { + src := visitOrder[i] + for j, k := range parentIdx[parentStart[src]:parentStart[src+1]] { + if k >= 0 { + dst[j] = pos[k] + continue + } + if p, ok := otherPos[out[i].ParentOIDs[j]]; ok { + dst[j] = p + } else { + dst[j] = -1 + } + } + continue + } + for j, oid := range out[i].ParentOIDs { + if k, ok := ix.lookup(oid); ok { + dst[j] = pos[k] + } else if p, ok := otherPos[oid]; ok { + dst[j] = p + } else { + dst[j] = -1 + } + } + } + } + }() + } + cw.Wait() + return out, commitParents{start: start, idx: idx} + } + for _, tip := range tips { + classify(tip) + } + for firstErr == nil { + for len(stack) > 0 { + n := len(stack) - 1 + i := stack[n] + stack = stack[:n] + visitOrder = append(visitOrder, i) + for j, k := range parentIdx[parentStart[i]:parentStart[i+1]] { + if k < 0 { + oid := enumerated[i].ParentOIDs[j] + if _, ok := seenOther[oid]; !ok { + seenOther[oid] = struct{}{} + request(oid) + } + continue + } + if !visited[k] { + visited[k] = true + stack = append(stack, k) + } + } + } + if inFlight == 0 { + break + } + // Nothing left in memory: wait for a read, then take every other + // finished read before resuming the walk. + consume(<-readRes) + for len(readRes) > 0 { + consume(<-readRes) + } + } + for inFlight > 0 { + consume(<-readRes) + } + if firstErr != nil { + return nil, commitParents{}, firstErr + } + out, parents := assemble() + return out, parents, nil +} diff --git a/commit_enum_test.go b/commit_enum_test.go new file mode 100644 index 0000000..63a3eda --- /dev/null +++ b/commit_enum_test.go @@ -0,0 +1,211 @@ +package objstore + +import ( + "os" + "path/filepath" + "sort" + "testing" + + "github.com/stretchr/testify/require" +) + +func requireSameCommitSet(t *testing.T, want, got []commitInfo) { + t.Helper() + require.Len(t, got, len(want)) + wm := make(map[Hash]commitInfo, len(want)) + for _, c := range want { + wm[c.OID] = c + } + for _, c := range got { + w, ok := wm[c.OID] + require.Truef(t, ok, "commit %s not in the reference set", c.OID) + require.Equal(t, w.TreeOID, c.TreeOID, c.OID) + require.Equal(t, w.ParentOIDs, c.ParentOIDs, c.OID) + require.Equal(t, w.Timestamp, c.Timestamp, c.OID) + } +} + +func refWalkCommits(t *testing.T, s *HistoryScanner) []commitInfo { + t.Helper() + var out []commitInfo + require.NoError(t, s.walkCommitsFromRefsOrdered(func(c commitInfo) error { + out = append(out, c) + return nil + })) + return out +} + +// TestLoadFromRefs_MatchesRefWalk pins the pack-enumeration load to the +// reachable DAG walk on every packed fixture: the same commits with the +// same headers, in parent-first order. +func TestLoadFromRefs_MatchesRefWalk(t *testing.T) { + for _, repo := range []string{"simple-linear", "with-merges", "no-commit-graph", "large-repo", "very-large-repo-1k", "super-large-repo-10k"} { + s := createScannerForRepo(t, repo) + want := refWalkCommits(t, s) + got, _, _, err := s.loadFromRefs() + require.NoError(t, err, repo) + requireSameCommitSet(t, want, got) + ordered := orderCommitsParentFirst(append([]commitInfo(nil), want...)) + for i := range ordered { + require.Equal(t, ordered[i].OID, got[i].OID, "%s: position %d", repo, i) + } + s.Close() + } +} + +// TestEnumeratePackedCommits_CoversWholeCommits pins the enumeration to the +// set of commits stored whole in the packs, with headers equal to what +// readCommitHeader parses, whatever the worker count. +func TestEnumeratePackedCommits_CoversWholeCommits(t *testing.T) { + s := createScannerForRepo(t, "very-large-repo-1k") + defer s.Close() + got := s.store.enumeratePackedCommits(4) + var want []commitInfo + for _, pf := range s.store.packs { + for i, e := range pf.entries { + typ, _, err := peekObjectType(pf.pack, e.offset) + require.NoError(t, err) + if typ != ObjCommit { + continue + } + hdr, err := s.store.readCommitHeader(pf.oidTable[i]) + require.NoError(t, err) + info, err := parseCommitInfoFromHeader(pf.oidTable[i], hdr) + require.NoError(t, err) + want = append(want, info) + } + } + require.NotEmpty(t, want) + requireSameCommitSet(t, want, got) + requireSameCommitSet(t, want, s.store.enumeratePackedCommits(1)) +} + +// buildEnumRepo creates a repository whose history mixes every shape the +// load must cover: packed commits, a commit left loose after the repack, an +// annotated tag as a ref tip, and a packed commit that a deleted branch made +// unreachable. It returns the .git directory, the OIDs of the commits git +// reports reachable, and the unreachable commit. +func buildEnumRepo(t *testing.T) (gitDir string, reachable map[Hash]bool, unreachable Hash) { + t.Helper() + requireGit(t) + repo := t.TempDir() + runGit(t, repo, "init", "--quiet", "-b", "main") + write := func(name, content string) { + require.NoError(t, os.WriteFile(filepath.Join(repo, name), []byte(content), 0o644)) + runGit(t, repo, "add", name) + } + write("a.txt", "one\n") + runGit(t, repo, "commit", "-q", "-m", "one") + write("b.txt", "two\n") + runGit(t, repo, "commit", "-q", "-m", "two") + runGit(t, repo, "tag", "-a", "-m", "release", "v1") + runGit(t, repo, "checkout", "-q", "-b", "side") + write("c.txt", "side\n") + runGit(t, repo, "commit", "-q", "-m", "side") + unreachable = gitRevParse(t, repo, "side") + runGit(t, repo, "checkout", "-q", "main") + // Pack everything, including the side branch, then drop the branch so + // the pack holds an unreachable commit. + runGit(t, repo, "repack", "-a", "-d", "-q") + runGit(t, repo, "branch", "-D", "side") + // A commit after the repack stays loose. + write("d.txt", "loose\n") + runGit(t, repo, "commit", "-q", "-m", "loose") + + reachable = map[Hash]bool{} + for _, ref := range []string{"main", "main~1", "main~2"} { + reachable[gitRevParse(t, repo, ref)] = true + } + return filepath.Join(repo, ".git"), reachable, unreachable +} + +func gitRevParse(t *testing.T, repo, ref string) Hash { + t.Helper() + out, err := gitTestCommand(repo, "rev-parse", ref+"^{commit}").Output() + require.NoError(t, err) + h, err := ParseHash(string(out[:40])) + require.NoError(t, err) + return h +} + +// TestLoadFromRefs_MixedStorageAndUnreachable pins the reachable-walk +// semantics over the enumeration: packed commits reachable only through a +// loose commit and an annotated tag are loaded, and a packed commit no ref +// reaches is left out. +func TestLoadFromRefs_MixedStorageAndUnreachable(t *testing.T) { + gitDir, reachable, unreachable := buildEnumRepo(t) + s, err := NewHistoryScanner(gitDir) + require.NoError(t, err) + defer s.Close() + + // The pack holds the unreachable commit, so the enumeration sees it. + var enumerated []Hash + for _, c := range s.store.enumeratePackedCommits(2) { + enumerated = append(enumerated, c.OID) + } + require.Contains(t, enumerated, unreachable) + + got, _, _, err := s.loadFromRefs() + require.NoError(t, err) + var oids []Hash + for _, c := range got { + oids = append(oids, c.OID) + } + sort.Slice(oids, func(i, j int) bool { return oids[i].String() < oids[j].String() }) + require.Len(t, oids, len(reachable)) + for _, oid := range oids { + require.Truef(t, reachable[oid], "loaded %s, which no ref reaches", oid) + } + require.NotContains(t, oids, unreachable) + requireSameCommitSet(t, refWalkCommits(t, s), got) +} + +// TestLoadFromRefs_LooseOnlyRepository covers a history with no packs at +// all: the enumeration finds nothing and every commit is read through the +// walk. +func TestLoadFromRefs_LooseOnlyRepository(t *testing.T) { + requireGit(t) + repo := t.TempDir() + runGit(t, repo, "init", "--quiet", "-b", "main") + for i, name := range []string{"a", "b", "c"} { + require.NoError(t, os.WriteFile(filepath.Join(repo, name), []byte{byte('0' + i), '\n'}, 0o644)) + runGit(t, repo, "add", name) + runGit(t, repo, "commit", "-q", "-m", name) + } + s, err := NewHistoryScanner(filepath.Join(repo, ".git")) + require.NoError(t, err) + defer s.Close() + require.Empty(t, s.store.enumeratePackedCommits(2)) + got, _, _, err := s.loadFromRefs() + require.NoError(t, err) + require.Len(t, got, 3) + requireSameCommitSet(t, refWalkCommits(t, s), got) +} + +// TestBuildCommitGraphIndexed_MatchesLookupBuilder pins the remapped parent +// indexes to the OID-lookup builder on the loaded commits of every packed +// fixture, and the ordering permutation to the ordered list. +func TestBuildCommitGraphIndexed_MatchesLookupBuilder(t *testing.T) { + for _, repo := range []string{"with-merges", "large-repo", "very-large-repo-1k"} { + s := createScannerForRepo(t, repo) + ordered, parents, perm, err := s.loadFromRefs() + require.NoError(t, err, repo) + want := buildCommitGraphFromCommits(ordered) + got := buildCommitGraphIndexed(ordered, parents, perm) + require.Equal(t, want.OrderedOIDs, got.OrderedOIDs, repo) + require.Equal(t, want.TreeOIDs, got.TreeOIDs, repo) + require.Equal(t, want.Timestamps, got.Timestamps, repo) + require.Equal(t, want.OIDToIndex, got.OIDToIndex, repo) + require.Equal(t, want.Parents, got.Parents, repo) + require.Equal(t, len(want.parentIndices), len(got.parentIndices), repo) + for i := range want.parentIndices { + require.Equalf(t, want.parentIndices[i], got.parentIndices[i], "%s: commit %d", repo, i) + } + // Each ordered commit sits at perm[k] in the pre-order list the + // walk produced, and parents index that list. + require.Len(t, perm, len(ordered)) + require.Len(t, parents.start, len(ordered)+1) + require.Equal(t, parents.start[len(ordered)], int32(len(parents.idx))) + s.Close() + } +} diff --git a/commit_fallback.go b/commit_fallback.go index bfae27a..11bb132 100644 --- a/commit_fallback.go +++ b/commit_fallback.go @@ -25,79 +25,63 @@ import ( "io/fs" "os" "path/filepath" - "runtime" "slices" "strconv" "strings" - "sync" ) // loadFromRefs is the entry point for the commit-graph fallback path. // It discovers all ref tips, walks the reachable commit DAG, and returns // commits ordered parent-first (i.e. every parent appears before its // children) so that downstream consumers can process them in a single -// forward pass. -func (hs *HistoryScanner) loadFromRefs() ([]commitInfo, error) { - // walkCommitsFromRefs calls visit concurrently, so the append needs its - // own mutual exclusion. The walk joins its workers before returning, so - // reading out afterwards needs no further synchronization. - var mu sync.Mutex - out := make([]commitInfo, 0, 256) - if err := hs.walkCommitsFromRefs(func(info commitInfo) error { - mu.Lock() - out = append(out, info) - mu.Unlock() - return nil - }); err != nil { - return nil, err +// forward pass, with the parent positions and permutation +// buildCommitGraphIndexed needs to index the result without OID lookups. +func (hs *HistoryScanner) loadFromRefs() ([]commitInfo, commitParents, []int32, error) { + tips, err := collectRefTips(hs.gitDir) + if err != nil { + return nil, commitParents{}, nil, err + } + if len(tips) == 0 { + return nil, commitParents{start: []int32{0}}, nil, nil } - return orderCommitsParentFirst(out), nil + // The packs' whole commits are read in parallel up front and the + // reachable walk runs over them in memory (commit_enum.go); the + // parallel DAG walk is latency-bound on the first-parent chain. + out, parents, err := hs.loadCommitsReachable(tips) + if err != nil { + return nil, commitParents{}, nil, err + } + compactParentOIDs(out) + ordered, perm := orderCommitsParentFirstIndexed(out, parents) + return ordered, parents, perm, nil } -// walkCommitsFromRefs performs a ref-based reachable commit walk and calls -// visit once per commit, using one worker per CPU (capped at 16). -// -// The walk is a parallel BFS over the commit DAG: header inflation (a zlib -// decompression per commit) dominates the cost and is embarrassingly -// parallel, so a serial walk leaves the machine idle while downstream -// pipeline stages starve. Worker goroutines pop frontier OIDs from a shared -// stack, inflate and parse headers concurrently, then push unseen parents. -// -// Ordering: visits happen in nondeterministic order, so every caller must be -// insensitive to it. loadFromRefs re-establishes parent-first order via -// orderCommitsParentFirst, and DiffHistoryHunksFunc processes commits -// independently. Callers whose output depends on visit order (first-wins -// blob dedup in the scan planners) use walkCommitsFromRefsOrdered instead. -// -// Concurrency: visit is called from up to min(NumCPU, 16) worker goroutines at -// the same time and MUST be safe for concurrent use; the walk holds no lock -// across it. A single internal mutex around visit is not viable here: at 16 -// workers it serializes roughly a third of all walk-worker time, while -// neither production caller needs it (DiffHistoryHunksFunc's visit only -// stores into a sync.Map and sends on a channel). Callers that touch -// unsynchronized state take their own lock, as loadFromRefs does. visit must -// also not block indefinitely: a stalled visit occupies a walk worker. -func (hs *HistoryScanner) walkCommitsFromRefs(visit func(commitInfo) error) error { - return hs.walkCommitsFromRefsWorkers(min(runtime.NumCPU(), 16), visit) +// compactParentOIDs moves every commit's ParentOIDs into one shared backing +// array. The walk allocates each list separately; the scanner retains the +// commit set for its lifetime, and one array costs the garbage collector a +// single object instead of one per commit. +func compactParentOIDs(commits []commitInfo) { + total := 0 + for i := range commits { + total += len(commits[i].ParentOIDs) + } + backing := make([]Hash, total) + for i := range commits { + n := len(commits[i].ParentOIDs) + if n == 0 { + continue + } + copy(backing[:n], commits[i].ParentOIDs) + commits[i].ParentOIDs = backing[:n:n] + backing = backing[n:] + } } -// walkCommitsFromRefsOrdered performs the same reachable walk with a single -// worker, which yields a deterministic depth-first visit order: ref tips -// sorted by hash seed the stack, the last tip is explored first, and a -// commit's parents are explored before remaining siblings. Callers that -// attribute deduplicated results to the first commit visited rely on this -// order being reproducible across runs. -// -// Concurrency: one worker means visit is never called concurrently, so it may -// touch caller state without additional locking. +// walkCommitsFromRefsOrdered visits each commit reachable from the ref tips +// once, on the calling goroutine, in an order that repeats across runs: the +// first-wins blob dedupe in scan_plan.go depends on it. The first error from +// visit or a commit read ends the walk. func (hs *HistoryScanner) walkCommitsFromRefsOrdered(visit func(commitInfo) error) error { - return hs.walkCommitsFromRefsWorkers(1, visit) -} - -// walkCommitsFromRefsWorkers implements the reachable commit walk shared by -// walkCommitsFromRefs and walkCommitsFromRefsOrdered. visit is called from -// numWorkers goroutines concurrently (hence serially when numWorkers is 1). -func (hs *HistoryScanner) walkCommitsFromRefsWorkers(numWorkers int, visit func(commitInfo) error) error { if visit == nil { return nil } @@ -106,143 +90,82 @@ func (hs *HistoryScanner) walkCommitsFromRefsWorkers(numWorkers int, visit func( if err != nil { return err } - if len(tips) == 0 { - return nil - } - - var ( - mu sync.Mutex // guards seen, stack, active, firstErr - cond = sync.NewCond(&mu) - seen = make(map[Hash]struct{}, len(tips)*4) - stack = append([]Hash(nil), tips...) - active int - firstErr error - ) + var firstErr error fail := func(err error) { - mu.Lock() if firstErr == nil { firstErr = err } - mu.Unlock() - cond.Broadcast() } - - worker := func() { - for { - mu.Lock() - for len(stack) == 0 && active > 0 && firstErr == nil { - cond.Wait() - } - if firstErr != nil || len(stack) == 0 { - // Done: either an error occurred, or the stack is empty with - // no worker still processing (which could add more work). - // Broadcast so peers blocked in Wait also observe the - // termination condition and exit. - mu.Unlock() - cond.Broadcast() - return - } - n := len(stack) - 1 - oid := stack[n] - stack = stack[:n] - if _, ok := seen[oid]; ok { - mu.Unlock() - continue - } - seen[oid] = struct{}{} - active++ - mu.Unlock() - - pushed := hs.walkOne(oid, visit, func(next Hash) { - mu.Lock() - if _, ok := seen[next]; !ok { - stack = append(stack, next) - } - mu.Unlock() - }, fail) - - mu.Lock() - active-- - mu.Unlock() - // Wake waiters: either new work was pushed or active hit zero. - if pushed { - cond.Broadcast() - } else { - cond.Signal() - } + seen := make(map[Hash]struct{}, len(tips)*4) + stack := append([]Hash(nil), tips...) + push := func(next Hash) { + if _, ok := seen[next]; !ok { + stack = append(stack, next) } } - - var wg sync.WaitGroup - for range numWorkers { - wg.Add(1) - go func() { - defer wg.Done() - worker() - }() + for len(stack) > 0 && firstErr == nil { + n := len(stack) - 1 + oid := stack[n] + stack = stack[:n] + if _, ok := seen[oid]; ok { + continue + } + seen[oid] = struct{}{} + hs.walkOne(oid, visit, push, fail) } - wg.Wait() - return firstErr } // walkOne processes a single OID popped from the walk frontier: it inflates // the commit header, invokes visit, and enqueues parents via push. Tag -// objects are peeled to their target. The return value reports whether any -// new OIDs were pushed. Errors are reported through fail. -// -// visit runs unsynchronized: it is the caller's contract (see -// walkCommitsFromRefs) that it is safe for concurrent use. +// objects are peeled to their target. Errors are reported through fail. func (hs *HistoryScanner) walkOne( oid Hash, visit func(commitInfo) error, push func(Hash), fail func(error), -) (pushed bool) { +) { hdr, err := hs.store.readCommitHeader(oid) if err != nil { if errors.Is(err, ErrObjectNotFound) { // Stale refs and shallow parents can legitimately point to objects // absent from local packs. Skip and continue the reachable walk. - return false + return } if !errors.Is(err, ErrObjectNotCommit) { fail(fmt.Errorf("read commit header %s: %w", oid, err)) - return false + return } // Non-commit refs (tags, trees, etc.) are allowed. target, ok, tagErr := hs.resolveTagTarget(oid) if tagErr != nil { if errors.Is(tagErr, ErrObjectNotFound) { - return false + return } fail(tagErr) - return false + return } if ok { push(target) - return true } - return false + return } info, err := parseCommitInfoFromHeader(oid, hdr) if err != nil { fail(err) - return false + return } if err := visit(info); err != nil { fail(err) - return false + return } for _, p := range info.ParentOIDs { push(p) - pushed = true } - return pushed } // resolveTagTarget attempts to peel an annotated tag object to find its @@ -458,10 +381,17 @@ func parseCommitInfoFromHeader(oid Hash, hdr []byte) (commitInfo, error) { ParentOIDs: make([]Hash, 0, 2), } - sc := bufio.NewScanner(bytes.NewReader(hdr)) var haveTree, haveTS bool - for sc.Scan() { - line := sc.Bytes() + for rest := hdr; len(rest) > 0; { + var line []byte + if nl := bytes.IndexByte(rest, '\n'); nl >= 0 { + line, rest = rest[:nl], rest[nl+1:] + } else { + line, rest = rest, nil + } + if len(line) > 0 && line[len(line)-1] == '\r' { + line = line[:len(line)-1] + } if len(line) == 0 { break } @@ -489,9 +419,6 @@ func parseCommitInfoFromHeader(oid Hash, hdr []byte) (commitInfo, error) { haveTS = true } } - if err := sc.Err(); err != nil { - return commitInfo{}, err - } if !haveTree { return commitInfo{}, fmt.Errorf("missing tree line in commit %s", oid) diff --git a/commit_fallback_test.go b/commit_fallback_test.go index af402da..11f2e7a 100644 --- a/commit_fallback_test.go +++ b/commit_fallback_test.go @@ -1,19 +1,15 @@ // commit_fallback_test.go tests the graceful fallback behaviour when a // repository's HEAD or branch refs point to objects that do not exist in any // pack file. The scanner should return zero commits rather than an error, -// allowing callers to handle repositories with dangling references. It also -// pins the visit-concurrency contract of the two walk entry points, which -// callers rely on to decide whether they need their own locking. +// allowing callers to handle repositories with dangling references. package objstore import ( "os" "path/filepath" "runtime" - "sync" "sync/atomic" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -96,65 +92,6 @@ func TestReadRefHash_ChecksScannerError(t *testing.T) { assert.False(t, ok) } -// TestWalkCommitsFromRefs_VisitsConcurrently proves the parallel walk does not -// serialize visit. The barrier below is released only once two workers sit -// inside visit at the same time, so it fails if the walk holds any lock across -// the callback: DiffHistoryHunksFunc's visit is already concurrency-safe, and -// funneling every visit through one mutex parks the walk workers instead of -// inflating commit headers. -func TestWalkCommitsFromRefs_VisitsConcurrently(t *testing.T) { - if runtime.NumCPU() < 2 { - t.Skip("the walk runs a single worker on a single-CPU machine") - } - - // with-merges publishes two ref tips, so the frontier offers two commits - // from the first pop onward. - scanner := createScannerForRepo(t, "with-merges") - defer scanner.Close() - - const wantInFlight = 2 - var ( - mu sync.Mutex - inFlight int - gaveUp atomic.Bool - once sync.Once - ) - reached := make(chan struct{}) - - err := scanner.walkCommitsFromRefs(func(commitInfo) error { - mu.Lock() - inFlight++ - n := inFlight - mu.Unlock() - - if n >= wantInFlight { - once.Do(func() { close(reached) }) - } - if !gaveUp.Load() { - select { - case <-reached: - case <-time.After(10 * time.Second): - // Serialized visits can never reach the barrier. Stop waiting - // so the walk finishes and the assertion below reports it, - // rather than letting every commit burn the full deadline. - gaveUp.Store(true) - } - } - - mu.Lock() - inFlight-- - mu.Unlock() - return nil - }) - require.NoError(t, err) - - select { - case <-reached: - default: - t.Fatal("no two visits were in flight at once: the walk serializes visit") - } -} - // TestWalkCommitsFromRefsOrdered_VisitsSeriallyInStableOrder pins the contract // the scan planners depend on: a single worker never calls visit concurrently, // and the visit order is reproducible, so first-wins blob dedup attributes each diff --git a/commit_graph.go b/commit_graph.go index e91abe4..b11f8ac 100644 --- a/commit_graph.go +++ b/commit_graph.go @@ -28,6 +28,7 @@ import ( "io" "os" "path/filepath" + "sync" "unsafe" "golang.org/x/exp/mmap" @@ -129,16 +130,43 @@ func (g *commitGraphData) parentsOf(idx int) []Hash { return parents } -// buildCommitGraphFromCommits synthesizes a commitGraphData from an -// already-collected slice of commitInfo values (typically obtained via the -// ref-walk fallback in commit_fallback.go). -// -// This constructor exists so that both the commit-graph file reader and the -// fallback ref-walker produce the same data structure, allowing the rest of -// the codebase to be agnostic to the source. The resulting commitGraphData -// owns all of its slices (no aliasing with the input) and is safe for -// concurrent reads. -func buildCommitGraphFromCommits(commits []commitInfo) *commitGraphData { +// buildCommitGraphIndexed takes commits ordered by +// orderCommitsParentFirstIndexed: parents gives the parents of the pre-order +// input as positions in it and perm the input position of each ordered +// commit. buildCommitGraphIndexed remaps parent indexes through perm to +// match the parent-first commit order. +func buildCommitGraphIndexed(ordered []commitInfo, parents commitParents, perm []int32) *commitGraphData { + n := len(ordered) + // inv maps a pre-order position to its ordered position. + inv := make([]int32, n) + for k, i := range perm { + inv[i] = int32(k) + } + parentIndices := make([][]int32, n) + backing := make([]int32, len(parents.idx)) + for k, i := range perm { + src := parents.idx[parents.start[i]:parents.start[i+1]] + if len(src) == 0 { + continue + } + idxs := backing[:len(src):len(src)] + backing = backing[len(src):] + for j, pi := range src { + if pi < 0 { + idxs[j] = -1 // Sentinel: parent not in graph. + } else { + idxs[j] = inv[pi] + } + } + parentIndices[k] = idxs + } + return buildCommitGraph(ordered, parentIndices) +} + +// buildCommitGraph assembles the graph over commits in their given order; +// parentIndices[i] holds commit i's parent positions, -1 for a parent +// outside commits. +func buildCommitGraph(commits []commitInfo, parentIndices [][]int32) *commitGraphData { n := len(commits) parents := make(Parents, n) ordered := make([]Hash, n) @@ -146,35 +174,30 @@ func buildCommitGraphFromCommits(commits []commitInfo) *commitGraphData { times := make([]int64, n) oidToIdx := make(map[Hash]int, n) + // The two maps are built concurrently; each is a few hundred thousand + // inserts on a large history and they share no state. + var parentsDone sync.WaitGroup + parentsDone.Add(1) + go func() { + defer parentsDone.Done() + for _, c := range commits { + // commitInfo.ParentOIDs is immutable after construction, so + // the graph shares the slice. + if len(c.ParentOIDs) > 0 { + parents[c.OID] = c.ParentOIDs + } else { + parents[c.OID] = nil + } + } + }() for i, c := range commits { ordered[i] = c.OID trees[i] = c.TreeOID times[i] = c.Timestamp - if len(c.ParentOIDs) > 0 { - parents[c.OID] = append([]Hash(nil), c.ParentOIDs...) - } else { - parents[c.OID] = nil - } oidToIdx[c.OID] = i } - // Build flat parent indices. - parentIndices := make([][]int32, n) - for i, c := range commits { - if len(c.ParentOIDs) == 0 { - continue - } - idxs := make([]int32, len(c.ParentOIDs)) - for j, p := range c.ParentOIDs { - if pi, ok := oidToIdx[p]; ok { - idxs[j] = int32(pi) - } else { - idxs[j] = -1 // Sentinel: parent not in graph. - } - } - parentIndices[i] = idxs - } - + parentsDone.Wait() return &commitGraphData{ Parents: parents, OrderedOIDs: ordered, diff --git a/commit_load_helpers_test.go b/commit_load_helpers_test.go new file mode 100644 index 0000000..07f9b81 --- /dev/null +++ b/commit_load_helpers_test.go @@ -0,0 +1,47 @@ +package objstore + +func (hs *HistoryScanner) loadAllCommits() ([]commitInfo, error) { + commits, _, err := hs.loadCommitsAndGraph() + return commits, err +} + +func (hs *HistoryScanner) collectCommitPairs(c commitInfo, stopCh <-chan struct{}, admit func() error) ([]blobPairWork, error) { + return hs.collectCommitPairsIn(nil, c, stopCh, admit) +} + +func commitParentIndexes(commits []commitInfo) commitParents { + byOID := make(map[Hash]int32, len(commits)) + for i := range commits { + byOID[commits[i].OID] = int32(i) + } + start := make([]int32, len(commits)+1) + for i := range commits { + start[i+1] = start[i] + int32(len(commits[i].ParentOIDs)) + } + idx := make([]int32, start[len(commits)]) + for i := range commits { + dst := idx[start[i]:start[i+1]] + for j, p := range commits[i].ParentOIDs { + if pi, ok := byOID[p]; ok { + dst[j] = pi + } else { + dst[j] = -1 + } + } + } + return commitParents{start: start, idx: idx} +} + +func orderCommitsParentFirst(commits []commitInfo) []commitInfo { + out, _ := orderCommitsParentFirstIndexed(commits, commitParentIndexes(commits)) + return out +} + +func buildCommitGraphFromCommits(commits []commitInfo) *commitGraphData { + parents := commitParentIndexes(commits) + perm := make([]int32, len(commits)) + for i := range perm { + perm[i] = int32(i) + } + return buildCommitGraphIndexed(commits, parents, perm) +} diff --git a/commit_order.go b/commit_order.go index d698a67..86873d3 100644 --- a/commit_order.go +++ b/commit_order.go @@ -9,7 +9,8 @@ // predictable order. // // The algorithm works in three steps: -// 1. Build an in-degree map over commits whose parents are in the input set. +// 1. Build in-degree and child lists over commits whose parents are in the +// input set. // 2. Seed a min-heap with all root commits (in-degree == 0). // 3. Pop the minimum-timestamp commit, decrement children's in-degree, and // push newly zero-in-degree children onto the heap. @@ -21,147 +22,166 @@ package objstore import ( "bytes" - "container/heap" "slices" ) -// commitQueueItem is a lightweight pair of (object-ID, committer-timestamp) -// used as the element type for the min-heap priority queue. Keeping only -// these two fields avoids storing full commitInfo values in the heap, which -// would increase memory pressure during large traversals. -type commitQueueItem struct { - oid Hash - ts int64 +// commitOrderHeap is a min-heap of commit indices ordered by ascending +// committer timestamp, with lexicographic OID comparison as the tie-breaker +// so the output order is fully deterministic regardless of input order. +// Indices refer to the commits slice; the heap holds int32 values so pushes +// and pops move four bytes and allocate nothing. +type commitOrderHeap struct { + commits []commitInfo + items []int32 } -// commitQueue implements heap.Interface as a min-heap of commitQueueItems. -// The ordering is by ascending timestamp first, with lexicographic OID -// comparison as a tie-breaker to guarantee deterministic output regardless -// of map iteration order. -type commitQueue []commitQueueItem - -func (q commitQueue) Len() int { return len(q) } - -// Less defines the min-heap ordering: earlier timestamps surface first. -// When two commits share the same timestamp (common in squash-merges and -// fast-imports), the raw OID bytes are compared lexicographically so that -// the output order is fully deterministic and reproducible across runs. -func (q commitQueue) Less(i, j int) bool { - if q[i].ts != q[j].ts { - return q[i].ts < q[j].ts +func (h *commitOrderHeap) less(a, b int32) bool { + ca, cb := &h.commits[a], &h.commits[b] + if ca.Timestamp != cb.Timestamp { + return ca.Timestamp < cb.Timestamp } - return bytes.Compare(q[i].oid[:], q[j].oid[:]) < 0 + return bytes.Compare(ca.OID[:], cb.OID[:]) < 0 } -func (q commitQueue) Swap(i, j int) { q[i], q[j] = q[j], q[i] } +func (h *commitOrderHeap) push(i int32) { + h.items = append(h.items, i) + j := len(h.items) - 1 + for j > 0 { + parent := (j - 1) / 2 + if !h.less(h.items[j], h.items[parent]) { + break + } + h.items[j], h.items[parent] = h.items[parent], h.items[j] + j = parent + } +} -func (q *commitQueue) Push(x any) { - *q = append(*q, x.(commitQueueItem)) +func (h *commitOrderHeap) pop() int32 { + top := h.items[0] + last := len(h.items) - 1 + h.items[0] = h.items[last] + h.items = h.items[:last] + j := 0 + for { + l := 2*j + 1 + if l >= last { + break + } + m := l + if r := l + 1; r < last && h.less(h.items[r], h.items[l]) { + m = r + } + if !h.less(h.items[m], h.items[j]) { + break + } + h.items[j], h.items[m] = h.items[m], h.items[j] + j = m + } + return top } -func (q *commitQueue) Pop() any { - old := *q - n := len(old) - item := old[n-1] - *q = old[:n-1] - return item +// commitParents holds every commit's parents as positions in the commits +// slice, -1 for a parent outside it, in one flat array: +// idx[start[i]:start[i+1]] are commit i's parents, in header order. +type commitParents struct { + start []int32 + idx []int32 } -// orderCommitsParentFirst returns a deterministic parent-before-child ordering -// of the input commits using Kahn's algorithm with a min-heap priority queue. -// -// Algorithm: -// 1. Index every commit by OID and compute its in-degree (number of parents -// that are also in the input set). -// 2. Push all zero-in-degree (root) commits into a min-heap ordered by -// (timestamp, OID). -// 3. Repeatedly pop the minimum element, emit it, and decrement the -// in-degree of each of its children. When a child reaches in-degree 0, -// push it onto the heap. -// -// Cycle fallback: if the input graph contains cycles (e.g. grafted history), -// the main loop will terminate before emitting every commit. The remaining -// commits are sorted by (timestamp, OID) and appended so that the caller -// always receives exactly len(commits) results. -// -// Determinism guarantee: for any fixed input set the output order is fully -// reproducible, regardless of Go map iteration order, because the heap -// tie-breaks on OID bytes. -func orderCommitsParentFirst(commits []commitInfo) []commitInfo { - if len(commits) < 2 { - out := make([]commitInfo, len(commits)) +// orderCommitsParentFirstIndexed is orderCommitsParentFirst over parents +// already resolved to positions. It also returns perm, the position in +// commits of each element of the result, so callers that index the result +// can remap positions without a second OID lookup. +func orderCommitsParentFirstIndexed(commits []commitInfo, parents commitParents) (out []commitInfo, perm []int32) { + n := len(commits) + if n < 2 { + out = make([]commitInfo, n) copy(out, commits) - return out + perm = make([]int32, n) + for i := range perm { + perm[i] = int32(i) + } + return out, perm } - byOID := make(map[Hash]commitInfo, len(commits)) - children := make(map[Hash][]Hash, len(commits)) - inDegree := make(map[Hash]int, len(commits)) - - for _, c := range commits { - byOID[c.OID] = c - inDegree[c.OID] = 0 + // Count in-degrees and children per parent, then lay the child lists + // out contiguously: children of commit p are + // childList[childStart[p]:childStart[p+1]]. + inDegree := make([]int32, n) + childStart := make([]int32, n+1) + for i := range commits { + for _, pi := range parents.idx[parents.start[i]:parents.start[i+1]] { + if pi < 0 { + continue + } + inDegree[i]++ + childStart[pi+1]++ + } } - - for _, c := range commits { - for _, p := range c.ParentOIDs { - if _, ok := byOID[p]; !ok { + for i := 1; i <= n; i++ { + childStart[i] += childStart[i-1] + } + childList := make([]int32, childStart[n]) + fill := make([]int32, n) + copy(fill, childStart[:n]) + for i := range commits { + for _, pi := range parents.idx[parents.start[i]:parents.start[i+1]] { + if pi < 0 { continue } - inDegree[c.OID]++ - children[p] = append(children[p], c.OID) + childList[fill[pi]] = int32(i) + fill[pi]++ } } - q := make(commitQueue, 0, len(commits)) - for _, c := range commits { - if inDegree[c.OID] == 0 { - q = append(q, commitQueueItem{oid: c.OID, ts: c.Timestamp}) + q := commitOrderHeap{commits: commits, items: make([]int32, 0, n)} + for i := range commits { + if inDegree[i] == 0 { + q.push(int32(i)) } } - heap.Init(&q) - - out := make([]commitInfo, 0, len(commits)) - for q.Len() > 0 { - item := heap.Pop(&q).(commitQueueItem) - c := byOID[item.oid] - out = append(out, c) - for _, child := range children[item.oid] { + out = make([]commitInfo, 0, n) + perm = make([]int32, 0, n) + emitted := make([]bool, n) + for len(q.items) > 0 { + i := q.pop() + out = append(out, commits[i]) + perm = append(perm, i) + emitted[i] = true + for _, child := range childList[childStart[i]:childStart[i+1]] { inDegree[child]-- if inDegree[child] == 0 { - cc := byOID[child] - heap.Push(&q, commitQueueItem{oid: child, ts: cc.Timestamp}) + q.push(child) } } } // Defensive fallback for malformed input with cycles. - if len(out) < len(commits) { - seen := make(map[Hash]struct{}, len(out)) - for _, c := range out { - seen[c.OID] = struct{}{} - } - - rest := make([]commitInfo, 0, len(commits)-len(out)) - for _, c := range commits { - if _, ok := seen[c.OID]; ok { + if len(out) < n { + rest := make([]int32, 0, n-len(out)) + for i := range commits { + if emitted[i] { continue } - rest = append(rest, c) + rest = append(rest, int32(i)) } - slices.SortFunc(rest, func(a, b commitInfo) int { - if a.Timestamp < b.Timestamp { + slices.SortFunc(rest, func(a, b int32) int { + ca, cb := &commits[a], &commits[b] + if ca.Timestamp < cb.Timestamp { return -1 } - if a.Timestamp > b.Timestamp { + if ca.Timestamp > cb.Timestamp { return 1 } - return bytes.Compare(a.OID[:], b.OID[:]) + return bytes.Compare(ca.OID[:], cb.OID[:]) }) - out = append(out, rest...) + for _, i := range rest { + out = append(out, commits[i]) + perm = append(perm, i) + } } - return out + return out, perm } diff --git a/commit_walk_determinism_test.go b/commit_walk_determinism_test.go index a5fe1cc..9f6f95f 100644 --- a/commit_walk_determinism_test.go +++ b/commit_walk_determinism_test.go @@ -125,7 +125,7 @@ func TestParallelCommitWalk_Deterministic(t *testing.T) { // Baseline: one walk on a fresh scanner. base := newScanner() defer base.Close() - want, err := base.loadFromRefs() + want, _, _, err := base.loadFromRefs() require.NoError(t, err) require.NotEmpty(t, want) wantCanon := canonicalCommits(want) @@ -135,7 +135,7 @@ func TestParallelCommitWalk_Deterministic(t *testing.T) { // Repeated walks on the SAME scanner must be identical. for run := 0; run < 8; run++ { - got, err := base.loadFromRefs() + got, _, _, err := base.loadFromRefs() require.NoError(t, err) require.Equalf(t, wantCanon, canonicalCommits(got), "same-scanner run %d diverged", run) @@ -144,7 +144,7 @@ func TestParallelCommitWalk_Deterministic(t *testing.T) { // Fresh scanners must also be identical. for run := 0; run < 4; run++ { s := newScanner() - got, err := s.loadFromRefs() + got, _, _, err := s.loadFromRefs() s.Close() require.NoError(t, err) require.Equalf(t, wantCanon, canonicalCommits(got), @@ -167,7 +167,7 @@ func TestParallelCommitWalk_GOMAXPROCSInvariant(t *testing.T) { s, err := NewHistoryScanner(gitDir) require.NoError(t, err) defer s.Close() - commits, err := s.loadFromRefs() + commits, _, _, err := s.loadFromRefs() require.NoError(t, err) return canonicalCommits(commits) } diff --git a/commit_walk_fast_test.go b/commit_walk_fast_test.go new file mode 100644 index 0000000..15c688f --- /dev/null +++ b/commit_walk_fast_test.go @@ -0,0 +1,275 @@ +package objstore + +import ( + "bytes" + "container/heap" + "math/rand" + "slices" + "sort" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// referenceOrderItem and referenceOrderQueue implement Kahn's algorithm over +// Hash-keyed maps with container/heap; referenceOrderParentFirst defines the +// (timestamp, OID) parent-first order orderCommitsParentFirst must produce. +type referenceOrderItem struct { + oid Hash + ts int64 +} + +type referenceOrderQueue []referenceOrderItem + +func (q referenceOrderQueue) Len() int { return len(q) } +func (q referenceOrderQueue) Less(i, j int) bool { + if q[i].ts != q[j].ts { + return q[i].ts < q[j].ts + } + return bytes.Compare(q[i].oid[:], q[j].oid[:]) < 0 +} +func (q referenceOrderQueue) Swap(i, j int) { q[i], q[j] = q[j], q[i] } +func (q *referenceOrderQueue) Push(x any) { *q = append(*q, x.(referenceOrderItem)) } +func (q *referenceOrderQueue) Pop() any { + old := *q + item := old[len(old)-1] + *q = old[:len(old)-1] + return item +} + +func referenceOrderParentFirst(commits []commitInfo) []commitInfo { + byOID := make(map[Hash]commitInfo, len(commits)) + children := make(map[Hash][]Hash, len(commits)) + inDegree := make(map[Hash]int, len(commits)) + for _, c := range commits { + byOID[c.OID] = c + inDegree[c.OID] = 0 + } + for _, c := range commits { + for _, p := range c.ParentOIDs { + if _, ok := byOID[p]; ok { + inDegree[c.OID]++ + children[p] = append(children[p], c.OID) + } + } + } + var q referenceOrderQueue + for _, c := range commits { + if inDegree[c.OID] == 0 { + q = append(q, referenceOrderItem{c.OID, c.Timestamp}) + } + } + heap.Init(&q) + out := make([]commitInfo, 0, len(commits)) + for q.Len() > 0 { + item := heap.Pop(&q).(referenceOrderItem) + out = append(out, byOID[item.oid]) + for _, child := range children[item.oid] { + inDegree[child]-- + if inDegree[child] == 0 { + heap.Push(&q, referenceOrderItem{child, byOID[child].Timestamp}) + } + } + } + if len(out) < len(commits) { + seen := make(map[Hash]bool, len(out)) + for _, c := range out { + seen[c.OID] = true + } + var rest []commitInfo + for _, c := range commits { + if !seen[c.OID] { + rest = append(rest, c) + } + } + slices.SortFunc(rest, func(a, b commitInfo) int { + if a.Timestamp != b.Timestamp { + if a.Timestamp < b.Timestamp { + return -1 + } + return 1 + } + return bytes.Compare(a.OID[:], b.OID[:]) + }) + out = append(out, rest...) + } + return out +} + +// randomHistory builds a DAG of n commits with timestamps that collide often +// and the occasional merge, shuffled so input order carries no information. +func randomHistory(rng *rand.Rand, n int) []commitInfo { + commits := make([]commitInfo, n) + for i := range commits { + var oid Hash + rng.Read(oid[:]) + commits[i] = commitInfo{OID: oid, TreeOID: Hash{byte(i)}, Timestamp: int64(rng.Intn(n/4 + 1))} + if i > 0 { + parents := 1 + if rng.Intn(5) == 0 { + parents = 2 + } + for k := 0; k < parents; k++ { + commits[i].ParentOIDs = append(commits[i].ParentOIDs, commits[rng.Intn(i)].OID) + } + } + } + rng.Shuffle(len(commits), func(i, j int) { commits[i], commits[j] = commits[j], commits[i] }) + return commits +} + +func TestOrderCommitsParentFirst_MatchesReference(t *testing.T) { + rng := rand.New(rand.NewSource(3)) + for iter := 0; iter < 200; iter++ { + commits := randomHistory(rng, rng.Intn(400)+2) + if iter%7 == 0 { + // A cycle with an external parent exercises the fallback. + commits = append(commits, + commitInfo{OID: Hash{0xaa}, ParentOIDs: []Hash{{0xbb}, {0xcc}}, Timestamp: 5}, + commitInfo{OID: Hash{0xbb}, ParentOIDs: []Hash{{0xaa}}, Timestamp: 4}) + } + got := orderCommitsParentFirst(commits) + want := referenceOrderParentFirst(commits) + require.Equal(t, len(want), len(got)) + for i := range want { + require.Equalf(t, want[i].OID, got[i].OID, "iteration %d position %d", iter, i) + } + } +} + +func TestCompactParentOIDsPreservesParents(t *testing.T) { + rng := rand.New(rand.NewSource(11)) + commits := randomHistory(rng, 500) + want := make([][]Hash, len(commits)) + for i, c := range commits { + want[i] = append([]Hash(nil), c.ParentOIDs...) + } + compactParentOIDs(commits) + for i, c := range commits { + assert.Equal(t, want[i], c.ParentOIDs) + assert.Equal(t, len(c.ParentOIDs), cap(c.ParentOIDs), "compacted lists have no spare capacity") + } +} + +func TestWalkCommitsFromRefsOrdered_VisitsEveryCommitOnce(t *testing.T) { + for _, repo := range []string{"simple-linear", "with-merges", "large-repo", "very-large-repo-1k"} { + s := createScannerForRepo(t, repo) + seen := make(map[Hash]int) + require.NoError(t, s.walkCommitsFromRefsOrdered(func(c commitInfo) error { + seen[c.OID]++ + return nil + })) + for oid, n := range seen { + assert.Equalf(t, 1, n, "%s: commit %s visited %d times", repo, oid, n) + } + commits, _, err := s.loadCommitsAndGraph() + require.NoError(t, err) + assert.Len(t, seen, len(commits), repo) + s.Close() + } +} + +func TestWalkCommitsFromRefsOrdered_StopsAtFirstVisitError(t *testing.T) { + s, err := NewHistoryScanner(buildMergeRepo(t)) + require.NoError(t, err) + defer s.Close() + total := 0 + require.NoError(t, s.walkCommitsFromRefsOrdered(func(commitInfo) error { total++; return nil })) + require.Greater(t, total, 2) + + for failAt := 1; failAt <= total; failAt++ { + calls := 0 + err := s.walkCommitsFromRefsOrdered(func(commitInfo) error { + calls++ + if calls == failAt { + return assert.AnError + } + return nil + }) + require.ErrorIs(t, err, assert.AnError) + require.Equalf(t, failAt, calls, "visit ran %d times after failing at call %d", calls-failAt, failAt) + } +} + +func TestSearchHashesMatchesLinearSearch(t *testing.T) { + rng := rand.New(rand.NewSource(13)) + hashes := make([]Hash, 2000) + for i := range hashes { + rng.Read(hashes[i][:]) + if i > 0 && i%10 == 0 { + // Share the leading word with a neighbour to force the tail compare. + copy(hashes[i][:8], hashes[i-1][:8]) + } + } + sort.Slice(hashes, func(i, j int) bool { return bytes.Compare(hashes[i][:], hashes[j][:]) < 0 }) + for i, h := range hashes { + pos, ok := searchHashes(hashes, h) + require.True(t, ok) + assert.Equal(t, h, hashes[pos]) + if i > 0 && hashes[i-1] != h { + assert.Equal(t, i, pos) + } + } + for iter := 0; iter < 1000; iter++ { + var probe Hash + rng.Read(probe[:]) + pos, ok := searchHashes(hashes, probe) + want, found := slices.BinarySearchFunc(hashes, probe, func(a, b Hash) int { return bytes.Compare(a[:], b[:]) }) + assert.Equal(t, found, ok) + assert.Equal(t, want, pos) + } + _, ok := searchHashes(nil, Hash{1}) + assert.False(t, ok) +} + +// TestOIDPrefixIndexMatchesFanout drives idxFile.findObject through the +// prefix index against a byte-fanout-only copy of the same table: every hit +// and miss agrees, across table sizes on both sides of a prefix-width step +// and with OIDs sharing long prefixes. +func TestOIDPrefixIndexMatchesFanout(t *testing.T) { + rng := rand.New(rand.NewSource(29)) + for _, n := range []int{0, 1, 2, 255, 256, 257, 5000, 70000} { + hashes := make([]Hash, n) + for i := range hashes { + rng.Read(hashes[i][:]) + if i > 0 && i%7 == 0 { + copy(hashes[i][:3], hashes[i-1][:3]) + } + } + sort.Slice(hashes, func(i, j int) bool { return bytes.Compare(hashes[i][:], hashes[j][:]) < 0 }) + hashes = slices.Compact(hashes) + entries := make([]idxEntry, len(hashes)) + var fanout [fanoutEntries]uint32 + for i, h := range hashes { + entries[i].offset = uint64(i) + 1 + fanout[h[0]]++ + } + for i := 1; i < fanoutEntries; i++ { + fanout[i] += fanout[i-1] + } + plain := &idxFile{fanout: fanout, entries: entries, oidTable: hashes} + fine := &idxFile{fanout: fanout, entries: entries, oidTable: hashes, prefixes: buildOIDPrefixIndex(hashes)} + require.GreaterOrEqual(t, fine.prefixes.bits, uint(minOIDPrefixBits)) + require.LessOrEqual(t, fine.prefixes.bits, uint(maxOIDPrefixBits)) + require.Equal(t, uint32(len(hashes)), fine.prefixes.starts[len(fine.prefixes.starts)-1]) + for _, h := range hashes { + off, ok := fine.findObject(h) + require.True(t, ok) + wantOff, _ := plain.findObject(h) + require.Equal(t, wantOff, off) + } + for iter := 0; iter < 2000; iter++ { + var probe Hash + rng.Read(probe[:]) + if iter%3 == 0 && len(hashes) > 0 { + // A near miss: share the first bytes of a present OID. + copy(probe[:4], hashes[rng.Intn(len(hashes))][:4]) + } + wantOff, want := plain.findObject(probe) + off, ok := fine.findObject(probe) + require.Equal(t, want, ok) + require.Equal(t, wantOff, off) + } + } +} diff --git a/delta.go b/delta.go index b669ad1..f5d18db 100644 --- a/delta.go +++ b/delta.go @@ -868,6 +868,28 @@ func deltaPayloadStart(pack *mmap.ReaderAt, offset uint64, deltaType ObjectType) return pos, payloadSize, nil } +func deltaTargetSize(pack *mmap.ReaderAt, offset uint64, deltaType ObjectType) (uint64, error) { + pos, _, err := deltaPayloadStart(pack, offset, deltaType) + if err != nil { + return 0, err + } + zr, release, err := openCommitHeaderStream(pack, pos) + if err != nil { + return 0, err + } + defer release() + br := getBR(zr) + defer putBR(br) + if _, err := readVarInt(br); err != nil { + return 0, fmt.Errorf("read delta base size: %w", err) + } + targetSize, err := readVarInt(br) + if err != nil { + return 0, fmt.Errorf("read delta target size: %w", err) + } + return targetSize, nil +} + // applyDeltaPrefix reconstructs the first limit bytes of the target of the // delta at offset, streaming the instruction stream and stopping once the // prefix is complete. A copy or insert that crosses the limit is truncated. diff --git a/diff_blob.go b/diff_blob.go index 2c34244..2bcff91 100644 --- a/diff_blob.go +++ b/diff_blob.go @@ -565,6 +565,13 @@ func addedHunksWithPosScratch(oldB, newB []byte, sc *lineScratch) []AddedHunk { oldB, newB = oldB[p:], newB[p:] } + // Skip the common byte suffix on a line boundary as well: a shared + // tail is matched in lockstep by any alignment, so eliding it is + // output-preserving and shrinks the edit-distance search below. + if q := commonSuffixLineBoundary(oldB, newB); q > 0 { + oldB, newB = oldB[:len(oldB)-q], newB[:len(newB)-q] + } + // Tokenize the old and new byte slices into lines for comparison. // This is a zero-copy operation, creating string views into the original slices. var oldLines, newLines []string @@ -576,6 +583,14 @@ func addedHunksWithPosScratch(oldB, newB []byte, sc *lineScratch) []AddedHunk { oldLines, newLines = tokenize(oldB), tokenize(newB) } + // A shortest edit script decides which new lines are additions: lines + // outside a longest common subsequence of the two sides. Pairs whose + // edit distance exceeds the search bound fall through to the greedy + // forward walk below. + if hunks, ok := addedHunksShortestEdit(oldLines, newLines, skippedLines); ok { + return hunks + } + // For larger files, a hash index over old lines provides O(1) lookups. // It is built lazily at the first mismatch: prefix-trimmed inputs // frequently diverge only near the tail, and eagerly indexing lines the @@ -673,6 +688,45 @@ func addedHunksWithPosScratch(oldB, newB []byte, sc *lineScratch) []AddedHunk { return hunks } +// addedHunksShortestEdit groups the new lines outside a longest common +// subsequence into hunks of consecutive line numbers; skippedLines is the +// count of common-prefix lines removed before tokenizing. It reports false +// when the edit distance exceeds the search bound. +func addedHunksShortestEdit(oldLines, newLines []string, skippedLines int) ([]AddedHunk, bool) { + if len(newLines) == 0 { + return nil, true + } + if len(oldLines) == 0 { + return []AddedHunk{{ + StartLine: uint32(skippedLines) + 1, + Lines: append([]string(nil), newLines...), + }}, true + } + sc := getDiffScratch() + defer putDiffScratch(sc) + if !addedLinesMyers(oldLines, newLines, sc) { + return nil, false + } + added := sc.added[:len(newLines)] + var hunks []AddedHunk + for i := 0; i < len(newLines); { + if !added[i] { + i++ + continue + } + j := i + 1 + for j < len(newLines) && added[j] { + j++ + } + hunks = append(hunks, AddedHunk{ + StartLine: uint32(i+skippedLines) + 1, + Lines: append([]string(nil), newLines[i:j]...), + }) + i = j + } + return hunks, true +} + // nlByte is '\n' as a []byte, for the bytes.Count newline scans. var nlByte = []byte{'\n'} diff --git a/diff_differential_test.go b/diff_differential_test.go index 87ef61d..6ee7239 100644 --- a/diff_differential_test.go +++ b/diff_differential_test.go @@ -9,87 +9,62 @@ import ( "github.com/stretchr/testify/require" ) -// referenceAddedHunksWithPos is the addedHunksWithPos implementation exactly -// as it existed at commit 73a0f5f, before the prefix-skip, lazy-index, and -// open-addressed lineIndex optimizations. It serves as the behavioral oracle -// for differential testing: the optimized path must produce identical hunks -// for every input. -func referenceAddedHunksWithPos(oldB, newB []byte) []AddedHunk { - if bytes.Equal(oldB, newB) { - return nil +// lcsLen is the O(NM) dynamic-programming longest-common-subsequence length, +// the oracle for the minimality of an added-line set. +func lcsLen(a, b []string) int { + prev := make([]int, len(b)+1) + cur := make([]int, len(b)+1) + for i := 1; i <= len(a); i++ { + for j := 1; j <= len(b); j++ { + if a[i-1] == b[j-1] { + cur[j] = prev[j-1] + 1 + } else { + cur[j] = max(prev[j], cur[j-1]) + } + } + prev, cur = cur, prev } + return prev[len(b)] +} +// requireMinimalAddedHunks checks hunks against the definition of the diff: +// every hunk line is the new line at its position, no line is reported +// twice, the unreported new lines form a subsequence of the old lines, the +// reported count equals len(new) - LCS(old, new), and hunks are maximal runs +// in increasing order. Any longest common subsequence satisfies this, so the +// check is insensitive to tie-breaking between equally short edit scripts. +func requireMinimalAddedHunks(t testing.TB, oldB, newB []byte, hunks []AddedHunk) { + t.Helper() oldLines, newLines := tokenize(oldB), tokenize(newB) - - const threshold = 50 - var oldLinePositions map[string][]uint32 - if len(oldLines) > threshold { - oldLinePositions = make(map[string][]uint32) - for i, line := range oldLines { - oldLinePositions[line] = append(oldLinePositions[line], uint32(i)) + addedAt := make([]bool, len(newLines)) + addedCount := 0 + for i, h := range hunks { + require.NotEmpty(t, h.Lines, "hunk %d is empty\nold=%q\nnew=%q", i, oldB, newB) + if i > 0 { + prev := hunks[i-1] + require.Greater(t, h.StartLine, prev.StartLine+uint32(len(prev.Lines)), "hunks %d and %d are adjacent or out of order\nold=%q\nnew=%q\nhunks=%v", i-1, i, oldB, newB, hunks) + } + for j, l := range h.Lines { + idx := int(h.StartLine) - 1 + j + require.True(t, idx >= 0 && idx < len(newLines), "hunk line %d outside new file\nold=%q\nnew=%q", idx+1, oldB, newB) + require.Equal(t, newLines[idx], l, "hunk line %d\nold=%q\nnew=%q", idx+1, oldB, newB) + require.False(t, addedAt[idx], "line %d reported twice", idx+1) + addedAt[idx] = true + addedCount++ } } - - var hunks []AddedHunk - var cur *AddedHunk - oldIdx := 0 - - for newIdx, newLine := range newLines { - lineNum := uint32(newIdx) + 1 - isAdded := false - - if oldIdx >= len(oldLines) { - isAdded = true - } else if newLine != oldLines[oldIdx] { - foundLater := false - - if oldLinePositions != nil { - if positions, exists := oldLinePositions[newLine]; exists { - for _, pos := range positions { - if pos >= uint32(oldIdx) { - foundLater = true - oldIdx = int(pos) - break - } - } - } - } else { - for j := oldIdx; j < len(oldLines); j++ { - if newLine == oldLines[j] { - foundLater = true - oldIdx = j - break - } - } - } - - if !foundLater { - isAdded = true - } + j := 0 + for i, l := range newLines { + if addedAt[i] { + continue } - - if isAdded { - if cur == nil || lineNum != cur.EndLine()+1 { - if cur != nil { - hunks = append(hunks, *cur) - } - cur = &AddedHunk{StartLine: lineNum} - } - cur.Lines = append(cur.Lines, newLine) - } else { - if cur != nil { - hunks = append(hunks, *cur) - cur = nil - } - oldIdx++ + for j < len(oldLines) && oldLines[j] != l { + j++ } + require.Less(t, j, len(oldLines), "unreported lines are not a subsequence of old\nold=%q\nnew=%q\nhunks=%v", oldB, newB, hunks) + j++ } - - if cur != nil { - hunks = append(hunks, *cur) - } - - return hunks + require.Equal(t, len(newLines)-lcsLen(oldLines, newLines), addedCount, "added-line count\nold=%q\nnew=%q\nhunks=%v", oldB, newB, hunks) } // genFile builds a synthetic text file from a small line vocabulary so that @@ -128,11 +103,9 @@ func mutate(r *rand.Rand, src []byte, vocab []string) []byte { return bytes.Join(lines, []byte{'\n'}) } -// TestAddedHunksWithPos_DifferentialAgainstReference fuzzes the optimized -// diff against the pre-optimization implementation across a wide range of -// shapes: tiny files (linear-search path), large files (indexed path), -// heavy duplication, shared prefixes, missing trailing newlines, and empty -// sides. +// TestAddedHunksWithPos_DifferentialAgainstReference fuzzes the diff against +// the LCS oracle across a wide range of shapes: tiny and large files, heavy +// duplication, shared prefixes, missing trailing newlines, and empty sides. func TestAddedHunksWithPos_DifferentialAgainstReference(t *testing.T) { t.Parallel() @@ -168,11 +141,7 @@ func TestAddedHunksWithPos_DifferentialAgainstReference(t *testing.T) { newB = append(append([]byte{}, oldB...), tail...) } - want := referenceAddedHunksWithPos(oldB, newB) - got := addedHunksWithPos(oldB, newB) - - require.Equal(t, want, got, - "iter=%d\nold=%q\nnew=%q", iter, oldB, newB) + requireMinimalAddedHunks(t, oldB, newB, addedHunksWithPos(oldB, newB)) } } @@ -194,10 +163,8 @@ func TestAddedHunksWithPos_DifferentialEdgeCases(t *testing.T) { {"a\nb\n", "b\na\n"}, // swap {"common\ncommon\nold\n", "common\ncommon\nnew\n"}, // shared prefix } - for i, c := range cases { + for _, c := range cases { oldB, newB := []byte(c[0]), []byte(c[1]) - want := referenceAddedHunksWithPos(oldB, newB) - got := addedHunksWithPos(oldB, newB) - require.Equal(t, want, got, "case %d: old=%q new=%q", i, c[0], c[1]) + requireMinimalAddedHunks(t, oldB, newB, addedHunksWithPos(oldB, newB)) } } diff --git a/diff_myers.go b/diff_myers.go new file mode 100644 index 0000000..95718a0 --- /dev/null +++ b/diff_myers.go @@ -0,0 +1,345 @@ +package objstore + +import ( + "bytes" + "sync" + "unsafe" +) + +// myersWorkBudget bounds the line comparisons one shortest-edit-script +// search may spend. The middle-snake search costs O((N+M)·D), so the budget +// admits every edit distance on files up to about 5,800 lines and an edit +// distance of about 670 on a fifty-thousand-line file, keeping a wholesale +// rewrite of a large file under about a hundred milliseconds. Pairs beyond +// it fall back to the greedy forward walk, whose output on such pairs is +// dominated by lines that are new in any alignment. On the rails history +// the budget trades 3% more emitted lines for 18% less scan CPU against a +// budget four times larger. +const myersWorkBudget = 1 << 24 + +// diffScratch holds the per-worker buffers of one shortest-edit-script +// search: the forward and reverse frontiers indexed by diagonal, the +// pending subproblems, and the added-line flags of the new side. +type diffScratch struct { + vf, vr []int32 + stack []diffRange + added []bool + fa, fb []uint64 + // seen is the fingerprint bitmap editDistanceLowerBound probes. + seen []uint64 +} + +// myersBoundLines is the pair size (old plus new lines) from which +// addedLinesMyers checks the edit-distance lower bound before searching. A +// search that ends at the budget costs up to the whole budget, which only a +// pair this large can reach ((n+m)·(n+m)/2 comparisons), and the bound costs +// a few nanoseconds per line. +const myersBoundLines = 2048 + +// myersBoundBits sizes the fingerprint bitmap of editDistanceLowerBound: +// 2^16 bits, 8 KiB, so clearing and probing it stay in L1. +const myersBoundBits = 16 + +// diffRange is one pending subproblem: old[a0:a1] against new[b0:b1]. +type diffRange struct{ a0, a1, b0, b1 int } + +var diffScratchPool = sync.Pool{New: func() any { return &diffScratch{} }} + +func getDiffScratch() *diffScratch { return diffScratchPool.Get().(*diffScratch) } + +// putDiffScratch returns sc to the pool; the added flags are capped so one +// huge blob cannot pin them for the process lifetime. +func putDiffScratch(sc *diffScratch) { + const maxPooledFlags = 1 << 20 + if cap(sc.added) > maxPooledFlags { + sc.added = nil + } + if cap(sc.fa) > maxPooledFlags { + sc.fa, sc.fb = nil, nil + } + diffScratchPool.Put(sc) +} + +// lineFingerprints fills dst with one cheap 64-bit fingerprint per line: +// the length mixed with the first and last eight bytes. Equal lines have +// equal fingerprints, and the frontier searches compare fingerprints before +// strings, so the common unequal comparison costs one word compare instead +// of two pointer dereferences and a length check. +func lineFingerprints(dst []uint64, lines []string) []uint64 { + dst = dst[:0] + for _, l := range lines { + var head, tail uint64 + switch { + case len(l) >= 8: + head = le64s(l[:8]) + tail = le64s(l[len(l)-8:]) + default: + for i := 0; i < len(l); i++ { + head = head<<8 | uint64(l[i]) + } + } + dst = append(dst, (uint64(len(l))*0x9E3779B97F4A7C15)^head^(tail*0xC2B2AE3D27D4EB4F)) + } + return dst +} + +// le64s reads the first eight bytes of s as a little-endian word; the +// compiler fuses the shifts into one load. +func le64s(s string) uint64 { + return uint64(s[0]) | uint64(s[1])<<8 | uint64(s[2])<<16 | uint64(s[3])<<24 | + uint64(s[4])<<32 | uint64(s[5])<<40 | uint64(s[6])<<48 | uint64(s[7])<<56 +} + +// addedLinesMyers marks in sc.added[i] whether newLines[i] lies outside a +// longest common subsequence of oldLines and newLines, using Myers' linear +// space shortest-edit-script algorithm: each subproblem is split at a middle +// snake found by forward and reverse frontier searches that meet halfway, +// and the two halves are solved in turn. Memory is O(D) for the frontiers +// plus O(M) for the flags. It reports false, leaving the flags unspecified, +// when the search would exceed myersWorkBudget. +func addedLinesMyers(oldLines, newLines []string, sc *diffScratch) bool { + n, m := len(oldLines), len(newLines) + if cap(sc.added) < m { + sc.added = make([]bool, m) + } + added := sc.added[:m] + for i := range added { + added[i] = false + } + // The top-level search explores forward and reverse distances up to + // half each, so its cost is about (n+m)·half comparisons; subproblems + // have smaller edit distances and sizes. + half := min((n+m+1)/2, myersWorkBudget/(n+m+1)+1) + // The edit distance is at least the length difference, so a pair the + // budget cannot cover is known before any comparison; a large addition + // to a small file is the common shape. + if n-m > 2*half || m-n > 2*half { + return false + } + width := 2*half + 3 + if cap(sc.vf) < width { + sc.vf = make([]int32, width) + sc.vr = make([]int32, width) + } + vf, vr := sc.vf[:width], sc.vr[:width] + sc.fa = lineFingerprints(sc.fa, oldLines) + sc.fb = lineFingerprints(sc.fb, newLines) + fa, fb := sc.fa, sc.fb + // The top-level middle snake is found at forward step d for an odd + // edit distance 2d-1 and at reverse step d for an even one 2d, both + // within the search iff D <= 2*limit. A lower bound above that is a + // search that would run to the budget and fail; the greedy walk takes + // over without the search, with the same result. + if n+m >= myersBoundLines { + limit := min((n+m+1)/2, half) + if editDistanceLowerBound(fa, fb, sc) > 2*limit { + return false + } + } + + sc.stack = append(sc.stack[:0], diffRange{0, n, 0, m}) + for len(sc.stack) > 0 { + r := sc.stack[len(sc.stack)-1] + sc.stack = sc.stack[:len(sc.stack)-1] + // Strip the common prefix and suffix of the subproblem; after that + // an empty side is a base case and a non-empty pair has edit + // distance at least two, so the middle snake splits it into two + // strictly smaller subproblems. + for r.a0 < r.a1 && r.b0 < r.b1 && oldLines[r.a0] == newLines[r.b0] { + r.a0++ + r.b0++ + } + for r.a0 < r.a1 && r.b0 < r.b1 && oldLines[r.a1-1] == newLines[r.b1-1] { + r.a1-- + r.b1-- + } + if r.b0 == r.b1 { + continue + } + if r.a0 == r.a1 { + for i := r.b0; i < r.b1; i++ { + added[i] = true + } + continue + } + xs, ys, xe, ye, ok := middleSnake(oldLines[r.a0:r.a1], newLines[r.b0:r.b1], fa[r.a0:r.a1], fb[r.b0:r.b1], vf, vr, half) + if !ok { + return false + } + sc.stack = append(sc.stack, + diffRange{r.a0 + xe, r.a1, r.b0 + ye, r.b1}, + diffRange{r.a0, r.a0 + xs, r.b0, r.b0 + ys}, + ) + } + return true +} + +// editDistanceLowerBound returns a lower bound on the edit distance between +// the line sequences with fingerprints fa and fb. A line whose fingerprint +// occurs nowhere on the other side is outside every common subsequence, so +// LCS <= min(m - uniqueB, n - uniqueA) and D = n + m - 2*LCS is at least +// max(n - m + 2*uniqueB, m - n + 2*uniqueA). Membership is tested through a +// bitmap over the low fingerprint bits: a bitmap hit for an absent line only +// lowers the count of lines known unique, so the bound stays a lower bound. +func editDistanceLowerBound(fa, fb []uint64, sc *diffScratch) int { + const words = 1 << (myersBoundBits - 6) + if cap(sc.seen) < 2*words { + sc.seen = make([]uint64, 2*words) + } + seenA, seenB := sc.seen[:words], sc.seen[words:2*words] + clear(seenA) + clear(seenB) + const mask = 1<>6] |= 1 << (f & 63) + } + for _, f := range fb { + seenB[(f&mask)>>6] |= 1 << (f & 63) + } + uniqueA, uniqueB := 0, 0 + for _, f := range fa { + if seenB[(f&mask)>>6]&(1<<(f&63)) == 0 { + uniqueA++ + } + } + for _, f := range fb { + if seenA[(f&mask)>>6]&(1<<(f&63)) == 0 { + uniqueB++ + } + } + n, m := len(fa), len(fb) + return max(n-m+2*uniqueB, m-n+2*uniqueA) +} + +// middleSnake finds a snake on the middle of a shortest edit path between a +// and b: forward and reverse frontier searches advance by one edit each +// until they overlap on a diagonal. It returns the snake's start and end +// (xs, ys)-(xe, ye) in a/b coordinates, and false when the searches exceed +// half edits each. fa and fb are the lines' fingerprints; vf and vr must +// hold at least 2*half+3 entries. +func middleSnake(a, b []string, fa, fb []uint64, vf, vr []int32, half int) (xs, ys, xe, ye int, ok bool) { + n, m := len(a), len(b) + delta := n - m + odd := delta&1 != 0 + limit := min((n+m+1)/2, half) + offset := limit + 1 + // The frontier loops below are the search's inner loop: every step + // reads two or three frontier entries by a diagonal index the compiler + // cannot bound, and every snake step reads a fingerprint and a line + // from each side by an index it has just compared against n and m. The + // accesses go through raw pointers so the loop carries no bounds + // checks; the indexes stay in range by construction: frontier entries + // offset+k with k in [-d-1, d+1] and d <= limit lie in [0, 2*limit+2] + // and the frontiers hold 2*half+3 >= 2*limit+3 entries (the caller + // sizes them for half), and x < n, y < m hold at every snake step. + fp := frontier{unsafe.Pointer(unsafe.SliceData(vf))} + rp := frontier{unsafe.Pointer(unsafe.SliceData(vr))} + side1 := diffSide{lines: unsafe.Pointer(unsafe.SliceData(a)), fps: unsafe.Pointer(unsafe.SliceData(fa))} + side2 := diffSide{lines: unsafe.Pointer(unsafe.SliceData(b)), fps: unsafe.Pointer(unsafe.SliceData(fb))} + _ = vf[2*limit+2] + _ = vr[2*limit+2] + fp.set(offset+1, 0) + rp.set(offset+1, 0) + for d := 0; d <= limit; d++ { + // Forward step over diagonals k = x - y. + for k := -d; k <= d; k += 2 { + var x int + if k == -d || (k != d && fp.at(offset+k-1) < fp.at(offset+k+1)) { + x = fp.at(offset + k + 1) + } else { + x = fp.at(offset+k-1) + 1 + } + y := x - k + x0, y0 := x, y + for x < n && y < m && side1.fp(x) == side2.fp(y) && side1.line(x) == side2.line(y) { + x++ + y++ + } + fp.set(offset+k, x) + // A reverse point on the same diagonal lies at reverse diagonal + // delta-k; the paths overlap once the forward x reaches it. + if odd { + if kr := delta - k; kr >= -(d-1) && kr <= d-1 && x+rp.at(offset+kr) >= n { + return x0, y0, x, y, true + } + } + } + // Reverse step: the same search over the reversed sequences, with + // x', y' counted from the ends of a and b. + for k := -d; k <= d; k += 2 { + var x int + if k == -d || (k != d && rp.at(offset+k-1) < rp.at(offset+k+1)) { + x = rp.at(offset + k + 1) + } else { + x = rp.at(offset+k-1) + 1 + } + y := x - k + x0, y0 := x, y + for x < n && y < m && side1.fp(n-1-x) == side2.fp(m-1-y) && side1.line(n-1-x) == side2.line(m-1-y) { + x++ + y++ + } + rp.set(offset+k, x) + if !odd { + if kf := delta - k; kf >= -d && kf <= d && x+fp.at(offset+kf) >= n { + return n - x, m - y, n - x0, m - y0, true + } + } + } + } + return 0, 0, 0, 0, false +} + +// frontier is an unchecked view of a middle-snake frontier array; see the +// bounds argument in middleSnake. +type frontier struct{ p unsafe.Pointer } + +func (f frontier) at(i int) int { return int(*(*int32)(unsafe.Add(f.p, uintptr(i)*4))) } +func (f frontier) set(i int, v int) { *(*int32)(unsafe.Add(f.p, uintptr(i)*4)) = int32(v) } + +// diffSide is an unchecked view of one side's lines and fingerprints; see +// the bounds argument in middleSnake. +type diffSide struct{ lines, fps unsafe.Pointer } + +func (s diffSide) fp(i int) uint64 { return *(*uint64)(unsafe.Add(s.fps, uintptr(i)*8)) } +func (s diffSide) line(i int) string { + return *(*string)(unsafe.Add(s.lines, uintptr(i)*unsafe.Sizeof(""))) +} + +// commonSuffixLineBoundary returns the length of the longest common byte +// suffix of a and b that starts immediately after a '\n' or at the start of +// a side. Trimming it leaves remainders that end on a line boundary on both +// sides, so line-based diffing of the remainders is equivalent to diffing +// the full inputs. +func commonSuffixLineBoundary(a, b []byte) int { + n := min(len(a), len(b)) + la, lb := len(a), len(b) + i := 0 + // Consecutive versions share most of their tail once the common prefix + // is gone, so the bulk of it is compared a chunk at a time through the + // runtime's vectorized equality, then the mismatching chunk is narrowed + // word- and byte-wise; the byte loop alone was 15% of a scan's CPU. + const chunk = 256 + for i+chunk <= n && bytes.Equal(a[la-i-chunk:la-i], b[lb-i-chunk:lb-i]) { + i += chunk + } + for i+8 <= n && le64(a[la-i-8:]) == le64(b[lb-i-8:]) { + i += 8 + } + for i < n && a[la-1-i] == b[lb-1-i] { + i++ + } + if i == 0 { + return 0 + } + startA, startB := len(a)-i, len(b)-i + if (startA == 0 || a[startA-1] == '\n') && (startB == 0 || b[startB-1] == '\n') { + return i + } + // Snap forward to the first newline within the common suffix. + j := bytes.IndexByte(a[startA:], '\n') + if j < 0 { + return 0 + } + return i - j - 1 +} diff --git a/diff_myers_test.go b/diff_myers_test.go new file mode 100644 index 0000000..c3437d3 --- /dev/null +++ b/diff_myers_test.go @@ -0,0 +1,259 @@ +package objstore + +import ( + "bytes" + "fmt" + "math/rand" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestAddedLinesMyersMinimalOnLargerEdits(t *testing.T) { + r := rand.New(rand.NewSource(99)) + vocab := make([]string, 40) + for i := range vocab { + vocab[i] = fmt.Sprintf("v%d", i) + } + vocab = append(vocab, "", "}", "{") + pick := func() string { return vocab[r.Intn(len(vocab))] } + for iter := 0; iter < 400; iter++ { + oldLines := make([]string, 50+r.Intn(400)) + for i := range oldLines { + oldLines[i] = pick() + } + var newLines []string + switch r.Intn(3) { + case 0: // many scattered edits + newLines = append([]string(nil), oldLines...) + for e := 0; e < 1+r.Intn(40); e++ { + pos := r.Intn(len(newLines) + 1) + switch r.Intn(3) { + case 0: + newLines = append(newLines[:pos], append([]string{pick()}, newLines[pos:]...)...) + case 1: + if pos < len(newLines) { + newLines = append(newLines[:pos], newLines[pos+1:]...) + } + case 2: + if pos < len(newLines) { + newLines[pos] = pick() + } + } + } + case 1: // block insertion whose lines repeat elsewhere in the file + pos := r.Intn(len(oldLines)) + block := make([]string, 3+r.Intn(30)) + for i := range block { + block[i] = pick() + } + newLines = append(append(append([]string(nil), oldLines[:pos]...), block...), oldLines[pos:]...) + case 2: // unrelated content + newLines = make([]string, r.Intn(400)) + for i := range newLines { + newLines[i] = pick() + } + } + oldB := []byte(strings.Join(oldLines, "\n") + "\n") + newB := []byte(strings.Join(newLines, "\n") + "\n") + if bytes.Equal(oldB, newB) { + continue + } + requireMinimalAddedHunks(t, oldB, newB, addedHunksWithPos(oldB, newB)) + } +} + +// A block inserted before an identical line is reported as the block alone; +// the greedy walk this replaced jumped to the later copy and reported the +// rest of the file. +func TestAddedHunksInsertedBlockBeforeRepeatedLine(t *testing.T) { + oldB := []byte("case a:\n\tcfg: simple\n\tx: 1\ncase b:\n\tcfg: simple\n\tx: 2\n") + newB := []byte("case a:\n\tcfg: simple\n\tkey: SECRET\n\tx: 1\ncase b:\n\tcfg: simple\n\tx: 2\n") + require.Equal(t, []AddedHunk{{StartLine: 3, Lines: []string{"\tkey: SECRET"}}}, addedHunksWithPos(oldB, newB)) + + oldB = []byte("import (\n\t\"a\"\n\t\"utils\"\n\t\"b\"\n\t\"secrets\"\n)\ntoken := \"sha256~abc\"\n") + newB = []byte("import (\n\t\"a\"\n\t\"b\"\n\t\"utils\"\n\n\t\"secrets\"\n)\ntoken := \"sha256~abc\"\n") + hunks := addedHunksWithPos(oldB, newB) + requireMinimalAddedHunks(t, oldB, newB, hunks) + for _, h := range hunks { + require.NotContains(t, h.Lines, "token := \"sha256~abc\"", "an unchanged line is never reported") + } +} + +// Beyond the work budget the greedy walk answers; its output is a valid +// added-line set, here the whole new file. +func TestAddedLinesMyersFallbackBeyondBudget(t *testing.T) { + const n = 20000 // (n+m)·(n+m)/2 comparisons exceed the budget + oldLines := make([]string, n) + newLines := make([]string, n) + for i := range oldLines { + oldLines[i] = fmt.Sprintf("old-%d", i) + newLines[i] = fmt.Sprintf("new-%d", i) + } + oldB := []byte(strings.Join(oldLines, "\n") + "\n") + newB := []byte(strings.Join(newLines, "\n") + "\n") + sc := getDiffScratch() + defer putDiffScratch(sc) + require.False(t, addedLinesMyers(tokenize(oldB), tokenize(newB), sc)) + hunks := addedHunksWithPos(oldB, newB) + require.Len(t, hunks, 1) + require.Equal(t, uint32(1), hunks[0].StartLine) + require.Len(t, hunks[0].Lines, n) +} + +// TestEditDistanceLowerBound pins the bound against brute-force edit +// distances on small random line sequences: never above the true distance, +// and exact when every line is unique to its side. +func TestEditDistanceLowerBound(t *testing.T) { + r := rand.New(rand.NewSource(5)) + sc := getDiffScratch() + defer putDiffScratch(sc) + for iter := 0; iter < 500; iter++ { + n, m := r.Intn(12), r.Intn(12) + a := make([]string, n) + b := make([]string, m) + for i := range a { + a[i] = fmt.Sprintf("l%d", r.Intn(8)) + } + for i := range b { + b[i] = fmt.Sprintf("l%d", r.Intn(8)) + } + fa := lineFingerprints(nil, a) + fb := lineFingerprints(nil, b) + lb := editDistanceLowerBound(fa, fb, sc) + require.LessOrEqual(t, lb, n+m-2*lcsLen(a, b), "a=%v b=%v", a, b) + require.GreaterOrEqual(t, lb, 0) + } + a := []string{"a", "b", "c"} + b := []string{"x", "y"} + require.Equal(t, 5, editDistanceLowerBound(lineFingerprints(nil, a), lineFingerprints(nil, b), sc)) + require.Equal(t, 0, editDistanceLowerBound(lineFingerprints(nil, a), lineFingerprints(nil, a), sc)) +} + +// TestAddedLinesMyersBoundSkipsHopelessSearch pins that a large pair whose +// lower bound exceeds the budget is refused before the search, with the +// same answer as the search at the budget would give. +func TestAddedLinesMyersBoundSkipsHopelessSearch(t *testing.T) { + const n = 20000 + oldLines := make([]string, n) + newLines := make([]string, n) + for i := range oldLines { + oldLines[i] = fmt.Sprintf("old-%d", i) + newLines[i] = fmt.Sprintf("new-%d", i) + } + sc := getDiffScratch() + defer putDiffScratch(sc) + fa := lineFingerprints(nil, oldLines) + fb := lineFingerprints(nil, newLines) + half := min((2*n+1)/2, myersWorkBudget/(2*n+1)+1) + require.Greater(t, editDistanceLowerBound(fa, fb, sc), 2*min((2*n+1)/2, half)) + start := time.Now() + require.False(t, addedLinesMyers(oldLines, newLines, sc)) + require.Less(t, time.Since(start), time.Second) +} + +func TestAddedLinesMyersWholesaleRewriteWithinBudget(t *testing.T) { + // A full rewrite of a 1024-line file: edit distance 2048, 2M comparisons. + oldLines := make([]string, 1024) + newLines := make([]string, 1024) + for i := range oldLines { + oldLines[i] = fmt.Sprintf("old-%d", i) + newLines[i] = fmt.Sprintf("new-%d", i) + } + sc := getDiffScratch() + defer putDiffScratch(sc) + require.True(t, addedLinesMyers(oldLines, newLines, sc)) + for i := range newLines { + require.True(t, sc.added[i]) + } +} + +func TestCommonSuffixLineBoundary(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"a\nb\nc\n", "x\nb\nc\n", 4}, + {"a\nb\nc\n", "a\nb\nc\n", 6}, + {"ab\nc\n", "xb\nc\n", 2}, + {"ab\nc", "xb\nc", 1}, + {"abc", "xbc", 0}, + {"", "x\n", 0}, + {"c\n", "x\nc\n", 2}, + {"a\nc\n", "c\n", 2}, + } + for _, c := range cases { + require.Equal(t, c.want, commonSuffixLineBoundary([]byte(c.a), []byte(c.b)), "commonSuffixLineBoundary(%q, %q)", c.a, c.b) + } +} + +// commonSuffixLineBoundaryRef is the byte-at-a-time definition the chunked +// implementation must match. +func commonSuffixLineBoundaryRef(a, b []byte) int { + n := min(len(a), len(b)) + i := 0 + for i < n && a[len(a)-1-i] == b[len(b)-1-i] { + i++ + } + if i == 0 { + return 0 + } + startA, startB := len(a)-i, len(b)-i + if (startA == 0 || a[startA-1] == '\n') && (startB == 0 || b[startB-1] == '\n') { + return i + } + j := bytes.IndexByte(a[startA:], '\n') + if j < 0 { + return 0 + } + return i - j - 1 +} + +// TestCommonSuffixLineBoundaryMatchesReference drives the chunked suffix +// compare across mismatch positions on every side of its 256-byte and 8-byte +// steps, with and without newlines near the boundary, against the byte loop. +func TestCommonSuffixLineBoundaryMatchesReference(t *testing.T) { + rng := rand.New(rand.NewSource(7)) + for iter := 0; iter < 20000; iter++ { + n := rng.Intn(1200) + tail := make([]byte, n) + for i := range tail { + if rng.Intn(6) == 0 { + tail[i] = '\n' + } else { + tail[i] = byte('a' + rng.Intn(4)) + } + } + headA := make([]byte, rng.Intn(40)) + headB := make([]byte, rng.Intn(40)) + for i := range headA { + headA[i] = byte(rng.Intn(256)) + } + for i := range headB { + headB[i] = byte(rng.Intn(256)) + } + a := append(append([]byte(nil), headA...), tail...) + b := append(append([]byte(nil), headB...), tail...) + if n > 0 && rng.Intn(2) == 0 { + // Flip one byte inside the shared tail so the mismatch lands + // inside a chunk or a word. + k := len(b) - 1 - rng.Intn(n) + b[k] ^= 1 + } + require.Equal(t, commonSuffixLineBoundaryRef(a, b), commonSuffixLineBoundary(a, b), "a=%q b=%q", a, b) + } +} + +func TestLineFingerprintsEqualLinesEqualFingerprints(t *testing.T) { + lines := []string{"", "a", "abcdefg", "abcdefgh", "abcdefghi", "the same long line here", "the same long line here", "the same long line herE"} + fps := lineFingerprints(nil, lines) + require.Len(t, fps, len(lines)) + require.Equal(t, fps[5], fps[6]) + require.NotEqual(t, fps[5], fps[7]) + require.NotEqual(t, fps[2], fps[3]) + require.NotEqual(t, fps[0], fps[1]) + again := lineFingerprints(fps, lines) + require.Equal(t, fps, again, "reuses the destination") +} diff --git a/history_scanner.go b/history_scanner.go index 5b9468c..bc2605c 100644 --- a/history_scanner.go +++ b/history_scanner.go @@ -214,6 +214,9 @@ func WithOffsetCacheBudget(bytes int) ScannerOption { // because ~1/3 of pair diffs in a typical walk are repeats) but it does not // change delivered hunks or their retention: a hunk's lines are compacted into // their own buffer whether or not the memo stores them. +// +// A deduplicating scan uses the same budget for its table of computed pairs, +// which lets it skip repeated pairs without diffing them. func WithPairCacheBudget(bytes int) ScannerOption { return func(hs *HistoryScanner) { hs.pairs.setBudget(bytes) @@ -473,6 +476,17 @@ func (hs *HistoryScanner) DiffHistoryHunksFunc(fn func(HunkAddition) error) erro fmt.Fprintf(os.Stderr, "Warning: failed to start profiling: %v\n", err) } + // Commits are diffed in parent-first timestamp order. Consecutive + // commits in that order touch consecutive versions of the same files, + // so their delta chains share tails that are still in the offset cache; + // the ref-walk visit order interleaves unrelated lineages and roughly + // triples the bytes materialized on a rails scan. The graph resolves + // every loaded commit's first-parent tree without a header read. + commits, graph, err := hs.loadCommitsAndGraph() + if err != nil { + return err + } + { type workItem struct { commit commitInfo @@ -486,10 +500,9 @@ func (hs *HistoryScanner) DiffHistoryHunksFunc(fn func(HunkAddition) error) erro // (cheap) and fans out per-file blob pairs; stage 2 computes hunks // (expensive: inflation + line diff) at blob-pair granularity, which // spreads a whale commit across every worker. - // workChan is deep so that the walk's visit callback, which runs on - // a walk worker, hands off without waiting on the tree stage: a few - // thousand commitInfo headers (~100 bytes each) buy full walk/tree - // decoupling for typical repositories. + // workChan is deep so the dispatcher stays ahead of the tree + // workers: a few thousand commitInfo headers (~100 bytes each) buy + // full dispatch/tree-stage decoupling for typical repositories. workChan := make(chan workItem, 8192) blobChan := make(chan []blobPairWork, 1024) // outstanding counts records sent on blobChan and not yet diffed. @@ -545,7 +558,7 @@ func (hs *HistoryScanner) DiffHistoryHunksFunc(fn func(HunkAddition) error) erro // the producer: the header inflation it requires is // the dominant cost of the walk and parallelizes // cleanly across the tree workers. - parentTree, err := hs.firstParentTree(work.commit) + parentTree, err := hs.firstParentTreeIn(graph, work.commit) if err != nil { c := work.commit setError(fmt.Errorf("resolve first-parent tree for commit %s: %w", c.OID, err)) @@ -613,45 +626,47 @@ func (hs *HistoryScanner) DiffHistoryHunksFunc(fn func(HunkAddition) error) erro }() } - walkErr := hs.walkCommitsFromRefs(func(c commitInfo) error { - // Publish the tree OID before dispatch so children resolving - // firstParentTree find it without re-inflating the header. - hs.treeOIDs.Store(c.OID, c.TreeOID) + dispatch: + for _, c := range commits { if hs.skipMergeDiffs && len(c.ParentOIDs) > 1 { - return nil + continue } select { case <-stopCh: - return errScanAborted + break dispatch case workChan <- workItem{commit: c}: - return nil } - }) + } close(workChan) treeWG.Wait() stage1Done.Store(true) finishIfDrained() blobWG.Wait() - if walkErr != nil && !errors.Is(walkErr, errScanAborted) { - setError(walkErr) - } - return firstErr } } -// maxTreeDiffWorkers is the absolute ceiling on the stage-1 tree-diff worker -// pool, applied on top of the NumCPU/2 halving in DiffHistoryHunksFunc. +// maxTreeDiffWorkers is the ceiling on the stage-1 tree-diff worker pool, +// applied on top of the NumCPU/2 halving in DiffHistoryHunksFunc: eight +// workers up to sixteen CPUs, half the CPUs beyond that. // -// Measured on a stage-1-bound history (BenchmarkDiffHistoryHunksManySmallCommits, -// 3000 single-file commits over a 200-file tree, 32-core arm64): raising the -// cap to NumCPU was ~7% slower, so the cap costs no throughput even when -// stage 1 dominates: stage-2 hunk workers, which are uncapped, set pipeline -// throughput. On the trufflehog history at 16 logical CPUs, caps of 16 and -// 32 measured within noise of 8. Re-run those benchmarks before changing -// this value. -const maxTreeDiffWorkers = 8 +// Tree diffing is about a fifth of a hunk scan's CPU (rails: 4.6 of 19.7 +// CPU-seconds), so a fixed cap of eight binds once the hunk stage has more +// than about forty workers' worth of CPU to spread the other four fifths +// over. Measured on the rails history at 32 cores (dedup scan, 32 hunk +// workers): eight tree workers ran at 5.5 of 8 busy while the hunk workers +// waited on them, 1.16 s; sixteen tree workers 1.03 s; thirty-two within +// noise of sixteen. Measured on a stage-1-bound history +// (BenchmarkDiffHistoryHunksManySmallCommits, 3000 single-file commits over +// a 200-file tree, 32-core arm64): a cap of NumCPU was ~7% slower than 8, +// and on the trufflehog history at 16 logical CPUs caps of 16 and 32 were +// within noise of 8, so the ceiling stays at eight through sixteen CPUs. +// Re-run those benchmarks before changing this value. +var maxTreeDiffWorkers = treeDiffWorkerCap(runtime.NumCPU()) + +// treeDiffWorkerCap is maxTreeDiffWorkers for a host with numCPU CPUs. +func treeDiffWorkerCap(numCPU int) int { return max(8, numCPU/2) } // errScanAborted marks an internal early-stop condition used to unwind commit walks. var errScanAborted = errors.New("scan aborted") @@ -668,11 +683,24 @@ var errScanAborted = errors.New("scan aborted") // This gracefully handles shallow clones and truncated history where parent // objects may be absent. func (hs *HistoryScanner) firstParentTree(c commitInfo) (Hash, error) { + return hs.firstParentTreeIn(nil, c) +} + +// firstParentTreeIn is firstParentTree with the loaded commit set's graph: +// a parent inside the set resolves with one map lookup, and only parents +// outside it (shallow history, refs walked without a load) go through the +// treeOIDs memo and header reads. +func (hs *HistoryScanner) firstParentTreeIn(graph *commitGraphData, c commitInfo) (Hash, error) { if len(c.ParentOIDs) == 0 { return Hash{}, nil } parentOID := c.ParentOIDs[0] + if graph != nil { + if idx, ok := graph.OIDToIndex[parentOID]; ok { + return graph.TreeOIDs[idx], nil + } + } // The commit walk records every visited commit's tree OID; a hit here // avoids re-inflating the parent's header (which would otherwise // happen once per child). @@ -1569,6 +1597,27 @@ func (hs *HistoryScanner) pairAddedHunks(oldOID, newOID Hash) ([]AddedHunk, erro return stored, nil } +// pairHunksUncached is pairHunks for the dedup pipeline: the diff is +// computed and compacted exactly as the pair memo would store it, so every +// returned hunk owns its line bytes, but the memo is neither consulted nor +// filled. In first-introduction order a repeated (old, new) transition +// contributes no text lines, so the dedup pipeline tracks repeats by key +// (pairFirstSeen) and has no use for memoized hunks. +func (hs *HistoryScanner) pairHunksUncached(work blobPairWork) ([]AddedHunk, error) { + sc := getLineScratch() + computed, err := computeAddedHunksScratch(hs.store, work.oldOID, work.newOID, sc) + if err != nil { + putLineScratch(sc) + return nil, fmt.Errorf("compute added hunks: %w", err) + } + hunks := compactPairHunks(makePairKey(work.oldOID, work.newOID), computed) + putLineScratch(sc) + if work.inferredRename { + return hs.gateInferredRenameHunks(work.oldOID, work.newOID, hunks) + } + return hunks, nil +} + // gateInferredRenameHunks validates a directory-rename pairing by content. // The pairing was inferred purely from paths, so the two blobs may be // unrelated; trusting the pair diff would silently drop any coincidentally @@ -1711,8 +1760,8 @@ func (hs *HistoryScanner) Close() error { hs.meta.clear() hs.meta.attachGraph(nil) } - // A dedup scan materializes the commit list and the graph built from - // it (loadAllCommits); both scale with history and have no use after + // A scan materializes the commit list and the graph built from it + // (loadCommitsAndGraph); both scale with history and have no use after // the object store is closed. hs.commitsMu.Lock() hs.commits, hs.commitsTips, hs.commitsShallow, hs.graphData = nil, nil, nil, nil @@ -1754,49 +1803,46 @@ func (s *HistoryScanner) GetCommitMetadata(oid Hash) (CommitMetadata, error) { return s.meta.get(oid) } -// loadAllCommits enumerates every reachable commit, caching the result for -// as long as the repository's ref tips and shallow boundary stay unchanged. -// The streaming hunk pipeline uses walkCommitsFromRefs directly; this -// materialized form serves the dedup pipeline (which needs a total order up -// front) and package tests. +// loadCommitsAndGraph enumerates every reachable commit in parent-first +// order together with the commit graph synthesized from the same load, so a +// scan can resolve parents through the graph without re-reading hs.graphData +// under the commit cache lock. Both scan pipelines consume it; the result is +// cached for as long as the repository's ref tips and shallow boundary stay +// unchanged. // // Every call reads the ref tips and the shallow file. A scanner reused after // new commits land, or after a shallow clone is deepened, re-walks the -// history on its next call, so a dedup scan sees the same commits a fresh -// walk would. Objects that appear with neither change (a partial clone -// fetching missing ancestors on demand) are outside this key. +// history on its next call, so a scan sees the same commits a fresh walk +// would. Objects that appear with neither change (a partial clone fetching +// missing ancestors on demand) are outside this key. // // Copy semantics: the returned []commitInfo is a shallow copy of the internal // cache. This prevents callers from mutating the scanner's cached state (e.g. // reordering or truncating the slice). The commitInfo values themselves are // safe to share because their mutable parts (the ParentOIDs slice) are never // modified after construction. -func (hs *HistoryScanner) loadAllCommits() ([]commitInfo, error) { +func (hs *HistoryScanner) loadCommitsAndGraph() ([]commitInfo, *commitGraphData, error) { tips, err := collectRefTips(hs.gitDir) if err != nil { - return nil, err + return nil, nil, err } slices.SortFunc(tips, func(a, b Hash) int { return bytes.Compare(a[:], b[:]) }) shallow, err := os.ReadFile(filepath.Join(hs.gitDir, "shallow")) if err != nil && !errors.Is(err, os.ErrNotExist) { - return nil, err + return nil, nil, err } hs.commitsMu.Lock() defer hs.commitsMu.Unlock() - switch { - case hs.commits != nil && slices.Equal(tips, hs.commitsTips) && bytes.Equal(shallow, hs.commitsShallow): - // Cache hit. - case hs.graphData != nil && hs.commits == nil: - hs.commits = hs.loadFromGraph() - hs.commitsTips, hs.commitsShallow = tips, shallow - default: - commits, err := hs.loadFromRefs() + // Loading a repository with no refs leaves hs.commits nil; hs.graphData + // marks the cached load. + if hs.graphData == nil || !slices.Equal(tips, hs.commitsTips) || !bytes.Equal(shallow, hs.commitsShallow) { + commits, parents, perm, err := hs.loadFromRefs() if err != nil { - return nil, err + return nil, nil, err } hs.commits, hs.commitsTips, hs.commitsShallow = commits, tips, shallow - hs.graphData = buildCommitGraphFromCommits(commits) + hs.graphData = buildCommitGraphIndexed(commits, parents, perm) if hs.meta != nil { hs.meta.attachGraph(hs.graphData) } @@ -1804,21 +1850,5 @@ func (hs *HistoryScanner) loadAllCommits() ([]commitInfo, error) { out := make([]commitInfo, len(hs.commits)) copy(out, hs.commits) - return out, nil -} - -// loadFromGraph converts commit‑graph rows into commitInfo values. -func (hs *HistoryScanner) loadFromGraph() []commitInfo { - n := len(hs.graphData.OrderedOIDs) - out := make([]commitInfo, n) - - for i, oid := range hs.graphData.OrderedOIDs { - out[i] = commitInfo{ - OID: oid, - TreeOID: hs.graphData.TreeOIDs[i], - ParentOIDs: hs.graphData.parentsOf(i), - Timestamp: hs.graphData.Timestamps[i], - } - } - return orderCommitsParentFirst(out) + return out, hs.graphData, nil } diff --git a/history_scanner_test.go b/history_scanner_test.go index 994804b..3c72406 100644 --- a/history_scanner_test.go +++ b/history_scanner_test.go @@ -965,3 +965,14 @@ func TestDiffHistoryHunksFunc_SmallHistorySpreadsPairsAcrossWorkers(t *testing.T require.NoError(t, err) require.GreaterOrEqual(t, maxInFn.Load(), int32(2), "expensive pairs of a single batch must reach more than one worker") } + +// TestTreeDiffWorkerCap pins the stage-1 ceiling: eight workers through +// sixteen CPUs, half the CPUs beyond that, so the tree stage keeps pace with +// an uncapped hunk stage on wide hosts. +func TestTreeDiffWorkerCap(t *testing.T) { + cases := map[int]int{1: 8, 4: 8, 8: 8, 16: 8, 24: 12, 32: 16, 64: 32, 96: 48} + for numCPU, want := range cases { + assert.Equalf(t, want, treeDiffWorkerCap(numCPU), "numCPU=%d", numCPU) + } + assert.Equal(t, treeDiffWorkerCap(runtime.NumCPU()), maxTreeDiffWorkers) +} diff --git a/hunk_dedup.go b/hunk_dedup.go index e71ad02..4b0d347 100644 --- a/hunk_dedup.go +++ b/hunk_dedup.go @@ -23,7 +23,7 @@ // construction because no stage ever blocks waiting on something // downstream of itself: // -// 1. Ordered commit source: orderCommitsParentFirst(loadAllCommits()), +// 1. Ordered commit source: loadCommitsAndGraph() in parent-first order, // dispatched to the tree stage up to dedupLookaheadCommits ahead of // the emit cursor while the pairs held ahead of it fit under // dedupPendingPairsCap. @@ -114,10 +114,6 @@ const ( // ordinary work instead. dedupExpensivePairBytes = 4 << 20 - // dedupSizeProbeHops bounds the delta-chain headers estimatePackedSize - // follows; a long chain's early hops already reveal a large base. - dedupSizeProbeHops = 8 - // dedupEarlyBytesCap bounds the estimated blob bytes of expensive pairs // forwarded ahead of order and not yet decided. Their results, and the // blobs their lines reference, sit in the reorder ring until their @@ -202,6 +198,7 @@ type dedupProbe struct { peakPendingPairs atomic.Int64 peakRetainedBytes atomic.Int64 + decisionRediffs atomic.Uint64 } func notePeak(peak *atomic.Int64, v int64) { @@ -750,9 +747,9 @@ type dedupCommitSlot struct { // estimatePackedSize returns an estimate of oid's materialized size from pack // headers alone, without inflating anything: the header size of a // non-delta object, or for a delta the largest header size seen while -// following up to dedupSizeProbeHops base links. It reports 0 for objects it -// cannot find or follow (loose objects, broken chains); the estimate is a -// scheduling hint and never affects results. +// following up to whaleSizeProbeHops base links (whaleSizeAt). It reports 0 +// for objects it cannot find (loose objects); the estimate is a scheduling +// hint and never affects results. func (hs *HistoryScanner) estimatePackedSize(oid Hash) uint64 { if oid.IsZero() { return 0 @@ -761,38 +758,8 @@ func (hs *HistoryScanner) estimatePackedSize(oid Hash) uint64 { if !ok { return 0 } - var best uint64 - for hop := 0; hop < dedupSizeProbeHops; hop++ { - var buf [32]byte - n, err := pack.ReadAt(buf[:], int64(off)) - if n == 0 || (err != nil && n < 1) { - return best - } - typ, size, hdrLen := parseObjectHeaderUnsafe(buf[:n]) - if hdrLen <= 0 { - return best - } - best = max(best, size) - switch typ { - case ObjOfsDelta: - back, _, err := readOfsDeltaOffset(pack, int64(off)+int64(hdrLen)) - if err != nil || back == 0 || back > off { - return best - } - off -= back - case ObjRefDelta: - var base Hash - if _, err := pack.ReadAt(base[:], int64(off)+int64(hdrLen)); err != nil { - return best - } - if pack, off, ok = hs.store.findPackedObject(base); !ok { - return best - } - default: - return best - } - } - return best + size, _, _ := hs.store.whaleSizeAt(pack, off) + return size } // dedupExpensivePair names one pair of a commit whose blobs are estimated @@ -822,8 +789,16 @@ type dedupPairWork struct { seq uint64 work blobPairWork earlyBytes uint64 + + // pos orders pairs like seq does, as commit index << 32 | pair index, + // so the first-seen pair table can compare hand-out positions. + pos uint64 } +// dedupPairPos is the hand-out position of pair index i of the commit at +// order index c; positions order exactly like seqs. +func dedupPairPos(c, i int) uint64 { return uint64(c)<<32 | uint64(uint32(i)) } + // kib is the retainedKiB total of the pairs whose hunks are in the batch. type dedupYieldBatch struct { hunks []HunkAddition @@ -848,8 +823,14 @@ type dedupCandidate struct { // present in the snapshot, so it is a duplicate at its position unless the // set is saturated when the pair is reached (see decideResult). type dedupPairResult struct { - seq uint64 - hunks []HunkAddition + seq uint64 + hunks []HunkAddition + // candidates is dedupSkippedMarker when the worker found the pair + // registered at an earlier position with a text-only result and + // skipped the diff: every line of its hunks is in the set when this + // seq is decided, so the result is empty unless the set is saturated, + // in which case the decision stage recomputes the hunks from the work + // item kept in dedupSkippedWork to honor fail-open. candidates []dedupCandidate // earlyKiB is the pair's dedupPairWork.earlyBytes in KiB. retainedKiB @@ -862,6 +843,41 @@ type dedupPairResult struct { retainedKiB uint32 } +// dedupSkippedMarker is the candidates value of a result whose diff the +// worker skipped as a repeat; see dedupPairResult.candidates. The marker is +// shared between results; its single entry has a nil unresolved list, which +// prepBacklog leaves untouched. +var dedupSkippedMarker = []dedupCandidate{{hunk: -1}} + +func (r *dedupPairResult) skipped() bool { + return len(r.candidates) == 1 && r.candidates[0].hunk < 0 +} + +// dedupSkippedWork keeps the work item of every skipped repeat from its +// delivery until the decision stage reaches its seq, so a saturated set can +// still diff the repeat and emit its hunks. +type dedupSkippedWork struct { + mu sync.Mutex + m map[uint64]blobPairWork +} + +func (w *dedupSkippedWork) put(seq uint64, work blobPairWork) { + w.mu.Lock() + if w.m == nil { + w.m = make(map[uint64]blobPairWork, 256) + } + w.m[seq] = work + w.mu.Unlock() +} + +func (w *dedupSkippedWork) take(seq uint64) (blobPairWork, bool) { + w.mu.Lock() + work, ok := w.m[seq] + delete(w.m, seq) + w.mu.Unlock() + return work, ok +} + // toKiB rounds n bytes up to whole KiB, saturating at the uint32 range. func toKiB(n uint64) uint32 { return uint32(min((n+1023)>>10, math.MaxUint32)) @@ -877,18 +893,55 @@ func toKiB(n uint64) uint32 { // hunk's position in the total order. A miss is only a hint and is resolved // authoritatively by the decision stage. func prefilterHunk(h *HunkAddition, sn *fingerprintSnapshot, buf []uint64) []uint64 { + scratch := fingerprintScratchPool.Get().(*[]uint64) + fps := (*scratch)[:0] for _, line := range h.lines { fp := lineFingerprint(line) if fp == 0 { fp = dedupZeroFingerprint } - if !sn.contains(fp) { - buf = append(buf, fp) + fps = append(fps, fp) + } + // Probe in a second pass over the hunk's fingerprints, touching each + // group's home slots first. The table is far larger than L2 and a + // hunk's lines land on unrelated lines of it, so issuing the loads of + // a group together overlaps their cache misses; probing each line + // right after hashing it would serialize one miss per line. + var touched uint64 + for rest := fps; len(rest) > 0; { + n := min(len(rest), dedupProbePrefetch) + for _, fp := range rest[:n] { + touched += atomic.LoadUint64(&sn.slots[fp&sn.mask]) + } + for _, fp := range rest[:n] { + if !sn.contains(fp) { + buf = append(buf, fp) + } } + rest = rest[n:] + } + consumeTouched(touched) + if cap(fps) <= fingerprintScratchMaxCap { + *scratch = fps[:0] + fingerprintScratchPool.Put(scratch) } return buf } +// fingerprintScratchPool holds the per-hunk fingerprint buffers prefilterHunk +// hashes into before probing; a buffer that grew past +// fingerprintScratchMaxCap entries is dropped instead of pooled. +var fingerprintScratchPool = sync.Pool{New: func() any { s := make([]uint64, 0, 256); return &s }} + +const fingerprintScratchMaxCap = 1 << 16 + +// consumeTouched keeps the touch loads of prefilterHunk observable without a +// shared variable: workers run prefilterHunk concurrently, so a global sink +// would be a data race. +// +//go:noinline +func consumeTouched(uint64) {} + // pairCandidates runs the worker-side half of the split verdict over one // pair's hunks: it returns the hunks the decision stage must still look at // (see dedupCandidate), in hunk order, and the pair's hunkRetainedBytes @@ -912,14 +965,15 @@ func pairCandidates(hunks []HunkAddition, sn *fingerprintSnapshot) (cands []dedu return cands, total } -// collectCommitPairs resolves c's first-parent tree and collects the -// commit's changed blob pairs in deterministic tree order. Errors come back -// pre-wrapped with the same message formats the streaming pipeline uses, so -// tree workers only record and propagate them. A closed stopCh aborts the -// tree walk with errScanAborted. admit, when non-nil, runs before each pair -// is appended and may block or return an error to abort. -func (hs *HistoryScanner) collectCommitPairs(c commitInfo, stopCh <-chan struct{}, admit func() error) ([]blobPairWork, error) { - parentTree, err := hs.firstParentTree(c) +// collectCommitPairsIn resolves c's first-parent tree through graph, the +// loaded commit set's graph, and collects the commit's changed blob pairs in +// deterministic tree order. Errors come back pre-wrapped with the same +// message formats the streaming pipeline uses, so tree workers only record +// and propagate them. A closed stopCh aborts the tree walk with +// errScanAborted. admit, when non-nil, runs before each pair is appended and +// may block or return an error to abort. +func (hs *HistoryScanner) collectCommitPairsIn(graph *commitGraphData, c commitInfo, stopCh <-chan struct{}, admit func() error) ([]blobPairWork, error) { + parentTree, err := hs.firstParentTreeIn(graph, c) if err != nil { return nil, fmt.Errorf("resolve first-parent tree for commit %s: %w", c.OID, err) } @@ -971,25 +1025,17 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err fmt.Fprintf(os.Stderr, "Warning: failed to start profiling: %v\n", err) } - // loadAllCommits re-walks refs when the ref tips moved, so a reused - // scanner sees commits added since its previous scan. - commits, err := hs.loadAllCommits() + // loadCommitsAndGraph re-walks refs when the ref tips moved, so a + // reused scanner sees commits added since its previous scan. It returns + // a fresh copy in parent-first order on both the ref-walk and + // commit-graph paths (each runs orderCommitsParentFirst), and the graph + // resolves every loaded commit's first-parent tree, including skipped + // merges that are another commit's first parent. + commits, graph, err := hs.loadCommitsAndGraph() if err != nil { return err } - // Publish every commit's tree OID up front so firstParentTree never - // re-inflates a header. This covers skipped merge commits too: a merge - // excluded from diffing can still be another commit's first parent. - for _, c := range commits { - hs.treeOIDs.Store(c.OID, c.TreeOID) - } - - // loadAllCommits already returns parent-first order on the ref-walk - // path, but the commit-graph path materializes rows in on-disk - // (OID-lexicographic) order, so the dedup pipeline imposes the order - // itself. orderCommitsParentFirst is idempotent and cheap relative to - // the scan. - order := orderCommitsParentFirst(commits) + order := commits if hs.skipMergeDiffs { filtered := order[:0] for _, c := range order { @@ -1001,6 +1047,12 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err order = filtered } + // Whale blobs inflate for tens of milliseconds each; inflating them + // from the start overlaps that work with the rest of the scan (see + // prefetchWhales). + waitWhales := hs.store.prefetchWhales() + defer waitWhales() + limits := hs.dedupLimits window := limits.inFlightPairs ringMask := window - 1 @@ -1083,6 +1135,9 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err // The fingerprint set is owned by the decision goroutine; hunk workers // only read its published snapshot. set := newLineFingerprintSetWithBudget(dedupInitialSlotsLog2, hs.hunkDedupBudget) + firstSeen := newPairFirstSeen(hs.pairs.budget()) + firstSeenReleased := false + var skippedWork dedupSkippedWork inFlight := newDedupInFlightWindow(window, hs.dedupProbe) // releaseRetained wakes the sequencer only when the release brings the // charge back under the cap, the one transition stampable waits on. @@ -1155,7 +1210,7 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err } return nil } - pairs, err := hs.collectCommitPairs(order[idx], stopCh, admit) + pairs, err := hs.collectCommitPairsIn(graph, order[idx], stopCh, admit) if err != nil { pendingPairs.Add(-charged) } @@ -1313,7 +1368,7 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err break } select { - case expensiveChan <- []dedupPairWork{{seq: seq + uint64(e.index), work: slot.pairs[e.index], earlyBytes: e.bytes}}: + case expensiveChan <- []dedupPairWork{{seq: seq + uint64(e.index), work: slot.pairs[e.index], earlyBytes: e.bytes, pos: dedupPairPos(stamp, e.index)}}: slot.early = append(slot.early, e.index) earlyBytes.Add(uint64(toKiB(e.bytes)) << 10) default: @@ -1404,7 +1459,7 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err if batch == nil { batch = make([]dedupPairWork, 0, dedupPairBatchSize) } - batch = append(batch, dedupPairWork{seq: s, work: slot.pairs[emitPair]}) + batch = append(batch, dedupPairWork{seq: s, work: slot.pairs[emitPair], pos: dedupPairPos(emitCommit, emitPair)}) // In the final stretch, single-pair batches spread the // remaining work across workers. The stretch begins once every // commit is dispatched and the pairs still ahead of the cursor @@ -1459,17 +1514,35 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err // process materializes one pair and delivers its result. It reports // false after recording an error. process := func(pw dedupPairWork) bool { - added, err := hs.pairHunks(pw.work) + // A pair already computed at an earlier position adds no text + // lines in first-introduction order; register each computed pair + // and skip later repeats. A result with a dedup-exempt hunk is + // recomputed because exempt hunks are emitted at every occurrence. + var key pairKey + if !pw.work.inferredRename { + key = makePairKey(pw.work.oldOID, pw.work.newOID) + if e, ok := firstSeen.lookup(key); ok && !e.exempt && e.pos < pw.pos { + skippedWork.put(pw.seq, pw.work) + deliver(dedupPairResult{seq: pw.seq, candidates: dedupSkippedMarker, earlyKiB: toKiB(pw.earlyBytes)}) + return true + } + } + added, err := hs.pairHunksUncached(pw.work) if err != nil { setError(fmt.Errorf("failed diffing %s in commit %s: %w", pw.work.path, pw.work.commit, err)) return false } - // pairHunks returns lines that are either compacted into their own - // buffer or span the whole new blob, so the retained-byte charge - // below (line bytes) is what a result actually pins. + // pairHunksUncached returns lines that are either compacted into + // their own buffer or span the whole new blob, so the retained-byte + // charge below (line bytes) is what a result actually pins. hunks := make([]HunkAddition, len(added)) + exempt := false for i := range added { hunks[i] = newHunkAddition(pw.work, added[i]) + exempt = exempt || hunks[i].dedupExempt() + } + if !pw.work.inferredRename { + firstSeen.registerMin(key, pw.pos, exempt) } // Hash and speculatively probe here, in parallel, so the decision // goroutine only touches fingerprints the snapshot did not already @@ -1604,7 +1677,40 @@ func (hs *HistoryScanner) diffHistoryHunksDedup(fn func(HunkAddition) error) err earlyBytes.Add(^(uint64(r.earlyKiB)<<10 - 1)) } before := len(batch.hunks) - batch.hunks = set.decideResult(r, batch.hunks) + if r.skipped() { + work, ok := skippedWork.take(r.seq) + if !ok { + setError(fmt.Errorf("dedup skipped repeat seq %d has no work item", r.seq)) + return false + } + if set.saturated { + // Fail-open: a saturated set emits every non-empty + // hunk, so the skipped repeat is diffed here. + added, err := hs.pairHunksUncached(work) + if err != nil { + setError(fmt.Errorf("failed diffing %s in commit %s: %w", work.path, work.commit, err)) + return false + } + if probe != nil { + probe.decisionRediffs.Add(1) + } + hunks := make([]HunkAddition, len(added)) + var total int64 + for i := range added { + hunks[i] = newHunkAddition(work, added[i]) + total += hunkRetainedBytes(&hunks[i]) + } + r.retainedKiB = toKiB(uint64(total)) + retainedKiB.Add(int64(r.retainedKiB)) + batch.hunks = set.decideResult(dedupPairResult{seq: r.seq, hunks: hunks}, batch.hunks) + } + } else { + batch.hunks = set.decideResult(r, batch.hunks) + } + if set.saturated && !firstSeenReleased { + firstSeen.release() + firstSeenReleased = true + } switch { case len(batch.hunks) > before: batch.kib += int64(r.retainedKiB) diff --git a/hunk_dedup_pairs_test.go b/hunk_dedup_pairs_test.go new file mode 100644 index 0000000..e20982b --- /dev/null +++ b/hunk_dedup_pairs_test.go @@ -0,0 +1,160 @@ +package objstore + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPairFirstSeen_RegisterMinKeepsLowestPosition(t *testing.T) { + r := newPairFirstSeen(defaultPairCacheBudget) + k := makePairKey(Hash{1}, Hash{2}) + + _, ok := r.lookup(k) + assert.False(t, ok, "unregistered pair") + + r.registerMin(k, 40, false) + e, ok := r.lookup(k) + require.True(t, ok) + assert.Equal(t, uint64(40), e.pos) + assert.False(t, e.exempt) + + r.registerMin(k, 90, true) + e, _ = r.lookup(k) + assert.Equal(t, uint64(40), e.pos, "later position keeps the earlier entry") + assert.False(t, e.exempt) + + r.registerMin(k, 10, true) + e, _ = r.lookup(k) + assert.Equal(t, uint64(10), e.pos, "earlier position replaces the entry") + assert.True(t, e.exempt) + + _, ok = r.lookup(makePairKey(Hash{2}, Hash{1})) + assert.False(t, ok, "reversed pair is a distinct key") +} + +// repeatedPairRepo builds a history where the same (old, new) blob transition +// happens on two branches, so the dedup pipeline meets the pair twice: on +// the earlier branch its lines are new, on the later one every line is a +// repeat. A binary file goes through the same transition on both branches. +func repeatedPairRepo(t *testing.T) string { + b := newDedupRepoBuilder(t) + b.write("shared.txt", uniqueText("shared-v1", 8)) + b.write("blob.bin", "\x00\x01\x02v1\n") + b.commit("base") + base := b.git("rev-parse", "HEAD") + + b.write("shared.txt", uniqueText("shared-v2", 8)) + b.write("blob.bin", "\x00\x01\x02v2\n") + b.commit("first transition") + + b.git("checkout", "-q", "-b", "side", base) + b.write("other.txt", uniqueText("other", 3)) + b.commit("side filler") + b.write("shared.txt", uniqueText("shared-v2", 8)) + b.write("blob.bin", "\x00\x01\x02v2\n") + b.commit("second transition") + return b.finish() +} + +func TestDiffHistoryHunksDedup_RepeatedPairMatchesSerialReference(t *testing.T) { + gitDir := repeatedPairRepo(t) + want := serialDedupReference(t, gitDir, false, nil) + got := collectHunkScan(t, gitDir, WithHunkLineDedup(true)) + require.Equal(t, want, got) + + text, binary := 0, 0 + for _, h := range got { + switch { + case strings.Contains(h, "|shared.txt|"): + text++ + case strings.Contains(h, "|blob.bin|"): + binary++ + } + } + // The base commit adds both files; the v1->v2 transition then appears + // once for the text file and twice for the binary file. + assert.Equal(t, 2, text, "repeated text pair is emitted at its first introduction only") + assert.Equal(t, 3, binary, "binary hunks are emitted at every occurrence") +} + +func TestPairFirstSeen_SpreadsAdditionsAndDeletionsAcrossShards(t *testing.T) { + r := newPairFirstSeen(defaultPairCacheBudget) + const perShard = 64 + for i := range pairCacheShards * perShard { + var oid Hash + oid[0], oid[1] = byte(i), byte(i>>8) + r.registerMin(makePairKey(Hash{}, oid), uint64(i), false) + r.registerMin(makePairKey(oid, Hash{}), uint64(i), false) + } + for i := range r.shards { + assert.LessOrEqualf(t, len(r.shards[i].m), 4*2*perShard, "shard %d holds %d of %d entries", i, len(r.shards[i].m), 2*pairCacheShards*perShard) + } +} + +func TestPairFirstSeen_StaysWithinBudget(t *testing.T) { + const budget = 64 << 10 + r := newPairFirstSeen(budget) + key := func(i int) pairKey { + var oid Hash + oid[0], oid[1], oid[2] = byte(i), byte(i>>8), byte(i>>16) + return makePairKey(Hash{}, oid) + } + const n = 10 * budget / pairFirstSeenEntryBytes + for i := range n { + r.registerMin(key(i), uint64(i+1), false) + } + entries := 0 + for i := range r.shards { + entries += len(r.shards[i].m) + } + assert.LessOrEqual(t, entries*pairFirstSeenEntryBytes, budget, "%d entries", entries) + assert.Positive(t, entries) + + // A registered pair still takes a lower position once the table is full. + r.registerMin(key(0), 0, true) + e, ok := r.lookup(key(0)) + require.True(t, ok) + assert.Equal(t, uint64(0), e.pos) + + r.release() + _, ok = r.lookup(key(0)) + assert.False(t, ok, "release drops the entries") + r.registerMin(key(0), 0, false) + _, ok = r.lookup(key(0)) + assert.False(t, ok, "release stops registration") + + disabled := newPairFirstSeen(0) + disabled.registerMin(key(1), 1, false) + _, ok = disabled.lookup(key(1)) + assert.False(t, ok, "a zero budget registers nothing") +} + +func TestDiffHistoryHunksDedup_SaturatedSetEmitsSkippedRepeats(t *testing.T) { + gitDir := repeatedPairRepo(t) + // A 16-byte budget saturates the fingerprint table within the first + // commit, so every later verdict fails open. + s, err := NewHistoryScanner(gitDir, WithHunkLineDedup(true), WithHunkDedupBudget(16)) + require.NoError(t, err) + defer s.Close() + got := collectHunkScanWith(t, s) + want := collectHunkScan(t, gitDir) + require.Equal(t, want, got, "saturated dedup scan equals the plain scan") +} + +// With one pair in flight, the set saturates before later pairs reach +// workers, so workers diff repeated pairs. +func TestDiffHistoryHunksDedup_SaturatedSetStopsSkippingRepeats(t *testing.T) { + gitDir := repeatedPairRepo(t) + s, err := NewHistoryScanner(gitDir, WithHunkLineDedup(true), WithHunkDedupBudget(16)) + require.NoError(t, err) + defer s.Close() + s.dedupLimits.inFlightPairs = 1 + probe := &dedupProbe{} + s.dedupProbe = probe + got := collectHunkScanWith(t, s) + require.Equal(t, collectHunkScan(t, gitDir), got) + assert.Zero(t, probe.decisionRediffs.Load(), "the decision stage re-diffed skipped repeats") +} diff --git a/idx.go b/idx.go index d5b8ddd..5cb891c 100644 --- a/idx.go +++ b/idx.go @@ -95,6 +95,59 @@ type idxFile struct { // file (true) versus synthesized from sortedOffsets (false). Only trusted // reverse indexes can map offset order back to idx-entry order for CRC lookups. ridxCRCTrusted bool + + // prefixes narrows lookups to the entries sharing a longer OID prefix + // than the 256-entry fanout does; see oidPrefixIndex. + prefixes oidPrefixIndex +} + +// oidPrefixIndex is a fanout over the leading bits of the OIDs in a sorted +// table, sized so that a bucket holds about one entry: starts[p] is the +// position of the first OID whose leading bits are at least p, and +// starts[p+1] ends the bucket. The idx file's own fanout covers one byte, +// which on a million-object pack leaves a bucket of four thousand OIDs and a +// binary search of twelve cache-missing probes per lookup; a bucket of one +// or two entries settles the lookup in a probe or two. A zero value (nil +// starts) means the index was not built and callers use the byte fanout. +type oidPrefixIndex struct { + bits uint + starts []uint32 +} + +// oidPrefixBits bounds the prefix width: at least the byte fanout's eight +// bits, at most 22 (a 16 MiB table), which keeps buckets under three +// entries up to ten million objects. +const ( + minOIDPrefixBits = 8 + maxOIDPrefixBits = 22 +) + +// buildOIDPrefixIndex indexes the sorted table with ceil(log2(len)) prefix +// bits, clamped to [minOIDPrefixBits, maxOIDPrefixBits]. +func buildOIDPrefixIndex(table []Hash) oidPrefixIndex { + bits := uint(minOIDPrefixBits) + for bits < maxOIDPrefixBits && 1<> (24 - bits) +} + +// bucket returns the table range that can hold h. +func (ix *oidPrefixIndex) bucket(h *Hash) (start, end uint32) { + p := oidPrefix(h, ix.bits) + return ix.starts[p], ix.starts[p+1] } // findObject looks up hash in the tables that belong to a single @@ -111,14 +164,18 @@ type idxFile struct { // The receiver is immutable after Open has returned, therefore the method is // safe for concurrent callers. func (f *idxFile) findObject(hash Hash) (offset uint64, found bool) { - // Identify the search range via the fan-out table. - first := hash[0] - - start := uint32(0) - if first > 0 { - start = f.fanout[first-1] + // Identify the search range via the prefix index, or the fan-out table + // of an idxFile assembled without one. + var start, end uint32 + if f.prefixes.starts != nil { + start, end = f.prefixes.bucket(&hash) + } else { + first := hash[0] + if first > 0 { + start = f.fanout[first-1] + } + end = f.fanout[first] } - end := f.fanout[first] if start == end { return 0, false // bucket empty } @@ -470,6 +527,7 @@ func parseIdx(ix *mmap.ReaderAt) (*idxFile, error) { entries: entries, oidTable: oids, largeOffsets: largeOffsets, + prefixes: buildOIDPrefixIndex(oids), }, nil } diff --git a/midx_in_memory.go b/midx_in_memory.go index 64ff08b..bd83a0f 100644 --- a/midx_in_memory.go +++ b/midx_in_memory.go @@ -33,6 +33,8 @@ type inMemoryMidx struct { fanout [fanoutEntries]uint32 objectID []Hash entries []inMemoryMidxEntry + // prefixes narrows lookups past the byte fanout; see oidPrefixIndex. + prefixes oidPrefixIndex } // inMemoryMidxRecord is a scratch intermediate used only during @@ -154,6 +156,7 @@ func buildInMemoryMidx(packs []*idxFile) *inMemoryMidx { fanout: fanout, objectID: objectIDs, entries: entries, + prefixes: buildOIDPrefixIndex(objectIDs), } } @@ -175,12 +178,16 @@ func (m *inMemoryMidx) findObject(oid Hash) (*mmap.ReaderAt, uint64, bool) { // // Returns the entry and true on hit, or a zero-value entry and false on miss. func (m *inMemoryMidx) findEntry(oid Hash) (inMemoryMidxEntry, bool) { - first := oid[0] - start := uint32(0) - if first > 0 { - start = m.fanout[first-1] + var start, end uint32 + if m.prefixes.starts != nil { + start, end = m.prefixes.bucket(&oid) + } else { + first := oid[0] + if first > 0 { + start = m.fanout[first-1] + } + end = m.fanout[first] } - end := m.fanout[first] if start == end { return inMemoryMidxEntry{}, false } diff --git a/offset_cache.go b/offset_cache.go index 853c9f7..29bdac8 100644 --- a/offset_cache.go +++ b/offset_cache.go @@ -303,6 +303,13 @@ func (c *offsetCache) admits(size int) bool { return c.enabled() && size <= maxCacheableSize && size <= c.budgetPerShard } +// admitsCacheable reports whether every object up to maxCacheableSize is +// admitted, so callers that have no size yet can skip the ARC for packed +// objects. +func (c *offsetCache) admitsCacheable() bool { + return c.enabled() && c.budgetPerShard >= maxCacheableSize +} + // get returns the materialized object stored at (pack, off), if present. // The returned slice is shared and MUST NOT be mutated. // A nil receiver reports a miss so contexts without a store can share code. @@ -380,6 +387,54 @@ type whaleCandidate struct { compressed uint64 } +// whaleSizeProbeHops bounds the delta-chain headers whaleSizeAt follows for +// a deltified candidate; a whale's chain reveals its base within a few hops. +const whaleSizeProbeHops = 8 + +// whaleSizeAt estimates the materialized size of the object at (pack, off) +// from pack headers alone: the declared size of a non-delta object, or the +// largest declared size along up to whaleSizeProbeHops base links of a +// delta. It reports the type of the first non-delta header reached and true, +// or the size seen so far and false when the chain cannot be followed that +// far (a scheduling hint either way). A multi-MB blob stored as a delta of +// its previous version (two versions of a committed binary) is a whale as +// much as one stored whole: its delta payload is itself megabytes and the +// chain inflates for as long. +func (s *store) whaleSizeAt(pack *mmap.ReaderAt, off uint64) (size uint64, typ ObjectType, ok bool) { + for hop := 0; hop < whaleSizeProbeHops; hop++ { + var buf [32]byte + n, err := pack.ReadAt(buf[:], int64(off)) + if n == 0 || (err != nil && n < 1) { + return size, ObjBad, false + } + t, declared, hdrLen := parseObjectHeaderUnsafe(buf[:n]) + if hdrLen <= 0 { + return size, ObjBad, false + } + size = max(size, declared) + switch t { + case ObjOfsDelta: + back, _, err := readOfsDeltaOffset(pack, int64(off)+int64(hdrLen)) + if err != nil || back == 0 || back > off { + return size, ObjBad, false + } + off -= back + case ObjRefDelta: + var base Hash + if _, err := pack.ReadAt(base[:], int64(off)+int64(hdrLen)); err != nil { + return size, ObjBad, false + } + var found bool + if pack, off, found = s.findPackedObject(base); !found { + return size, ObjBad, false + } + default: + return size, t, true + } + } + return size, ObjBad, false +} + // whaleEntry is one prefetched object. Whoever sets claimed first, the // prefetch goroutine or a reader, inflates the entry; done closes once data, // typ, and err are final. A reader therefore waits only on an inflation that @@ -400,8 +455,8 @@ type whaleCache struct { m map[offCacheKey]*whaleEntry // order lists m's keys in prefetch order, largest first. order []offCacheKey - // load materializes one whale. - load func(pack *mmap.ReaderAt, off uint64) ([]byte, error) + // load materializes one whale and reports its type. + load func(pack *mmap.ReaderAt, off uint64) ([]byte, ObjectType, error) } // get returns the prefetched object at (pack, off). It inflates an entry no @@ -452,8 +507,7 @@ func (c *whaleCache) run(stop <-chan struct{}) { // fill inflates e and publishes the result; the caller holds e's claim. func (c *whaleCache) fill(key offCacheKey, e *whaleEntry) { - e.data, e.err = c.load(key.pack, key.off) - e.typ = ObjBlob + e.data, e.typ, e.err = c.load(key.pack, key.off) close(e.done) } @@ -553,29 +607,55 @@ func (s *store) releaseWhales() { } func (s *store) newWhaleCache() *whaleCache { + return s.newWhaleCacheWithBudget(whalePrefetchBudget) +} + +func (s *store) newWhaleCacheWithBudget(budget uint64) *whaleCache { cands := s.whaleCandidates() if len(cands) == 0 { return nil } c := &whaleCache{ m: make(map[offCacheKey]*whaleEntry, len(cands)), - load: func(pack *mmap.ReaderAt, off uint64) ([]byte, error) { + load: func(pack *mmap.ReaderAt, off uint64) ([]byte, ObjectType, error) { + typ, hdrLen, err := peekObjectType(pack, off) + if err != nil { + return nil, ObjBad, err + } + if typ == ObjOfsDelta || typ == ObjRefDelta { + // A deltified whale reconstructs through the chain; the + // result is too large for the offset cache and the delta + // window, so the caches see only the chain's hops. + ctx := getDeltaContext(s.maxDeltaDepth) + defer putDeltaContext(ctx) + return s.inflateFromPackWithOptions(inflationParams{p: pack, off: off, ctx: ctx, maxObjectSize: s.maxDeltaObjectSize}, false, true) + } + _ = hdrLen _, data, err := readRawObject(pack, off) if err == nil { err = s.verifyPackObjectCRCIfEnabled(pack, off, Hash{}) } - return data, err + return data, typ, err }, } - var budget uint64 = whalePrefetchBudget for _, cand := range cands { - typ, hdrLen, err := peekObjectType(cand.pack, cand.off) - if err != nil || typ != ObjBlob || hdrLen <= 0 { + size, typ, ok := s.whaleSizeAt(cand.pack, cand.off) + if !ok || typ != ObjBlob { continue } - var hdr [32]byte - n, _ := cand.pack.ReadAt(hdr[:], int64(cand.off)) - _, size, _ := parseObjectHeaderUnsafe(hdr[:n]) + // A copy-heavy delta can produce a target larger than its chain's + // header sizes, so deltified candidates are charged the target size + // declared by their instruction stream. + if first, _, err := peekObjectType(cand.pack, cand.off); err != nil { + continue + } else if first == ObjOfsDelta || first == ObjRefDelta { + if size, err = deltaTargetSize(cand.pack, cand.off, first); err != nil { + continue + } + if s.maxDeltaObjectSize > 0 && size > s.maxDeltaObjectSize { + continue + } + } if size > budget { continue } diff --git a/offset_cache_test.go b/offset_cache_test.go index df6ecda..914114b 100644 --- a/offset_cache_test.go +++ b/offset_cache_test.go @@ -1,6 +1,10 @@ package objstore import ( + "bytes" + "compress/zlib" + "crypto/sha1" + "encoding/binary" "os" "path/filepath" "sync" @@ -9,6 +13,7 @@ import ( "time" "unsafe" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/mmap" ) @@ -76,6 +81,17 @@ func TestOffsetCacheAddStaysWithinBudget(t *testing.T) { // whose compressed entry exceeds whaleMinCompressedBytes and one small text // file, and returns the store with the large blob's content. func openWhaleStore(t *testing.T) (*store, []byte) { + t.Helper() + repo, big := buildWhaleRepo(t) + s, err := open(filepath.Join(repo, ".git", "objects", "pack")) + require.NoError(t, err) + t.Cleanup(func() { s.Close() }) + return s, big +} + +// buildWhaleRepo creates the one-commit repository behind openWhaleStore and +// returns its work tree with the large blob's content. +func buildWhaleRepo(t *testing.T) (string, []byte) { t.Helper() requireGit(t) repo := t.TempDir() @@ -96,11 +112,203 @@ func openWhaleStore(t *testing.T) (*store, []byte) { runGit(t, repo, "add", "-A") runGit(t, repo, "commit", "-m", "add", "--quiet") runGit(t, repo, "repack", "-a", "-d", "-q") + return repo, big +} + +// TestWhalePrefetchCoversDeltifiedBlob pins that a multi-MB blob stored as a +// delta of its previous version is a whale too: both versions of the big +// file are candidates, the deltified one is sized through its chain, and +// the prefetch delivers its bytes with the blob type. +func TestWhalePrefetchCoversDeltifiedBlob(t *testing.T) { + requireGit(t) + repo := t.TempDir() + runGit(t, repo, "init", "--quiet") + // Three times the whale threshold of incompressible bytes; the second + // version rewrites the middle third, so the delta between them is + // itself above the threshold. + big := make([]byte, 3*whaleMinCompressedBytes) + rng := uint64(0x9E3779B97F4A7C15) + for i := range big { + rng ^= rng << 13 + rng ^= rng >> 7 + rng ^= rng << 17 + big[i] = byte(rng) + } + require.NoError(t, os.WriteFile(filepath.Join(repo, "big.bin"), big, 0o644)) + runGit(t, repo, "add", "-A") + runGit(t, repo, "commit", "-m", "v1", "--quiet") + // The second version shares most bytes with the first, so the repack + // stores one of them as a delta of the other, and the delta is still + // megabytes because the changed region is incompressible. + big2 := append([]byte(nil), big...) + for i := len(big2) / 3; i < 2*len(big2)/3; i++ { + rng ^= rng << 13 + rng ^= rng >> 7 + rng ^= rng << 17 + big2[i] = byte(rng) + } + require.NoError(t, os.WriteFile(filepath.Join(repo, "big.bin"), big2, 0o644)) + runGit(t, repo, "add", "-A") + runGit(t, repo, "commit", "-m", "v2", "--quiet") + runGit(t, repo, "repack", "-a", "-d", "-q", "--window=10", "--depth=10") s, err := open(filepath.Join(repo, ".git", "objects", "pack")) require.NoError(t, err) - t.Cleanup(func() { s.Close() }) - return s, big + defer s.Close() + + oid1, oid2 := calculateHash(ObjBlob, big), calculateHash(ObjBlob, big2) + deltified := 0 + for _, oid := range []Hash{oid1, oid2} { + p, off, ok := s.findPackedObject(oid) + require.True(t, ok) + typ, _, err := peekObjectType(p, off) + require.NoError(t, err) + if typ == ObjOfsDelta || typ == ObjRefDelta { + deltified++ + } + size, root, ok := s.whaleSizeAt(p, off) + require.True(t, ok) + require.Equal(t, ObjBlob, root) + require.GreaterOrEqual(t, size, uint64(len(big))) + } + require.Equal(t, 1, deltified, "the repack stores one version as a delta of the other") + + c := s.newWhaleCache() + require.NotNil(t, c) + require.Len(t, c.order, 2, "both versions are prefetched") + for _, want := range [][]byte{big, big2} { + p, off, _ := s.findPackedObject(calculateHash(ObjBlob, want)) + got, typ, ok := c.get(p, off, nil) + require.True(t, ok) + require.Equal(t, ObjBlob, typ) + require.Equal(t, want, got) + } +} + +// buildCopyHeavyDeltaPack builds a delta fixture whose 7 MiB target exceeds +// twice the largest header size in its chain. +func buildCopyHeavyDeltaPack(t *testing.T) (string, []byte) { + t.Helper() + rng := uint64(0x9E3779B97F4A7C15) + random := func(n int) []byte { + b := make([]byte, n) + for i := range b { + rng ^= rng << 13 + rng ^= rng >> 7 + rng ^= rng << 17 + b[i] = byte(rng) + } + return b + } + base := random(1 << 16) + literal := random(3 << 20) + const copies = 64 + target := append([]byte(nil), literal...) + for range copies { + target = append(target, base...) + } + + var delta bytes.Buffer + writeVarInt(&delta, uint64(len(base))) + writeVarInt(&delta, uint64(len(target))) + for rest := literal; len(rest) > 0; { + n := min(len(rest), 127) + delta.WriteByte(byte(n)) + delta.Write(rest[:n]) + rest = rest[n:] + } + for range copies { + // Opcode 0x80 copies 0x10000 bytes from offset zero, covering the + // whole base. + delta.WriteByte(0x80) + } + + baseOID, targetOID := calculateHash(ObjBlob, base), calculateHash(ObjBlob, target) + var pack bytes.Buffer + pack.WriteString("PACK") + require.NoError(t, binary.Write(&pack, binary.BigEndian, uint32(2))) + require.NoError(t, binary.Write(&pack, binary.BigEndian, uint32(2))) + baseOff := uint64(pack.Len()) + pack.Write(encodeObjHeader(uint8(ObjBlob), uint64(len(base)))) + zw := zlib.NewWriter(&pack) + _, err := zw.Write(base) + require.NoError(t, err) + require.NoError(t, zw.Close()) + deltaOff := uint64(pack.Len()) + obj, err := packRefDeltaObject(baseOID, delta.Bytes()) + require.NoError(t, err) + pack.Write(obj) + sum := sha1.Sum(pack.Bytes()) + pack.Write(sum[:]) + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "pack-copy.pack"), pack.Bytes(), 0o644)) + require.NoError(t, createV2IndexFile(filepath.Join(dir, "pack-copy.idx"), []Hash{baseOID, targetOID}, []uint64{baseOff, deltaOff})) + return dir, target +} + +func TestWhalePrefetchChargesDeltaTarget(t *testing.T) { + dir, target := buildCopyHeavyDeltaPack(t) + s, err := open(dir) + require.NoError(t, err) + defer s.Close() + + p, off, ok := s.findPackedObject(calculateHash(ObjBlob, target)) + require.True(t, ok) + headerMax, typ, ok := s.whaleSizeAt(p, off) + require.True(t, ok) + require.Equal(t, ObjBlob, typ) + require.Less(t, 2*headerMax, uint64(len(target)), "the fixture's target dwarfs its header sizes") + + assert.Nil(t, s.newWhaleCacheWithBudget(uint64(len(target))-1), "a budget below the target size rejects the delta") + + c := s.newWhaleCacheWithBudget(uint64(len(target))) + require.NotNil(t, c) + got, typ, ok := c.get(p, off, nil) + require.True(t, ok) + assert.Equal(t, ObjBlob, typ) + assert.Equal(t, target, got) +} + +func TestWhalePrefetchSkipsDeltaAboveObjectLimit(t *testing.T) { + dir, target := buildCopyHeavyDeltaPack(t) + s, err := open(dir) + require.NoError(t, err) + defer s.Close() + + budget := uint64(len(target)) + s.SetMaxDeltaObjectSize(budget - 1) + assert.Nil(t, s.newWhaleCacheWithBudget(budget), "a delta limit below the target size rejects the delta") + + s.SetMaxDeltaObjectSize(budget) + require.NotNil(t, s.newWhaleCacheWithBudget(budget), "a delta limit at the target size admits the delta") +} + +// TestDedupScanPrefetchesWhales pins that the dedup pipeline runs the whale +// prefetch for the duration of its scan and releases it on return: a +// multi-MB blob's inflation then starts at scan start instead of when the +// tree stage reaches its commit, and the scanner holds no whale bytes +// between scans. +func TestDedupScanPrefetchesWhales(t *testing.T) { + repo, big := buildWhaleRepo(t) + hs, err := NewHistoryScanner(filepath.Join(repo, ".git"), WithHunkLineDedup(true)) + require.NoError(t, err) + defer hs.Close() + require.NotEmpty(t, hs.store.whaleCandidates()) + + var sawPrefetch, sawBig atomic.Bool + require.NoError(t, hs.DiffHistoryHunksFunc(func(h HunkAddition) error { + if hs.store.whales.Load() != nil { + sawPrefetch.Store(true) + } + if h.IsBinary() && len(h.Lines()) == 1 && h.Lines()[0] == string(big) { + sawBig.Store(true) + } + return nil + })) + require.True(t, sawPrefetch.Load(), "whale prefetch was not active during the dedup scan") + require.True(t, sawBig.Load(), "the whale blob was not delivered") + require.Nil(t, hs.store.whales.Load(), "whale prefetch outlived the scan") } // TestWhalePrefetchSkippedWhenOffsetCacheDisabled pins that a disabled offset @@ -142,14 +350,14 @@ func TestWhaleGetInflatesUnstartedEntry(t *testing.T) { keyB: {done: make(chan struct{})}, }, order: []offCacheKey{keyA, keyB}, - load: func(pack *mmap.ReaderAt, _ uint64) ([]byte, error) { + load: func(pack *mmap.ReaderAt, _ uint64) ([]byte, ObjectType, error) { if pack == packA { close(entered) <-release - return []byte("a"), nil + return []byte("a"), ObjBlob, nil } loadsB.Add(1) - return []byte("b"), nil + return []byte("b"), ObjBlob, nil }, } stop := make(chan struct{}) diff --git a/pair_cache.go b/pair_cache.go index 8d2223d..4e17867 100644 --- a/pair_cache.go +++ b/pair_cache.go @@ -46,6 +46,10 @@ const pairCacheLineOverhead = 2 * int(unsafe.Sizeof(uint32(0))) // map's cardinality stays bounded by the byte budget. const pairCacheEntryOverhead = 128 +// pairFirstSeenEntryBytes charges one pairFirstSeen entry: its pairKey, its +// pairFirstSeenEntry value, and the map's per-slot bookkeeping. +const pairFirstSeenEntryBytes = 128 + // pairKey is the concatenation of the old and new blob OIDs. type pairKey [2 * hashSize]byte @@ -166,6 +170,13 @@ func (c *pairCache) setBudget(total int) { c.evictToBudget() } +func (c *pairCache) budget() int { + if c == nil { + return 0 + } + return c.budgetPerShard * pairCacheShards +} + // evictToBudget drops entries from every over-budget shard until it fits. // Victim choice is map-order like add's eviction, which is what the cache's // approximate-replacement contract already promises. @@ -192,10 +203,14 @@ func makePairKey(oldOID, newOID Hash) pairKey { } func (c *pairCache) shard(k *pairKey) *pairCacheShard { + return &c.shards[pairShardIndex(k)] +} + +func pairShardIndex(k *pairKey) int { // Mix one byte from each OID: additions carry a zero old OID and // deletions a zero new OID, so either byte alone would funnel an // entire class of pairs into shard 0. - return &c.shards[int(k[0]^k[hashSize])&(pairCacheShards-1)] + return int(k[0]^k[hashSize]) & (pairCacheShards - 1) } func (c *pairCache) get(k pairKey) ([]AddedHunk, bool) { @@ -326,6 +341,23 @@ func coversWholeNewBlob(k *pairKey, hunks []AddedHunk) bool { return len(hunks) == 1 && hunks[0].IsBinary } +// compactPairHunks returns hunks that own their line bytes under the same +// rule add applies before admission: a whole-new-blob result keeps its +// aliasing views, every other result is compacted. +func compactPairHunks(k pairKey, hunks []AddedHunk) []AddedHunk { + if coversWholeNewBlob(&k, hunks) { + return hunks + } + lineBytes := 0 + for i := range hunks { + for _, l := range hunks[i].Lines { + lineBytes += len(l) + } + } + stored, _ := compactHunks(hunks, lineBytes) + return stored +} + // compactHunks deep-copies hunks so that no line aliases its source blob. // All line bytes share one backing buffer sized to lineBytes, so the copy // costs a single allocation for the text plus the slice headers. It returns @@ -438,3 +470,87 @@ func aliasingEntry(hunks []AddedHunk) pairCacheEntry { data := unsafe.Slice(loPtr, int(hi-lo)) return pairCacheEntry{data: data, spans: spans, hunks: shapes} } + +// pairFirstSeen records, per (old, new) blob pair, the lowest hand-out +// position (see dedupPairPos) whose hunks have been computed and whether +// that result is a binary hunk. The dedup pipeline consults it before +// diffing: a pair already computed at an earlier position contributes no +// new text lines in first-introduction order (its lines were inserted, or +// already present, when the earlier position was decided), so the later +// position skips the diff. Results with a dedup-exempt hunk are recomputed +// because exempt hunks are emitted at every occurrence. +// +// Entries are written after a result is computed, so a reader that finds an +// entry with a lower position knows that result reaches the decision stage +// first. A reader that finds no entry, or an entry with a higher position, +// computes its own result and registers it; registerMin keeps the minimum. +// +// Each entry consumes pairFirstSeenEntryBytes of the scanner's pair-cache +// budget (WithPairCacheBudget). A pair that finds its shard full is diffed +// on each repeat. +type pairFirstSeen struct { + shards [pairCacheShards]pairFirstSeenShard + perShard int +} + +type pairFirstSeenShard struct { + mu sync.Mutex + m map[pairKey]pairFirstSeenEntry +} + +type pairFirstSeenEntry struct { + pos uint64 + // exempt marks a result with a dedup-exempt hunk (binary or oversize + // placeholder), which repeats must recompute. + exempt bool +} + +func newPairFirstSeen(budget int) *pairFirstSeen { + r := &pairFirstSeen{perShard: max(0, budget/pairFirstSeenEntryBytes/pairCacheShards)} + if r.perShard == 0 { + return r + } + for i := range r.shards { + r.shards[i].m = make(map[pairKey]pairFirstSeenEntry, min(r.perShard, 1024)) + } + return r +} + +func (r *pairFirstSeen) shard(k *pairKey) *pairFirstSeenShard { + return &r.shards[pairShardIndex(k)] +} + +// lookup returns the registered entry for k. +func (r *pairFirstSeen) lookup(k pairKey) (pairFirstSeenEntry, bool) { + s := r.shard(&k) + s.mu.Lock() + e, ok := s.m[k] + s.mu.Unlock() + return e, ok +} + +// registerMin records pos for k when k is new or pos is lower than the +// registered position. +func (r *pairFirstSeen) registerMin(k pairKey, pos uint64, exempt bool) { + s := r.shard(&k) + s.mu.Lock() + if s.m != nil { + if e, ok := s.m[k]; ok { + if pos < e.pos { + s.m[k] = pairFirstSeenEntry{pos: pos, exempt: exempt} + } + } else if len(s.m) < r.perShard { + s.m[k] = pairFirstSeenEntry{pos: pos, exempt: exempt} + } + } + s.mu.Unlock() +} + +func (r *pairFirstSeen) release() { + for i := range r.shards { + s := &r.shards[i] + s.mu.Lock() + s.m = nil + s.mu.Unlock() + } +} diff --git a/scan_alloc_test.go b/scan_alloc_test.go new file mode 100644 index 0000000..8db9dbf --- /dev/null +++ b/scan_alloc_test.go @@ -0,0 +1,76 @@ +package objstore + +import ( + "bytes" + "math/rand" + "slices" + "testing" +) + +func TestAddedHunksWithPosScratch_MinimalOnRandomEdits(t *testing.T) { + rng := rand.New(rand.NewSource(42)) + words := []string{"a", "b", "c", "d", "", "x y", "foo", "bar\tbaz", "longer line here", "q", "a", "b"} + gen := func(n int) []byte { + var out []byte + for i := 0; i < n; i++ { + out = append(out, words[rng.Intn(len(words))]...) + if i < n-1 || rng.Intn(3) > 0 { + out = append(out, '\n') + } + } + return out + } + mutate := func(src []byte) []byte { + lines := bytes.Split(src, []byte{'\n'}) + if len(src) > 0 && src[len(src)-1] == '\n' { + lines = lines[:len(lines)-1] + } + for k := rng.Intn(6) + 1; k > 0; k-- { + if len(lines) == 0 { + lines = append(lines, []byte(words[rng.Intn(len(words))])) + continue + } + i := rng.Intn(len(lines)) + switch rng.Intn(5) { + case 0: + lines = slices.Insert(lines, i, []byte(words[rng.Intn(len(words))])) + case 1: + lines = slices.Delete(lines, i, i+1) + case 2: + lines[i] = []byte(words[rng.Intn(len(words))]) + case 3: + j := rng.Intn(len(lines)) + lines[i], lines[j] = lines[j], lines[i] + case 4: + for m := 20 + rng.Intn(20); m > 0; m-- { + lines = slices.Insert(lines, i, []byte(words[rng.Intn(len(words))])) + } + } + } + out := bytes.Join(lines, []byte{'\n'}) + if rng.Intn(3) > 0 { + out = append(out, '\n') + } + return out + } + sc := &lineScratch{} + for iter := 0; iter < 50000; iter++ { + old := gen(rng.Intn(150)) + var nw []byte + switch rng.Intn(3) { + case 0: + nw = mutate(old) + case 1: + nw = gen(rng.Intn(150)) + case 2: + nw = append(append([]byte{}, old...), gen(rng.Intn(5))...) + } + var got []AddedHunk + if iter%2 == 0 { + got = addedHunksWithPosScratch(old, nw, sc) + } else { + got = addedHunksWithPosScratch(old, nw, nil) + } + requireMinimalAddedHunks(t, old, nw, got) + } +} diff --git a/scan_memory_test.go b/scan_memory_test.go new file mode 100644 index 0000000..7480760 --- /dev/null +++ b/scan_memory_test.go @@ -0,0 +1,108 @@ +package objstore + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Parents inside the loaded commit set resolve through the synthesized +// commit graph without touching the treeOIDs memo, so the memo stays empty +// across a scan over a complete history. +func TestFirstParentTreeIn_ResolvesLoadedCommitsThroughGraph(t *testing.T) { + for _, repo := range []string{"with-merges", "large-repo"} { + s := createScannerForRepo(t, repo) + commits, graph, err := s.loadCommitsAndGraph() + require.NoError(t, err) + require.NotNil(t, graph) + for _, c := range commits { + tree, err := s.firstParentTreeIn(graph, c) + require.NoError(t, err) + if len(c.ParentOIDs) == 0 { + assert.Equal(t, Hash{}, tree) + continue + } + idx, ok := graph.OIDToIndex[c.ParentOIDs[0]] + require.True(t, ok) + assert.Equal(t, graph.TreeOIDs[idx], tree) + } + memoized := 0 + s.treeOIDs.Range(func(_, _ any) bool { + memoized++ + return true + }) + assert.Zerof(t, memoized, "%s: parents outside the loaded set", repo) + s.Close() + } +} + +// loadCommitsAndGraph and loadAllCommits agree on the commit list, and the +// list is in parent-first order. +func TestLoadCommitsAndGraph_MatchesLoadAllCommits(t *testing.T) { + s := createScannerForRepo(t, "with-merges") + defer s.Close() + commits, graph, err := s.loadCommitsAndGraph() + require.NoError(t, err) + again, err := s.loadAllCommits() + require.NoError(t, err) + assert.Equal(t, again, commits) + assert.Equal(t, len(commits), len(graph.OrderedOIDs)) + pos := make(map[Hash]int, len(commits)) + for i, c := range commits { + pos[c.OID] = i + } + for i, c := range commits { + for _, p := range c.ParentOIDs { + if j, ok := pos[p]; ok { + assert.Less(t, j, i, "parent precedes child") + } + } + } +} + +func TestLoadCommitsAndGraph_SeesCommitsAfterEmptyLoad(t *testing.T) { + requireGit(t) + for _, dedup := range []bool{false, true} { + repo := t.TempDir() + runGit(t, repo, "init", "--quiet", "-b", "main") + var opts []ScannerOption + if dedup { + opts = append(opts, WithHunkLineDedup(true)) + } + s, err := NewHistoryScanner(filepath.Join(repo, ".git"), opts...) + require.NoError(t, err) + scan := func() int { + n := 0 + require.NoError(t, s.DiffHistoryHunksFunc(func(HunkAddition) error { n++; return nil })) + return n + } + require.Zero(t, scan(), "dedup=%v: a repository without refs has no hunks", dedup) + + require.NoError(t, os.WriteFile(filepath.Join(repo, "a.txt"), []byte("token=abc\n"), 0o644)) + runGit(t, repo, "add", "a.txt") + runGit(t, repo, "commit", "-q", "-m", "add a") + assert.Equal(t, 1, scan(), "dedup=%v: the scan after the first commit sees it", dedup) + commits, _, err := s.loadCommitsAndGraph() + require.NoError(t, err) + assert.Len(t, commits, 1, "dedup=%v", dedup) + s.Close() + } +} + +func TestWithHunkDedupRetainedBudget(t *testing.T) { + s := createScannerForRepo(t, "with-merges") + defer s.Close() + assert.Equal(t, int64(dedupRetainedBytesCap), s.dedupLimits.retainedBytesCap) + WithHunkDedupRetainedBudget(128 << 20)(s) + assert.Equal(t, int64(128<<20), s.dedupLimits.retainedBytesCap) + WithHunkDedupRetainedBudget(0)(s) + assert.Equal(t, int64(1<<20), s.dedupLimits.retainedBytesCap, "values below 1 MiB are raised to 1 MiB") + + gitDir := repeatedPairRepo(t) + want := collectHunkScan(t, gitDir, WithHunkLineDedup(true)) + got := collectHunkScan(t, gitDir, WithHunkLineDedup(true), WithHunkDedupRetainedBudget(1)) + assert.Equal(t, want, got, "a tight budget leaves the emission multiset unchanged") +} diff --git a/store.go b/store.go index 7bb542f..496116f 100644 --- a/store.go +++ b/store.go @@ -454,21 +454,34 @@ func (s *store) get(oid Hash) ([]byte, ObjectType, error) { // fallback for pack-resident objects whenever the offset cache misses // (disabled via WithOffsetCacheBudget, entry evicted, or first read). // On the hot path an offset-cache hit above returns without paying - // these two mutex acquisitions; on a miss the probes are noise next - // to the inflation they can avoid: without them, repeated reads of a + // these mutex acquisitions; on a miss the delta-window probe is noise + // next to the inflation it can avoid: without it, repeated reads of a // packed object would re-inflate every time in memory-constrained // configurations even though inflation populates the delta window. if b, ok := s.dw.acquire(oid); ok { d, t := b.Data(), b.Type() - // Promote to ARC cache on second access (delta window hit). - if len(d) <= maxCacheableSize { + // Promote to the ARC cache on second access (delta window hit). + // A pack-resident object under an enabled offset cache is already + // retained there by (pack, offset), so the ARC holds loose objects + // and the packed ones the offset cache rejects; the entry-count + // bound of the ARC otherwise pinned hundreds of megabytes of blobs + // the offset cache was already serving. + if len(d) <= maxCacheableSize && (!inPack || !s.offCache.admits(len(d))) { s.cache.Add(oid, cachedObj{data: d, typ: t}) } b.Release() return d, t, nil } - if b, ok := s.cache.Get(oid); ok { - return b.data, b.typ, nil + // When the offset cache admits every cacheable size, ARC promotion is + // restricted to loose objects, and inflateFromPack adds only to the + // delta window and the offset cache. The probe, an exclusive lock on + // the whole ARC, is therefore taken only when it can hit: a scan's + // first read of every tree and blob otherwise took that lock on every + // worker (2% of a rails scan's CPU). + if !inPack || !s.offCache.admitsCacheable() { + if b, ok := s.cache.Get(oid); ok { + return b.data, b.typ, nil + } } if !packLookupDone { diff --git a/store_test.go b/store_test.go index 91073a6..0f4cfd2 100644 --- a/store_test.go +++ b/store_test.go @@ -203,6 +203,60 @@ func TestGetFallsBackToOIDCachesWhenOffsetCacheDisabled(t *testing.T) { "second read must hit the delta window and promote to the ARC cache") } +// TestGetKeepsPackedObjectsOutOfARCWhenOffsetCacheEnabled verifies that a +// pack-resident object re-read under an enabled offset cache stays out of +// the entry-count-bounded ARC: the offset cache already retains it by +// (pack, offset), and a second copy pinned by the ARC was the largest +// avoidable heap component of a history scan. +func TestGetKeepsPackedObjectsOutOfARCWhenOffsetCacheEnabled(t *testing.T) { + packPath, _, cleanup := createTestPackWithDelta(t) + defer cleanup() + + store, err := OpenForTesting(filepath.Dir(packPath)) + require.NoError(t, err) + defer store.Close() + require.True(t, store.offCache.enabled()) + + targetHash := calculateHash(ObjBlob, []byte("modified data")) + want, _, err := store.get(targetHash) + require.NoError(t, err) + got, _, err := store.get(targetHash) + require.NoError(t, err) + require.Equal(t, want, got) + + _, ok := store.cache.Get(targetHash) + require.False(t, ok, "packed object must be served by the offset cache, not pinned by the ARC") +} + +func TestGetPromotesPackedObjectsTheOffsetCacheRejects(t *testing.T) { + packPath, _, cleanup := createTestPackWithDelta(t) + defer cleanup() + + store, err := OpenForTesting(filepath.Dir(packPath)) + require.NoError(t, err) + defer store.Close() + // A one-byte budget per shard keeps the offset cache enabled while + // forcing it to reject the target object. + store.offCache.setBudget(offsetCacheShards) + require.True(t, store.offCache.enabled()) + + targetHash := calculateHash(ObjBlob, []byte("modified data")) + want, _, err := store.get(targetHash) + require.NoError(t, err) + require.False(t, store.offCache.admits(len(want))) + got, _, err := store.get(targetHash) + require.NoError(t, err) + require.Equal(t, want, got) + + cached, ok := store.cache.Get(targetHash) + require.True(t, ok, "a packed object the offset cache rejects must be promoted to the ARC") + require.Equal(t, want, cached.data) + + got, _, err = store.get(targetHash) + require.NoError(t, err) + require.Equal(t, want, got) +} + // TestParseIdx validates the v2 pack-index parser against invalid magic bytes, // unsupported versions, minimal valid indices, large-offset handling, // multi-object sorted order, and truncated files.