Skip to content

Consolidate reachability logic

Derrick Stolee via GitGitGadget git 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest16 patches 60 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/16commit-reach: move walk methods from commit.cMakefile commit-reach.c commit-reach.h +1 more+401 -3582 replies
2/16commit-reach: move ref_newer from remote.cbuiltin/remote.c commit-reach.c commit-reach.h +3 more+59 -501 reply
3/16commit-reach: move commit_contains from ref-filtercommit-reach.c commit-reach.h fast-import.c +1 more+146 -1401 reply
4/16upload-pack: make reachable() more genericupload-pack.c+8 -80 replies
5/16upload-pack: refactor ok_to_give_up()upload-pack.c+22 -110 replies
6/16upload-pack: generalize commit date cutoffupload-pack.c+11 -62 replies
7/16commit-reach: move can_all_from_reach_with_flagscommit-reach.c commit-reach.h upload-pack.c+76 -681 reply
8/16test-reach: create new test tool for ref_newerMakefile t/helper/test-reach.c t/helper/test-tool.c +2 more+151 -02 replies
9/16test-reach: test in_merge_basest/helper/test-reach.c t/t6600-test-reach.sh+24 -00 replies
10/16test-reach: test is_descendant_oft/helper/test-reach.c t/t6600-test-reach.sh+30 -00 replies
11/16test-reach: test get_merge_bases_manyt/helper/test-reach.c t/t6600-test-reach.sh+30 -02 replies
12/16test-reach: test reduce_headst/helper/test-reach.c t/t6600-test-reach.sh+29 -02 replies
13/16test-reach: test can_all_from_reach_with_flagscommit-reach.c commit-reach.h t/helper/test-reach.c +1 more+102 -23 replies
14/16commit-reach: replace ref_newer logiccommit-reach.c+3 -231 reply
15/16commit-reach: make can_all_from_reach... linearcommit-reach.c commit-reach.h upload-pack.c+85 -5024 replies
16/16commit-reach: use can_all_from_reachcommit-graph.c commit-graph.h commit-reach.c+41 -71 reply

Reviews trailers per patch

1 of 16 patches has a Reviewed-by or Acked-by.

PatchDerrick Stolee
1/16
2/16
3/16
4/16
5/16
6/16
7/16
8/16
9/16
10/16
11/16
12/16
13/16
14/16
15/16R-b
16/16

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v1 thread

Cover letter [PATCH 00/16] Consolidate reachability logic

There are many places in Git that use a commit walk to determine reachability between commits and/or refs. A lot of this logic is duplicated.

I wanted to achieve the following:

1. Consolidate several different commit walks into one file
2. Reduce duplicate reachability logic
3. Increase testability (correctness and performance)
4. Improve performance of reachability queries

My approach is mostly in three parts:

I. Move code to a new commit-reach.c file.
II. Add a 'test-tool reach' command to test these methods directly.
III. Modify the logic by improving performance and calling methods with
similar logic but different prototypes.

The 'test-tool reach' command is helpful to make sure I don't break anything as I change the logic, but also so I can test methods that are normally only exposed by other more complicated commands. For instance, ref_newer() is part of 'git push -f' and ok_to_give_up() is buried deep within fetch negotiation. Both of these methods have some problematic performance issues that are corrected by this series. As I discovered them, it was clear that it would be better to consolidate walk logic instead of discovering a new walk in another file hidden somewhere.

For the ok_to_give_up() method, I refactored the method so I could pull the logic out of the depths of fetch negotiation. In the commit "commit-reach: make can_all_from_reach... linear" I discuss how the existing algorithm is quadratic and how we can make it linear. Also, we can use heuristic knowledge about the shape of the commit graph and the usual haves/wants to get some extra performance bonus. (The heuristic is to do a DFS with first-parents first, and stop on first found result. We expect haves/wants to include ref tips, which typically have their previous values in their first-parent history.)

One major difference in this series versus the RFC is that I added a new method 'generation_numbers_enabled()' to detect if we have a commit-graph file with non-zero generation numbers. Using can_all_from_reach in is_descendant_of is only faster if we have generation numbers as a cutoff.

Thanks,
-Stolee

This series is based on jt/commit-graph-per-object-store

Derrick Stolee (16):
commit-reach: move walk methods from commit.c
commit-reach: move ref_newer from remote.c
commit-reach: move commit_contains from ref-filter
upload-pack: make reachable() more generic
upload-pack: refactor ok_to_give_up()
upload-pack: generalize commit date cutoff
commit-reach: move can_all_from_reach_with_flags
test-reach: create new test tool for ref_newer
test-reach: test in_merge_bases
test-reach: test is_descendant_of
test-reach: test get_merge_bases_many
test-reach: test reduce_heads
test-reach: test can_all_from_reach_with_flags
commit-reach: replace ref_newer logic
commit-reach: make can_all_from_reach... linear
commit-reach: use can_all_from_reach

 Makefile              |   2 +
 builtin/remote.c      |   1 +
 commit-graph.c        |  18 ++
 commit-graph.h        |   6 +
 commit-reach.c        | 662 ++++++++++++++++++++++++++++++++++++++++++
 commit-reach.h        |  76 +++++
 commit.c              | 358 -----------------------
 fast-import.c         |   1 +
 http-push.c           |   1 +
 ref-filter.c          | 147 +---------
 remote.c              |  50 +---
 remote.h              |   1 -
 t/helper/test-reach.c | 104 +++++++
 t/helper/test-tool.c  |   1 +
 t/helper/test-tool.h  |   1 +
 t/t6600-test-reach.sh | 208 +++++++++++++
 upload-pack.c         |  58 +---
 17 files changed, 1095 insertions(+), 600 deletions(-)
 create mode 100644 commit-reach.c
 create mode 100644 commit-reach.h
 create mode 100644 t/helper/test-reach.c
 create mode 100755 t/t6600-test-reach.sh

base-commit: 596e28576ef3ca69432dbe5953b7bdcd18a32876
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-10%2Fderrickstolee%2Freach%2Frefactor-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-10/derrickstolee/reach/refactor-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/10

Signature · 1 line
gitgitgadget