git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 11/12] line-log: try to use generation number-based topo-ordering

From
SZEDER Gábor via GitGitGadget <gitgitgadget@gmail.com>
Date
May 1, 2020, 15:30 UTC
Message-ID
<3abc713092456b7c34ac72c0064b0b5c51ac726f.1588347029.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.622.git.1588347029.gitgitgadget@gmail.com>
From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <szeder.dev@gmail.com>

The previous patch made it possible to perform line-level filtering during history traversal instead of in an expensive preprocessing step, but it still requires some simpler preprocessing steps, notably topo-ordering. However, nowadays we have commit-graphs storing generation numbers, which make it possible to incrementally traverse the history in topological order, without the preparatory limit_list() and sort_in_topological_order() steps; see b45424181e (revision.c: generation-based topo-order algorithm, 2018-11-01).

This patch combines the two, so we can do both the topo-ordering and the line-level filtering during history traversal, eliminating even those simpler preprocessing steps, and thus further reducing the delay before showing the first commit modifying the given line range.

The 'revs->limited' flag plays the central role in this, because, due to limitations of the current implementation, the generation number-based topo-ordering is only enabled when this flag remains unset. Line-level log, however, always sets this flag in setup_revisions() ever since the feature was introduced in 12da1d1f6f (Implement line-history search (git log -L), 2013-03-28). The reason for setting 'limited' is unclear, though, because the line-level log itself doesn't directly depend on it, and it doesn't affect how the limit_list() function limits the revision range. However, there is an indirect dependency: the line-level log requires topo-ordering, and the "traditional" sort_in_topological_order() requires an already limited commit list since e6c3505b44 (Make sure we generate the whole commit list before trying to sort it topologically, 2005-07-06). The new, generation numbers-based topo-ordering doesn't require a limited commit list anymore.

So don't set 'revs->limited' for line-level log, unless it is really necessary, namely:

  - The user explicitly requested parent rewriting, because that is
    still done in the line_log_filter() preprocessing step (see
    previous patch), which requires sort_in_topological_order() and in
    turn limit_list() as well.
  - A commit-graph file is not available or it doesn't yet contain
    generation numbers.  In these cases we had to fall back on
    sort_in_topological_order() and in turn limit_list().  The
    existing condition with generation_numbers_enabled() has already
    ensured that the 'limited' flag is set in these cases; this patch
    just makes sure that the line-level log sets 'revs->topo_order'
    before that condition.

While the reduced delay before showing the first commit is measurable in git.git, it takes a bigger repository to make it clearly noticable. In both cases below the line ranges were chosen so that they were modified rather close to the starting revisions, so the effect of this change is most noticable.

  # git.git
  $ time git --no-pager log -L:read_alternate_refs:sha1-file.c -1 v2.23.0
  Before:
    real    0m0.107s
    user    0m0.091s
    sys     0m0.013s
  After:
    real    0m0.058s
    user    0m0.050s
    sys     0m0.005s
  # linux.git
  $ time git --no-pager log \
    -L:build_restore_work_registers:arch/mips/mm/tlbex.c -1 v5.2
  Before:
    real   0m1.129s
    user   0m1.061s
    sys    0m0.069s
  After:
    real   0m0.096s
    user   0m0.087s
    sys    0m0.009s

Additional testing by Derrick Stolee: Since this patch improves the performance for the first result, I repeated the experiment from the previous patch on the Linux kernel repository:

    Command: git log -L 100,200:MAINTAINERS -n 1 >/dev/null
     Before: 0.71 s
      After: 0.05 s

Now, we have dropped the full topo-order of all ~910,000 commits before reporting the first result. The remaining performance improvements then are:

 1. Update the parent-rewriting logic to be incremental similar to
    how "git log --graph" behaves.
 2. Use changed-path Bloom filters to reduce the time spend in the
    tree-diff to see if the path(s) changed.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 revision.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/revision.c b/revision.c
index 3228db9af6d..3356ede9a20 100644
--- a/revision.c
+++ b/revision.c
@@ -2790,6 +2790,12 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->diffopt.objfind)
 		revs->simplify_history = 0;
 
+	if (revs->line_level_traverse) {
+		if (want_ancestry(revs))
+			revs->limited = 1;
+		revs->topo_order = 1;
+	}
+
 	if (revs->topo_order && !generation_numbers_enabled(the_repository))
 		revs->limited = 1;
 
