Skip to content

Commit ebdb323

Browse files
committed
rn-136: add batch prefetching article
1 parent 540daeb commit ebdb323

1 file changed

Lines changed: 220 additions & 2 deletions

File tree

rev_news/drafts/edition-136.md

Lines changed: 220 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,227 @@ This edition covers what happened during the months of May and June 2026.
2121
### General
2222
-->
2323

24-
<!---
24+
2525
### Reviews
26-
-->
26+
27+
+ [[PATCH 0/3] Batch prefetching](https://lore.kernel.org/git/pull.2089.git.1776379694.gitgitgadget@gmail.com)
28+
29+
Elijah Newren sent a 3 patch series to improve the performance of a
30+
couple of commands in [partial clones](https://git-scm.com/docs/partial-clone).
31+
The work was spurred by a real-world report where `git cherry` jobs were each
32+
doing hundreds of single-blob fetches, at a cost of around 3 seconds
33+
each, so that batching those downloads should dramatically speed up
34+
such jobs. As Elijah put it, he "decided to fix up git grep
35+
similarly while at it". The series also corrected a small
36+
documentation typo he had noticed in `patch-ids.h` (a missing
37+
trailing parenthesis in a comment), as a preparatory fixup.
38+
39+
For readers unfamiliar with the trade-off, partial clones let users
40+
avoid downloading blobs upfront, at the expense of needing to
41+
download them later as they run other commands. That trade-off can
42+
sometimes be more painful than expected: when the needed blobs are
43+
discovered one at a time as they are accessed, each one triggers a
44+
separate network round-trip. Some commands like `checkout`, `diff`,
45+
and `merge` already mitigate this by doing batch prefetches of the
46+
blobs they will need, which dramatically reduces the cost of
47+
on-demand loading. The aim of this series was to extend that ability
48+
to two more commands, `git cherry` and `git grep`.
49+
50+
The interesting part for `git cherry` is how to figure out,
51+
*without* fetching anything yet, which blobs will eventually be
52+
needed. As Elijah explained, `git cherry` works in two phases: it
53+
first computes header-only patch IDs (based on file paths and
54+
modes), and only falls back to full content-based IDs when the
55+
header-only IDs collide. Those full IDs are what require reading
56+
blob content, and the comparison is driven by a hashmap whose
57+
comparison function, `patch_id_neq()`, is exactly what triggers the
58+
on-demand fetches. To enumerate the colliding blobs ahead of time,
59+
the patch temporarily swaps the hashmap's comparison function for a
60+
trivial `always_match()` function, walks the entries that would
61+
collide to collect their blob OIDs into an `oidset`, restores the
62+
original comparison function, and then fetches everything in a
63+
single batch via `promisor_remote_get_direct()`. A helper,
64+
`collect_diff_blob_oids()`, lists the blob OIDs touched by a
65+
commit's diff. It leaves out files that are explicitly marked as
66+
binary in the userdiff configuration, because for those files
67+
patch-ID just hashes the OID with `oid_to_hex()` instead of
68+
reading the blob, so there is no point downloading it.
69+
70+
While git cherry relies on hashmap comparisons, the `git grep` patch
71+
takes an analogous but simpler approach: it adds a preliminary walk
72+
over the tree (similar to `grep_tree()`) that collects the blobs of
73+
interest and prefetches them in one go.
74+
75+
Junio Hamano, the Git maintainer, took a first look and immediately
76+
spotted something that did not belong: the series added a 210-line
77+
`investigations/cherry-prefetch-design-spec.md` file to the
78+
project. He pointed out that, as a document describing how
79+
`git cherry` works, "it is vastly lacking", that much of its content
80+
is the sort of material that would normally go in a commit message,
81+
and that he was "not sure how others would benefit from being able
82+
to read it" once the series landed. Elijah's reply was short and to
83+
the point: "Ugh, no, sorry." That stray file had been committed by
84+
mistake.
85+
86+
Elijah quickly sent [version 2](https://lore.kernel.org/git/pull.2089.v2.git.1776472347.gitgitgadget@gmail.com),
87+
whose only change compared to v1 was to remove that stray file,
88+
noting it was "So embarrassing that I didn't catch that before
89+
submitting."
90+
91+
Phillip Wood reviewed v2 and made an interesting connection:
92+
`git rebase` without `--reapply-cherry-picks` suffers from the same
93+
problem, since it does the equivalent of `git log --cherry-pick`. He
94+
asked whether `prefetch_cherry_blobs()` could be shared with the
95+
cherry-pick detection in `revision.c`. Elijah agreed the connection
96+
was correct, explaining that `git rebase` (without
97+
`--reapply-cherry-picks`) and `git log --cherry-pick` both go
98+
through `cherry_pick_list()` in `revision.c`, which has the same
99+
shape as the loop in `cmd_cherry()` and triggers fetches from the
100+
same `patch_id_neq()` callback. He even sketched what sharing the
101+
code would look like.
102+
103+
However, he preferred to leave that out of the current series,
104+
expressing reservations about expanding partial-clone support
105+
further into this area: `git cherry`, `git log --cherry-pick`, and
106+
the default cherry-pick detection in `git rebase` all exist to
107+
answer "has this patch already landed upstream?", a question that,
108+
in repositories large enough to need partial clones, he felt "is
109+
rarely worth the cost of computing patch-ids across arbitrary
110+
amounts of history." His honest guidance for users on a large
111+
repository would be to pass `--reapply-cherry-picks` (with rebase)
112+
and skip the detection entirely, or to narrow the range under
113+
consideration. He noted that the omission of a
114+
`--no-reapply-cherry-picks` option in `git replay` had been a
115+
deliberate choice rather than an oversight. He had only implemented
116+
the `git cherry` fix because of a specific customer whose tooling
117+
had already baked in the operation, and prefetching at least made
118+
the worst case tolerable. He added that he would happily review a
119+
patch from anyone wanting to carry the shared code forward.
120+
121+
Phillip continued the exchange with several good questions, asking
122+
whether patch IDs are computed for every upstream commit or just the
123+
ones modifying the same paths, and remarking that it "is a shame
124+
that we don't have a config setting for `--reapply-cherry-picks` as
125+
it is easy to forget to pass that option" (a setting made awkward
126+
because the apply backend does not support that option). He was also
127+
"a bit surprised customers aren't complaining about tools that use
128+
`git rebase` being slow."
129+
130+
Elijah replied that determining which upstream commits modify the
131+
same paths still requires walking the upstream commits and doing a
132+
tree-diff for each, and that in the biggest repositories "even a
133+
merge-base operation can start to feel expensive." On the surprise
134+
about rebase, he answered "Are you sure they aren't complaining?",
135+
explaining that the merging parts of a rebase already do batch
136+
prefetching, but the cherry-pick-detection part does not. He also
137+
noted that the customer in question was using `git replay` rather
138+
than `git rebase`, probably because early versions of `git replay`
139+
lacked the drop-commits-that-become-empty logic that Phillip later
140+
added (he thanked Phillip again for that), and that the prefetch
141+
patch lets things stay fast even if they keep their `git cherry`
142+
calls.
143+
144+
Derrick Stolee then reviewed v2, reading both the `git cherry` and
145+
`git grep` patches together. He worried that
146+
`collect_diff_blob_oids()` being "hidden in builtin/log.c may not be
147+
the right long-term home", anticipating more and more cases where
148+
Git would want to prefetch blobs, and wondered whether the logic
149+
could take advantage of, or live alongside, the existing
150+
`diff_queued_diff_prefetch()` within `diffcore_std()` in
151+
`diff.c`. He framed the `git cherry` patch as caring about a diff
152+
and the `git grep` patch as caring about a "scan prep", suggesting
153+
`git archive` as a closer analog for the latter than `checkout`. He
154+
was careful to add that he did not mean to complicate the series and
155+
was "most interested in having this logic be more reusable in the
156+
future without needing to move code across files."
157+
158+
Junio, seeing that Stolee's two review messages had gone unanswered
159+
for a while, asked whether he should keep the patches in his tree
160+
"hoping that responses may come some day", and said he would mark
161+
the topic as expecting review responses in the draft "What's
162+
cooking" report for the time being. Elijah apologized for the delay,
163+
explaining he had been pulled into firefighting and remediation
164+
duties after a number of incidents at work, and suggested marking
165+
the series as expecting a re-roll since Stolee had asked for an
166+
additional test.
167+
168+
Elijah then answered Stolee's reusability question in detail. He
169+
read the patch differently: `collect_diff_blob_oids()` already leans
170+
on the diff library at the per-commit level (`diff_tree_oid()` plus
171+
`diffcore_std()`), and the real value of the series lives *above*
172+
the diff library, in the accumulation across many commits.
173+
174+
Concretely, the motivating case was a patch touching a few files
175+
where upstream had tens of thousands of commits in the relevant
176+
range, several hundred of which modified the same set of files: a
177+
per-diff prefetch like `diff.c` uses would turn that into hundreds
178+
of small fetches, "what this series gives you is one fetch." He
179+
pointed out two further `git cherry`-specific filters that he felt
180+
did not belong in the diff library: most commits are skipped before
181+
patch-ID is even computed (so prefetching for them would be wasted),
182+
and content for binary files is skipped because patch-ID uses
183+
`oid_to_hex()` for them. To check Stolee's idea concretely, he
184+
reviewed all of the existing `promisor_remote_get_direct()` call
185+
sites and concluded that none of them shared the "diff two trees and
186+
harvest OIDs" shape, so there was no natural shared layer above the
187+
`promisor_remote_get_direct()` primitive itself. He agreed
188+
`git archive` would be the closest analog if it ever grew prefetch
189+
logic, and proposed factoring out a tree-walk helper only when a
190+
second caller actually wanted one.
191+
192+
For the `git grep` patch, Stolee asked for a test that exercises a
193+
pathspec filter, with files like `matches.txt`, `nomatch.txt`, and
194+
`matches.md`, so that `git grep -c "needle" HEAD -- *.txt` would
195+
download only the matching subset. This turned out to be more
196+
valuable than a simple test improvement: Elijah replied "Yes,
197+
absolutely", and discovered that while he was handling pathspecs
198+
correctly, he was unconditionally requesting whatever objects
199+
matched the pathspecs even when those blobs were already present
200+
locally. He promised to send a fix along with the updated test.
201+
202+
That fix arrived in [version 3](https://lore.kernel.org/git/pull.2089.v3.git.1778775928.gitgitgadget@gmail.com),
203+
which made three changes compared to v2:
204+
205+
- the final patch's test case was updated, as Stolee had suggested,
206+
to exercise a pathspec,
207+
208+
- the last two patches were modified to avoid re-downloading blobs
209+
already present locally (checking with
210+
`odb_read_object_info_extended()` and `OBJECT_INFO_FOR_PREFETCH`
211+
on the `git cherry` side, and `odb_has_object()` on the `git grep`
212+
side), with the tests adjusted to verify it, and
213+
214+
- a new first patch was inserted documenting the filtering contract
215+
of `promisor_remote_get_direct()`.
216+
217+
That documentation patch explains that the function does not filter
218+
out OIDs already present locally on its happy path, so callers are
219+
responsible for filtering and deduplicating themselves. Elijah
220+
candidly noted in the commit message that he "missed this originally
221+
and wrote two problematic callers". He also mentioned that he had
222+
not pursued Stolee's code-sharing suggestion, since it appeared to
223+
be based on a misunderstanding that the `git cherry` patch was about
224+
a diff.
225+
226+
Stolee reviewed v3 and declared it "good to go", graciously adding
227+
that Elijah's detailed responses in the v2 thread "helped me
228+
understand that my thought was misguided" and gave him "extra
229+
confidence" in the approach. Junio agreed the series was "in a good
230+
shape" and marked the topic for the `next` branch. Elijah thanked
231+
Stolee one more time, noting that the comments on the `git grep`
232+
patch in particular "led me to what would have been a rather
233+
annoying bug", so calling out the test improvement had been time
234+
well spent.
235+
236+
In the end, the series was merged into the `master` branch and is part
237+
of the recent v2.55.0 release. A concrete customer pain point led to
238+
extending Git's existing batch-prefetching habit to two more
239+
commands, `git cherry` and `git grep`, as well as a bug fix and
240+
improved documentation. The thread also clarified the boundaries of
241+
partial-clone friendliness for cherry-pick detection, leaving the
242+
door open for sharing the new code with `git rebase` and
243+
`git log --cherry-pick` should someone wish to carry that work
244+
forward.
27245

28246
<!---
29247
### Support

0 commit comments

Comments
 (0)