Optimize commit iterator

go/cmd/dolt/commands/archive.go

@@ -250,7 +250,7 @@ func historicalFuzzyMatching(ctx context.Context, heads hash.HashSet, groupings
 		return err
 	}
 	for {
-		h, _, err := iterator.Next(ctx)
+		h, _, _, _, err := iterator.Next(ctx)
 		if err != nil {
 			if err == io.EOF {
 				break

go/libraries/doltcore/doltdb/commit_itr.go

@@ -28,9 +28,11 @@ import (
 
 // CommitItr is an interface for iterating over a set of unique commits
 type CommitItr[C Context] interface {
-	// Next returns the hash of the next commit, and a pointer to that commit.  Implementations of Next must handle
-	// making sure the list of commits returned are unique.  When complete Next will return hash.Hash{}, nil, io.EOF
-	Next(ctx C) (hash.Hash, *OptionalCommit, error)
+	// Next returns the next commit's hash, pointer, metadata, and height.
+	// Implementations of Next must handle making sure the list of commits returned are unique.
+	// When complete Next will return hash.Hash{}, nil, nil, 0, io.EOF
+	// Note: Some implementations may always return nil for the metadata and height return values.
+	Next(ctx C) (hash.Hash, *OptionalCommit, *datas.CommitMeta, uint64, error)
 
 	// Reset the commit iterator back to the start
 	Reset(ctx context.Context) error
@@ -88,25 +90,27 @@ func (cmItr *commitItr[C]) Reset(ctx context.Context) error {
 	return nil
 }
 
-// Next returns the hash of the next commit, and a pointer to that commit.  It handles making sure the list of commits
-// returned are unique.  When complete Next will return hash.Hash{}, nil, io.EOF
-func (cmItr *commitItr[C]) Next(ctx C) (hash.Hash, *OptionalCommit, error) {
+// Next returns the hash of the next commit, and a pointer to that commit.
+// It handles making sure the list of commits returned are unique.
+// When complete Next will return hash.Hash{}, nil, nil, 0, io.EOF.
+// Note: This implementation always returns nil for the metadata and height return values.
+func (cmItr *commitItr[C]) Next(ctx C) (hash.Hash, *OptionalCommit, *datas.CommitMeta, uint64, error) {
 	for cmItr.curr == nil {
 		if cmItr.currentRoot >= len(cmItr.rootCommits) {
-			return hash.Hash{}, nil, io.EOF
+			return hash.Hash{}, nil, nil, 0, io.EOF
 		}
 
 		cm := cmItr.rootCommits[cmItr.currentRoot]
 		h, err := cm.HashOf()
 
 		if err != nil {
-			return hash.Hash{}, nil, err
+			return hash.Hash{}, nil, nil, 0, err
 		}
 
 		if !cmItr.added[h] {
 			cmItr.added[h] = true
 			cmItr.curr = cm
-			return h, &OptionalCommit{cmItr.curr, h}, nil
+			return h, &OptionalCommit{cmItr.curr, h}, nil, 0, nil
 		}
 
 		cmItr.currentRoot++
@@ -115,7 +119,7 @@ func (cmItr *commitItr[C]) Next(ctx C) (hash.Hash, *OptionalCommit, error) {
 	parents, err := cmItr.curr.ParentHashes(ctx)
 
 	if err != nil {
-		return hash.Hash{}, nil, err
+		return hash.Hash{}, nil, nil, 0, err
 	}
 
 	for _, h := range parents {
@@ -137,13 +141,13 @@ func (cmItr *commitItr[C]) Next(ctx C) (hash.Hash, *OptionalCommit, error) {
 	cmItr.unprocessed = cmItr.unprocessed[:numUnprocessed-1]
 	cmItr.curr, err = HashToCommit(ctx, cmItr.ddb.ValueReadWriter(), cmItr.ddb.ns, next)
 	if err != nil && err != ErrGhostCommitEncountered {
-		return hash.Hash{}, nil, err
+		return hash.Hash{}, nil, nil, 0, err
 	}
 	if err == ErrGhostCommitEncountered {
 		cmItr.curr = nil
 	}
 
-	return next, &OptionalCommit{cmItr.curr, next}, nil
+	return next, &OptionalCommit{cmItr.curr, next}, nil, 0, nil
 }
 
 func HashToCommit(ctx context.Context, vrw types.ValueReadWriter, ns tree.NodeStore, h hash.Hash) (*Commit, error) {
@@ -183,19 +187,18 @@ func NewFilteringCommitItr[C Context](itr CommitItr[C], filter CommitFilter[C])
 
 // Next returns the hash of the next commit, and a pointer to that commit.  Implementations of Next must handle
 // making sure the list of commits returned are unique.  When complete Next will return hash.Hash{}, nil, io.EOF
-func (itr FilteringCommitItr[C]) Next(ctx C) (hash.Hash, *OptionalCommit, error) {
+func (itr FilteringCommitItr[C]) Next(ctx C) (hash.Hash, *OptionalCommit, *datas.CommitMeta, uint64, error) {
 	// iteration will terminate on io.EOF or a commit that is !filteredOut
 	for {
-		h, cm, err := itr.itr.Next(ctx)
-
+		h, cm, meta, height, err := itr.itr.Next(ctx)
 		if err != nil {
-			return hash.Hash{}, nil, err
+			return hash.Hash{}, nil, nil, 0, err
 		}
 
 		if filterOut, err := itr.filter(ctx, h, cm); err != nil {
-			return hash.Hash{}, nil, err
+			return hash.Hash{}, nil, nil, 0, err
 		} else if !filterOut {
-			return h, cm, nil
+			return h, cm, meta, height, nil
 		}
 	}
 }
@@ -217,12 +220,14 @@ type CommitSliceIter[C Context] struct {
 
 var _ CommitItr[context.Context] = (*CommitSliceIter[context.Context])(nil)
 
-func (i *CommitSliceIter[C]) Next(ctx C) (hash.Hash, *OptionalCommit, error) {
+// Next implements the CommitItr interface.
+// Note: This implementation always returns nil for the metadata and height return values.
+func (i *CommitSliceIter[C]) Next(ctx C) (hash.Hash, *OptionalCommit, *datas.CommitMeta, uint64, error) {
 	if i.i >= len(i.h) {
-		return hash.Hash{}, nil, io.EOF
+		return hash.Hash{}, nil, nil, 0, io.EOF
 	}
 	i.i++
-	return i.h[i.i-1], &OptionalCommit{i.cm[i.i-1], i.h[i.i-1]}, nil
+	return i.h[i.i-1], &OptionalCommit{i.cm[i.i-1], i.h[i.i-1]}, nil, 0, nil
 
 }
 
@@ -232,7 +237,7 @@ func (i *CommitSliceIter[C]) Reset(ctx context.Context) error {
 }
 
 func NewOneCommitIter[C Context](cm *Commit, h hash.Hash, meta *datas.CommitMeta) *OneCommitIter[C] {
-	return &OneCommitIter[C]{cm: &OptionalCommit{cm, h}, h: h}
+	return &OneCommitIter[C]{cm: &OptionalCommit{cm, h}, h: h, m: meta}
 }
 
 type OneCommitIter[C Context] struct {
@@ -244,13 +249,14 @@ type OneCommitIter[C Context] struct {
 
 var _ CommitItr[context.Context] = (*OneCommitIter[context.Context])(nil)
 
-func (i *OneCommitIter[C]) Next(_ C) (hash.Hash, *OptionalCommit, error) {
+// Next implements the CommitItr interface.
+// Note: This implementation always returns nil for the metadata and height return values.
+func (i *OneCommitIter[C]) Next(_ C) (hash.Hash, *OptionalCommit, *datas.CommitMeta, uint64, error) {
 	if i.done {
-		return hash.Hash{}, nil, io.EOF
+		return hash.Hash{}, nil, nil, 0, io.EOF
 	}
 	i.done = true
-	return i.h, i.cm, nil
-
+	return i.h, i.cm, i.m, 0, nil
 }
 
 func (i *OneCommitIter[C]) Reset(_ context.Context) error {

go/libraries/doltcore/doltdb/doltdb.go

@@ -474,6 +474,23 @@ func (ddb *DoltDB) Resolve(ctx context.Context, cs *CommitSpec, cwb ref.DoltRef)
 	return commit.GetAncestor(ctx, cs.aSpec)
 }
 
+// ResolveHash takes a hash and returns an OptionalCommit directly.
+// This assumes no ancestor spec resolution and no current working branch (cwb) needed.
+func (ddb *DoltDB) ResolveHash(ctx context.Context, hash hash.Hash) (*OptionalCommit, error) {
+	commitValue, err := datas.LoadCommitAddr(ctx, ddb.vrw, hash)
+	if err != nil {
+		return nil, err
+	}
+	if commitValue.IsGhost() {
+		return &OptionalCommit{nil, hash}, nil
+	}
+	commit, err := NewCommit(ctx, ddb.vrw, ddb.ns, commitValue)
+	if err != nil {
+		return nil, err
+	}
+	return &OptionalCommit{commit, hash}, nil
+}
+
 // BootstrapShallowResolve is a special case of Resolve that is used to resolve a commit prior to pulling it's history
 // in a shallow clone. In general, application code should call Resolve and get an OptionalCommit. This is a special case
 // where we need to get the head commit for the commit closure used to determine what commits should skipped.

go/libraries/doltcore/env/actions/commitwalk/commitwalk.go

@@ -69,23 +69,25 @@ func (q *q) Swap(i, j int) {
 // the commit with the newer timestamp is "less". Finally if both commits are ghost commits, we don't really have enough
 // information to compare on, so we just compare the hashes to ensure that the results are stable.
 func (q *q) Less(i, j int) bool {
-	_, okI := q.pending[i].commit.ToCommit()
-	_, okJ := q.pending[i].commit.ToCommit()
+	cI := q.pending[i]
+	cJ := q.pending[j]
+	_, okI := cI.commit.ToCommit()
+	_, okJ := cJ.commit.ToCommit()
 
 	if !okI && okJ {
 		return true
 	} else if okI && !okJ {
 		return false
 	} else if !okI && !okJ {
-		return q.pending[i].hash.String() < q.pending[j].hash.String()
+		return cI.hash.String() < cJ.hash.String()
 	}
 
-	if q.pending[i].height > q.pending[j].height {
+	if cI.height > cJ.height {
 		return true
 	}
 
-	if q.pending[i].height == q.pending[j].height {
-		return q.pending[i].meta.UserTimestamp > q.pending[j].meta.UserTimestamp
+	if cI.height == cJ.height {
+		return cI.meta.UserTimestamp > cJ.meta.UserTimestamp
 	}
 	return false
 }
@@ -128,15 +130,11 @@ func (q *q) SetInvisible(ctx context.Context, ddb *doltdb.DoltDB, id hash.Hash)
 }
 
 func load(ctx context.Context, ddb *doltdb.DoltDB, h hash.Hash) (*doltdb.OptionalCommit, error) {
-	cs, err := doltdb.NewCommitSpec(h.String())
+	oc, err := ddb.ResolveHash(ctx, h)
 	if err != nil {
 		return nil, err
 	}
-	c, err := ddb.Resolve(ctx, cs, nil)
-	if err != nil {
-		return nil, err
-	}
-	return c, nil
+	return oc, nil
 }
 
 func (q *q) Get(ctx context.Context, ddb *doltdb.DoltDB, id hash.Hash) (*c, error) {
@@ -158,14 +156,21 @@ func (q *q) Get(ctx context.Context, ddb *doltdb.DoltDB, id hash.Hash) (*c, erro
 	if err != nil {
 		return nil, err
 	}
+
 	meta, err := commit.GetCommitMeta(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	c := &c{ddb: ddb, commit: &doltdb.OptionalCommit{Commit: commit, Addr: id}, meta: meta, height: h, hash: id}
-	q.loaded[id] = c
-	return c, nil
+	lc := &c{
+		ddb:    ddb,
+		commit: &doltdb.OptionalCommit{Commit: commit, Addr: id},
+		meta:   meta,
+		height: h,
+		hash:   id,
+	}
+	q.loaded[id] = lc
+	return lc, nil
 }
 
 func newQueue() *q {
@@ -190,7 +195,7 @@ func GetDotDotRevisions(ctx context.Context, includedDB *doltdb.DoltDB, included
 
 	var commitList []*doltdb.OptionalCommit
 	for num < 0 || len(commitList) < num {
-		_, commit, err := itr.Next(ctx)
+		_, commit, _, _, err := itr.Next(ctx)
 		if err == io.EOF {
 			break
 		} else if err != nil {
@@ -234,43 +239,35 @@ func newCommiterator[C doltdb.Context](ctx context.Context, ddb *doltdb.DoltDB,
 }
 
 // Next implements doltdb.CommitItr
-func (iter *commiterator[C]) Next(ctx C) (hash.Hash, *doltdb.OptionalCommit, error) {
+func (iter *commiterator[C]) Next(ctx C) (hash.Hash, *doltdb.OptionalCommit, *datas.CommitMeta, uint64, error) {
 	if iter.q.NumVisiblePending() > 0 {
 		nextC := iter.q.PopPending()
-
-		var err error
-		parents := []hash.Hash{}
 		commit, ok := nextC.commit.ToCommit()
 		if ok {
-			parents, err = commit.ParentHashes(ctx)
-			if err != nil {
-				return hash.Hash{}, nil, err
-			}
-		}
-
-		for _, parentID := range parents {
-			if err := iter.q.AddPendingIfUnseen(ctx, nextC.ddb, parentID); err != nil {
-				return hash.Hash{}, nil, err
+			for _, parentCommit := range commit.DatasParents() {
+				if err := iter.q.AddPendingIfUnseen(ctx, nextC.ddb, parentCommit.Addr()); err != nil {
+					return hash.Hash{}, nil, nil, 0, err
+				}
 			}
 		}
 
+		var err error
 		matches := true
 		if iter.matchFn != nil {
 			matches, err = iter.matchFn(nextC.commit)
-
 			if err != nil {
-				return hash.Hash{}, nil, err
+				return hash.Hash{}, nil, nil, 0, err
 			}
 		}
 
 		if matches {
-			return nextC.hash, &doltdb.OptionalCommit{Commit: commit, Addr: nextC.hash}, nil
+			return nextC.hash, &doltdb.OptionalCommit{Commit: commit, Addr: nextC.hash}, nextC.meta, nextC.height, nil
 		}
 
 		return iter.Next(ctx)
 	}
 
-	return hash.Hash{}, nil, io.EOF
+	return hash.Hash{}, nil, nil, 0, io.EOF
 }
 
 // Reset implements doltdb.CommitItr
@@ -301,7 +298,7 @@ func GetTopNTopoOrderedCommitsMatching(ctx context.Context, ddb *doltdb.DoltDB,
 
 	var commitList []*doltdb.Commit
 	for n < 0 || len(commitList) < n {
-		_, optCmt, err := itr.Next(ctx)
+		_, optCmt, _, _, err := itr.Next(ctx)
 		if err == io.EOF {
 			break
 		} else if err != nil {
@@ -345,47 +342,47 @@ func newDotDotCommiterator[C doltdb.Context](ctx context.Context, includedDdb *d
 }
 
 // Next implements doltdb.CommitItr
-func (i *dotDotCommiterator[C]) Next(ctx C) (hash.Hash, *doltdb.OptionalCommit, error) {
+func (i *dotDotCommiterator[C]) Next(ctx C) (hash.Hash, *doltdb.OptionalCommit, *datas.CommitMeta, uint64, error) {
 	if i.q.NumVisiblePending() > 0 {
 		nextC := i.q.PopPending()
 
 		commit, ok := nextC.commit.ToCommit()
 		if !ok {
-			return nextC.hash, nextC.commit, nil
+			return nextC.hash, nextC.commit, nextC.meta, nextC.height, nil
 		}
 
 		parents, err := commit.ParentHashes(ctx)
 		if err != nil {
-			return hash.Hash{}, nil, err
+			return hash.Hash{}, nil, nil, 0, err
 		}
 
 		for _, parentID := range parents {
 			if nextC.invisible {
 				if err := i.q.SetInvisible(ctx, nextC.ddb, parentID); err != nil {
-					return hash.Hash{}, nil, err
+					return hash.Hash{}, nil, nil, 0, err
 				}
 			}
 			if err := i.q.AddPendingIfUnseen(ctx, nextC.ddb, parentID); err != nil {
-				return hash.Hash{}, nil, err
+				return hash.Hash{}, nil, nil, 0, err
 			}
 		}
 
 		matches := true
 		if i.matchFn != nil {
 			matches, err = i.matchFn(nextC.commit)
 			if err != nil {
-				return hash.Hash{}, nil, err
+				return hash.Hash{}, nil, nil, 0, err
 			}
 		}
 
-		// If not invisible, return commit. Otherwise get next commit
+		// If not invisible, return commit. Otherwise, get next commit
 		if !nextC.invisible && matches {
-			return nextC.hash, nextC.commit, nil
+			return nextC.hash, nextC.commit, nextC.meta, nextC.height, nil
 		}
 		return i.Next(ctx)
 	}
 
-	return hash.Hash{}, nil, io.EOF
+	return hash.Hash{}, nil, nil, 0, io.EOF
 }
 
 // Reset implements doltdb.CommitItr

go/libraries/doltcore/rebase/rebase.go

@@ -200,7 +200,7 @@ func findRebaseCommits(ctx *sql.Context, currentBranchCommit, upstreamBranchComm
 	// Drain the iterator into a slice so that we can easily reverse the order of the commits
 	// so that the oldest commit is first in the generated rebase plan.
 	for {
-		_, optCmt, err := commitItr.Next(ctx)
+		_, optCmt, _, _, err := commitItr.Next(ctx)
 		if err == io.EOF {
 			return commits, nil
 		} else if err != nil {