@@ -2809,11 +2815,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	revs->diffopt.abbrev = revs->abbrev;
 
-	if (revs->line_level_traverse) {
-		revs->limited = 1;
-		revs->topo_order = 1;
-	}
-
 	diff_setup_done(&revs->diffopt);
 
 	grep_commit_pattern_type(GREP_PATTERN_TYPE_UNSPECIFIED,
-- 
gitgitgadget
Previous: Taylor BlauNext: Taylor Blau
Message 20 of 42 in “Integrating line-log and changed-path Bloom filters”
  1. 00/12 Integrating line-log and changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 1, 2020
  2. 01/12 bloom: fix whitespace around tab lengthDerrick Stolee via GitGitGadget, May 1, 2020
  3. Taylor BlauMay 1, 2020
  4. 03/12 Documentation: changed-path Bloom filters use byte wordsDerrick Stolee via GitGitGadget, May 1, 2020
  5. Taylor BlauMay 1, 2020
  6. 02/12 test-bloom: fix usage typoDerrick Stolee via GitGitGadget, May 1, 2020
  7. Taylor BlauMay 1, 2020
  8. 04/12 bloom: de-duplicate directory entriesDerrick Stolee via GitGitGadget, May 1, 2020
  9. Taylor BlauMay 1, 2020
  10. 05/12 bloom: parse commit before computing filtersDerrick Stolee via GitGitGadget, May 1, 2020
  11. Taylor BlauMay 1, 2020
  12. 07/12 completion: offer '--(no-)patch' among 'git log' optionsSZEDER Gábor via GitGitGadget, May 1, 2020
  13. Taylor BlauMay 1, 2020
  14. 08/12 line-log: remove unused fields from 'struct line_log_data'SZEDER Gábor via GitGitGadget, May 1, 2020
  15. Taylor BlauMay 1, 2020
  16. 06/12 bloom: use num_changes not nr for limit detectionDerrick Stolee via GitGitGadget, May 1, 2020
  17. Taylor BlauMay 1, 2020
  18. 09/12 t4211-line-log: add tests for parent oidsSZEDER Gábor via GitGitGadget, May 1, 2020
  19. Taylor BlauMay 4, 2020
  20. 11/12 line-log: try to use generation number-based topo-orderingSZEDER Gábor via GitGitGadget, May 1, 2020
  21. Taylor BlauMay 4, 2020
  22. 10/12 line-log: more responsive, incremental 'git log -L'SZEDER Gábor via GitGitGadget, May 1, 2020
  23. Taylor BlauMay 4, 2020
  24. Derrick StoleeMay 4, 2020
  25. 12/12 line-log: integrate with changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 1, 2020
  26. Taylor BlauMay 4, 2020
  27. Junio C HamanoMay 1, 2020
  28. 00/12 Integrating line-log and changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 11, 2020
  29. 02/12 test-bloom: fix usage typoDerrick Stolee via GitGitGadget, May 11, 2020
  30. 03/12 bloom: parse commit before computing filtersDerrick Stolee via GitGitGadget, May 11, 2020
  31. 05/12 bloom: de-duplicate directory entriesDerrick Stolee via GitGitGadget, May 11, 2020
  32. SZEDER GáborJun 7, 2020
  33. 07/12 completion: offer '--(no-)patch' among 'git log' optionsSZEDER Gábor via GitGitGadget, May 11, 2020
  34. 08/12 line-log: remove unused fields from 'struct line_log_data'SZEDER Gábor via GitGitGadget, May 11, 2020
  35. 09/12 t4211-line-log: add tests for parent oidsSZEDER Gábor via GitGitGadget, May 11, 2020
  36. 10/12 line-log: more responsive, incremental 'git log -L'SZEDER Gábor via GitGitGadget, May 11, 2020
  37. 12/12 line-log: integrate with changed-path Bloom filtersDerrick Stolee via GitGitGadget, May 11, 2020
  38. 11/12 line-log: try to use generation number-based topo-orderingSZEDER Gábor via GitGitGadget, May 11, 2020
  39. 06/12 bloom: use num_changes not nr for limit detectionDerrick Stolee via GitGitGadget, May 11, 2020
  40. SZEDER GáborAug 4, 2020
  41. 01/12 bloom: fix whitespace around tab lengthDerrick Stolee via GitGitGadget, May 11, 2020
  42. 04/12 Documentation: changed-path Bloom filters use byte wordsDerrick Stolee via GitGitGadget, May 11, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.