On 3/6/2023 1:26 PM, Junio C Hamano wrote: > "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> These numbers can be computed by 'git rev-list --count B..C' and 'git >> rev-list --count C..B', but there are common needs that benefit from having >> the checks being done in the same process: > > This makes readers wonder if "git rev-list --count B...C" should be > the end-user facing UI for this new feature, perhaps? > > Of course if you are checking how C0, C1, C2,... relate to a single > B, the existing rev-list syntax would not work, and makes a totally > new subcommand a possibilty. > >> 2. When a branch is updated, a background job checks if any pull requests >> that target that branch should be closed because their branches were >> merged implicitly by that update. These queries can e batched into 'git >> ahead-behind' calls. >> >> In that second example, we don't need the full ahead/behind counts (although >> it is sufficient to look for branches that are "zero commits ahead", meaning >> they are reachable from the base), so this builtin has an extra '--contains' >> mode that only checks reachability from the base to each of the tips. 'git >> ahead-behind --contains' is sort of the reverse of 'git branch --contains'. > > I thought that the reverse of "git branch --contains" was "git > branch --merged". "git branch --merged maint ??/\*" is how I cull > topic branches that have already served their purpose. > > Isn't closing pull requests because they have been already merged > the same idea? "git for-each-ref --merged main refs/pull/\*" or > something, perhaps? You are definitely on to something, and I was not aware of --merged as an option to either of these. 'git branch --merged' has some limitations that tags cannot be used. 'git for-each-ref --merged' is probably sufficient. The only difference being that it would be nice to specify the matching refs over stdin with --stdin to avoid long argument lists. With this in mind, I can update the performance test to look like this (after updating the setup step to add branches for each line in 'refs') test_perf 'batch reachability: git ahead-behind --contains' ' git ahead-behind --contains --base=HEAD --stdin <refs ' test_perf 'batch reachability: git branch --merged' ' xargs git branch --merged=HEAD <branches ' test_perf 'batch reachability: git for-each-ref --merged' ' xargs git for-each-ref --merged=HEAD <refs ' And get decent results on all cases with the Linux kernel repository: Test this tree ------------------------------------------------------------------------- 1500.2: ahead-behind counts: git ahead-behind 0.26(0.24+0.01) 1500.3: ahead-behind counts: git rev-list 4.46(3.91+0.54) 1500.4: batch reachability: git ahead-behind --contains 0.02(0.01+0.01) 1500.5: batch reachability: git branch --merged 0.14(0.13+0.00) 1500.6: batch reachability: git for-each-ref --merged 0.14(0.13+0.00) So, there is benefit in using this tips_reachable_from_base() method in the two existing 'git (branch|for-each-ref) --merged' computations. The API boundary selected in this series might not be the most appropriate for those builtins, so let's kick out patch 8 from this series for now and I'll revisit it separately. Thanks, -Stolee
