-
-
Notifications
You must be signed in to change notification settings - Fork 596
Optimize commit iterator #9751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize commit iterator #9751
Conversation
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
fulghum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good. A few small things to clean up.
| // 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) { | ||
| func (cmItr *commitItr[C]) Next(ctx C) (hash.Hash, *OptionalCommit, *datas.CommitMeta, uint64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this Next() implementation will never actually return a *CommitMeta instance, right? That makes this interface a little bit tricky/frustrating to use when the caller doesn't know which implementations return which fields. At minimum, it's worth documenting this better, perhaps in the main interface definition (to document that some implementations may not return CommitMeta and callers should always test for nil before using that param) and perhaps also in the implementations that don't return CommitMeta data since they are not really fully implementing the interface's contract.
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
Changes:
The result of these changes is roughly 1.7x speed up in
select * from dolt_log()queries.Addresses: #9743