catfile: Stop requiring object info reader to read tree entries

We don't need the objecct info reader anymore to read tree entries. This
allows us to not spawn a separate process in some RPCs, which should
result in a small speedup.

5 files changed, 19 insertions, 32 deletions

diff --git a/internal/git/catfile/tree_entries.go b/internal/git/catfile/tree_entries.go
index bdbf1b7f3..6e0d71827 100644
--- a/internal/git/catfile/tree_entries.go
+++ b/internal/git/catfile/tree_entries.go
@@ -19,17 +19,15 @@ type revisionPath struct{ revision, path string }
 
 // TreeEntryFinder is a struct for searching through a tree with caching.
 type TreeEntryFinder struct {
-	objectReader     ObjectContentReader
-	objectInfoReader ObjectInfoReader
-	treeCache        map[revisionPath][]*gitalypb.TreeEntry
+	objectReader ObjectContentReader
+	treeCache    map[revisionPath][]*gitalypb.TreeEntry
 }
 
 // NewTreeEntryFinder initializes a TreeEntryFinder with an empty tree cache.
-func NewTreeEntryFinder(objectReader ObjectContentReader, objectInfoReader ObjectInfoReader) *TreeEntryFinder {
+func NewTreeEntryFinder(objectReader ObjectContentReader) *TreeEntryFinder {
 	return &TreeEntryFinder{
-		objectReader:     objectReader,
-		objectInfoReader: objectInfoReader,
-		treeCache:        make(map[revisionPath][]*gitalypb.TreeEntry),
+		objectReader: objectReader,
+		treeCache:    make(map[revisionPath][]*gitalypb.TreeEntry),
 	}
 }
 
@@ -48,7 +46,7 @@ func (tef *TreeEntryFinder) FindByRevisionAndPath(ctx context.Context, revision,
 
 	if !ok {
 		var err error
-		entries, err = TreeEntries(ctx, tef.objectReader, tef.objectInfoReader, revision, dir)
+		entries, err = TreeEntries(ctx, tef.objectReader, revision, dir)
 		if err != nil {
 			return nil, err
 		}
@@ -117,7 +115,6 @@ func extractEntryInfoFromTreeData(
 func TreeEntries(
 	ctx context.Context,
 	objectReader ObjectContentReader,
-	objectInfoReader ObjectInfoReader,
 	revision, path string,
 ) (_ []*gitalypb.TreeEntry, returnedErr error) {
 	span, ctx := tracing.StartSpanIfHasParent(
diff --git a/internal/gitaly/service/blob/get_blobs.go b/internal/gitaly/service/blob/get_blobs.go
index 167ba899a..c21cd3c20 100644
--- a/internal/gitaly/service/blob/get_blobs.go
+++ b/internal/gitaly/service/blob/get_blobs.go
@@ -27,7 +27,7 @@ func sendGetBlobsResponse(
 ) error {
 	ctx := stream.Context()
 
-	tef := catfile.NewTreeEntryFinder(objectReader, objectInfoReader)
+	tef := catfile.NewTreeEntryFinder(objectReader)
 
 	for _, revisionPath := range req.RevisionPaths {
 		revision := revisionPath.Revision
diff --git a/internal/gitaly/service/commit/tree_entries.go b/internal/gitaly/service/commit/tree_entries.go
index ec796d40e..d7974a2b7 100644
--- a/internal/gitaly/service/commit/tree_entries.go
+++ b/internal/gitaly/service/commit/tree_entries.go
@@ -42,7 +42,6 @@ func validateGetTreeEntriesRequest(in *gitalypb.GetTreeEntriesRequest) error {
 func populateFlatPath(
 	ctx context.Context,
 	objectReader catfile.ObjectContentReader,
-	objectInfoReader catfile.ObjectInfoReader,
 	entries []*gitalypb.TreeEntry,
 ) error {
 	for _, entry := range entries {
@@ -53,7 +52,7 @@ func populateFlatPath(
 		}
 
 		for i := 1; i < defaultFlatTreeRecursion; i++ {
-			subEntries, err := catfile.TreeEntries(ctx, objectReader, objectInfoReader, entry.CommitOid, string(entry.FlatPath))
+			subEntries, err := catfile.TreeEntries(ctx, objectReader, entry.CommitOid, string(entry.FlatPath))
 			if err != nil {
 				return err
 			}
@@ -82,10 +81,7 @@ func (s *server) sendTreeEntries(
 
 	var entries []*gitalypb.TreeEntry
 
-	var (
-		objectReader     catfile.ObjectContentReader
-		objectInfoReader catfile.ObjectInfoReader
-	)
+	var objectReader catfile.ObjectContentReader
 
 	// When we want to do a recursive listing, then it's a _lot_ more efficient to let
 	// git-ls-tree(1) handle this for us. In theory, we'd also want to do this for the
@@ -146,13 +142,7 @@ func (s *server) sendTreeEntries(
 		}
 		defer cancel()
 
-		objectInfoReader, cancel, err = s.catfileCache.ObjectInfoReader(stream.Context(), repo)
-		if err != nil {
-			return err
-		}
-		defer cancel()
-
-		entries, err = catfile.TreeEntries(ctx, objectReader, objectInfoReader, revision, path)
+		entries, err = catfile.TreeEntries(ctx, objectReader, revision, path)
 		if err != nil {
 			return err
 		}
@@ -192,7 +182,7 @@ func (s *server) sendTreeEntries(
 		// of my knowledge is as good as we can get. Doing this via git-ls-tree(1)
 		// wouldn't fly: we'd have to spawn a separate process for each of the
 		// subtrees, which is a lot of overhead.
-		if err := populateFlatPath(ctx, objectReader, objectInfoReader, entries); err != nil {
+		if err := populateFlatPath(ctx, objectReader, entries); err != nil {
 			return err
 		}
 	}
diff --git a/internal/gitaly/service/commit/tree_entry.go b/internal/gitaly/service/commit/tree_entry.go
index 3d43d0ddb..c61809174 100644
--- a/internal/gitaly/service/commit/tree_entry.go
+++ b/internal/gitaly/service/commit/tree_entry.go
@@ -22,7 +22,7 @@ func sendTreeEntry(
 ) error {
 	ctx := stream.Context()
 
-	treeEntry, err := catfile.NewTreeEntryFinder(objectReader, objectInfoReader).FindByRevisionAndPath(ctx, revision, path)
+	treeEntry, err := catfile.NewTreeEntryFinder(objectReader).FindByRevisionAndPath(ctx, revision, path)
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go
index 6cef6c272..b7e948414 100644
--- a/internal/gitaly/service/repository/archive.go
+++ b/internal/gitaly/service/repository/archive.go
@@ -137,13 +137,7 @@ func (s *server) validateGetArchivePrecondition(
 	}
 	defer cancel()
 
-	objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader(ctx, repo)
-	if err != nil {
-		return err
-	}
-	defer cancel()
-
-	f := catfile.NewTreeEntryFinder(objectReader, objectInfoReader)
+	f := catfile.NewTreeEntryFinder(objectReader)
 	if path != "." {
 		if ok, err := findGetArchivePath(ctx, f, commitID, path); err != nil {
 			return err
@@ -151,6 +145,12 @@ func (s *server) validateGetArchivePrecondition(
 			return structerr.NewFailedPrecondition("path doesn't exist")
 		}
 	} else {
+		objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader(ctx, repo)
+		if err != nil {
+			return err
+		}
+		defer cancel()
+
 		repoHash, err := repo.ObjectHash(ctx)
 		if err != nil {
 			return err