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

[PATCH] revision: use generation for A..B --topo-order queries

From
Derrick Stolee <stolee@gmail.com>
Date
May 21, 2019, 13:14 UTC
Message-ID
<20190521131438.58394-1-dstolee@microsoft.com>
In-Reply-To
<f14799c3-e343-eb41-3536-65de7e38fbd9@gmail.com>

If a commit-graph exists with computed generation numbers, then a 'git rev-list --topo-order -n <N> <rev>' query will use those generation numbers to reduce the number of commits walked before writing N commits.

One caveat put in b454241 (revision.c: generation-based topo-order algorithm, 2018-11-01) was to not enable the new algorithm for queries with a revision range "A..B". The logic was placed to walk from "A" and mark those commits as uninteresting, but the performance was actually worse than the existing logic in some cases.

The root cause of this performance degradation is that generation numbers _increase_ the number of commits we walk relative to the existing heuristic of walking by commit date. While generation numbers actually guarantee that the algorithm is correct, the existing logic is very rarely wrong and that added requirement is not worth the cost.

This motivates the planned "corrected commit date" to replace generation numbers in a future version of Git.

The current change enables the logic to use whatever reachability index is currently in the commit-graph (generation numbers or corrected commit date).

The limited flag in struct rev_info forces a full walk of the commit history (after discovering the A..B range). Previosuly, it is enabled whenever we see an uninteresting commit. We prevent enabling the parameter when we are planning to use the reachability index for a topo-order.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Mike,

If you have the chance, then please apply this patch (on v2.22.0-rc1) and re-run your test. This will confirm if my thoughts on this matter are correct.

Thanks, -Stolee

 revision.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index d4aaf0ef25..be6ccf5786 100644
--- a/revision.c
+++ b/revision.c
@@ -436,7 +436,9 @@ static struct commit *handle_commit(struct rev_info *revs,
 			die("unable to parse commit %s", name);
 		if (flags & UNINTERESTING) {
 			mark_parents_uninteresting(commit);
-			revs->limited = 1;
+
+			if (!revs->topo_order || !generation_numbers_enabled(the_repository))
+				revs->limited = 1;
 		}
 		if (revs->sources) {
 			char **slot = revision_sources_at(revs->sources, commit);
-- 
2.22.0.rc1
Previous: Derrick StoleeNext: Derrick Stolee
Message 20 of 23 in “Revision walking, commit dates, slop”
  1. Mike HommeyMay 18, 2019
  2. SZEDER GáborMay 18, 2019
  3. Mike HommeyMay 18, 2019
  4. Mike HommeyMay 18, 2019
  5. SZEDER GáborMay 18, 2019
  6. Jakub NarebskiMay 19, 2019
  7. Derrick StoleeMay 20, 2019
  8. Jakub NarebskiMay 20, 2019
  9. Derrick StoleeMay 20, 2019
  10. Jakub NarebskiMay 20, 2019
  11. Jakub NarebskiMay 20, 2019
  12. Derrick StoleeMay 21, 2019
  13. Jakub NarebskiMay 22, 2019
  14. Derrick StoleeMay 22, 2019
  15. Jakub NarebskiMay 23, 2019
  16. Jakub NarebskiJun 25, 2019
  17. Derrick StoleeJun 25, 2019
  18. commit-graph: generation v5 (backward compatible date ceiling)Jakub Narebski, Sep 18, 2019
  19. Derrick StoleeSep 18, 2019
  20. revision: use generation for A..B --topo-order queriesDerrick Stolee, May 21, 2019
  21. 2/2 revision: keep topo-walk free of unintersting commitsDerrick Stolee, May 21, 2019
  22. Mike HommeyMay 22, 2019
  23. Jonathan NiederMay 21, 2019

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.