On 4/18/2018 8:04 PM, Jakub Narebski wrote:
Derrick Stolee <dsto...@microsoft.com> writes:
-- >8 --
This is the one of several "small" patches that follow the serialized
Git commit graph patch (ds/commit-graph) and lazy-loading trees
(ds/lazy-load-trees).
As described in Documentation/technical/commit-graph.txt, the generation
number of a commit is one more than the maximum generation number among
its parents (trivially, a commit with no parents has generation number
one). This section is expanded to describe the interaction with special
generation numbers GENERATION_NUMBER_INFINITY (commits not in the commit-graph
file) and *_ZERO (commits in a commit-graph file written before generation
numbers were implemented).
This series makes the computation of generation numbers part of the
commit-graph write process.
Finally, generation numbers are used to order commits in the priority
queue in paint_down_to_common(). This allows a short-circuit mechanism
to improve performance of `git branch --contains`.
Further, use generation numbers for 'git tag --contains', providing a
significant speedup (at least 95% for some cases).
A more substantial refactoring of revision.c is required before making
'git log --graph' use generation numbers effectively.
This patch series is build on ds/lazy-load-trees.
Derrick Stolee (9):
commit: add generation number to struct commmit
Nice and short patch. Looks good to me.
commit-graph: compute generation numbers
Another quite easy to understand patch. LGTM.
commit: use generations in paint_down_to_common()
Nice and short patch; minor typo in comment in code.
Otherwise it looks good to me.
commit-graph.txt: update design document
I see that diagram got removed in this version; maybe it could be
replaced with relationship table?
Anyway, it looks good to me.
The diagrams and tables seemed to cause more confusion than clarity. I
think the reader should create their own mental model from the
definitions and description and we should avoid trying to make a summary.
ref-filter: use generation number for --contains
A question: how performance looks like after the change if commit-graph
is not available?
The performance issue is minor, but will be fixed in v4.
commit: use generation numbers for in_merge_bases()
Possible typo in the commit message, and stylistic inconsistence in
in_merge_bases() - though actually more clear than existing code.
Short, simple, and gives good performance improvenebts.
commit: add short-circuit to paint_down_to_common()
Looks good to me; ignore [mostly] what I have written in response to the
patch in question.
commit-graph: always load commit-graph information
Looks all right; question: parameter or one more global variable.
I responded to say that the global variable approach is incorrect.
Parameter is important to functionality and performance.
merge: check config before loading commits
This looks good to me.
Documentation/technical/commit-graph.txt | 30 +++++--
alloc.c | 1 +
builtin/merge.c | 5 +-
commit-graph.c | 99 +++++++++++++++++++-----
commit-graph.h | 8 ++
commit.c | 54 +++++++++++--
commit.h | 7 +-
object.c | 2 +-
ref-filter.c | 23 +++++-
sha1_file.c | 2 +-
t/t5318-commit-graph.sh | 9 +++
11 files changed, 199 insertions(+), 41 deletions(-)
base-commit: 7b8a21dba1bce44d64bd86427d3d92437adc4707
