index-pack: speed up promisor link recording - #2191
Open
arijit91 wants to merge 1 commit into
Open
Conversation
When indexing a promisor pack, index-pack parses every reconstructed non-blob object into the shared object model to record its outgoing links. Since parse_object_buffer() runs under read_mutex, worker threads serialize while allocating persistent tree, commit, and tag structures that are only needed to enumerate those links. Read the links directly from the reconstructed object buffers instead. Keep the strict and fsck paths unchanged, use worker-local typed oidmaps during normal promisor indexing, and merge them after the workers exit. Transfer entries during the merge so that it does not temporarily duplicate the complete link set. The typed entries preserve checks previously performed as a side effect of object parsing. Reject malformed commit and tag headers, conflicting expected types, and targets whose actual type disagrees when the target is present in the pack. Preserve commit-graft handling and the existing policy of recording only subtree entries from trees. With three runs per version on Debian 12, median end-to-end wall-clock time for a --filter=blob:none clone of linux.git decreased from 156 seconds to 133 seconds (15%). Trace2 attributed the change to the initial index-pack --promisor phase, whose median duration decreased from 121 seconds to 98 seconds (19%). System CPU time decreased by 46%. Two paired spot checks against GitHub showed end-to-end reductions of 18% and 26%. These measurements include network and server variability and are therefore corroborating rather than controlled results. A third pair was not interpretable because the baseline request encountered a transport stall. A full-clone control showed no material change, taking approximately 256 seconds with either version. This is expected because full clones do not exercise promisor-link recording. t5302-pack-index.sh passed with both SHA-1 and SHA-256, while t0410-partial-clone.sh and t5616-partial-clone.sh also passed. New coverage checks malformed commit headers, conflicting link types, and mismatched tag target types. Signed-off-by: Arijit Banerjee <arijit@effectiveailabs.com>
Author
|
/submit |
|
Submitted as pull.2191.git.1785706396130.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
"brian m. carlson" wrote on the Git mailing list (how to reply to this email): On 2026-08-02 at 21:33:15, Arijit Banerjee via GitGitGadget wrote:
> From: Arijit Banerjee <arijit@effectiveailabs.com>
>
> When indexing a promisor pack, index-pack parses every reconstructed
> non-blob object into the shared object model to record its outgoing links.
> Since parse_object_buffer() runs under read_mutex, worker threads serialize
> while allocating persistent tree, commit, and tag structures that are only
> needed to enumerate those links.
>
> Read the links directly from the reconstructed object buffers instead. Keep
> the strict and fsck paths unchanged, use worker-local typed oidmaps during
> normal promisor indexing, and merge them after the workers exit. Transfer
> entries during the merge so that it does not temporarily duplicate the
> complete link set.
>
> The typed entries preserve checks previously performed as a side effect of
> object parsing. Reject malformed commit and tag headers, conflicting
> expected types, and targets whose actual type disagrees when the target is
> present in the pack. Preserve commit-graft handling and the existing policy
> of recording only subtree entries from trees.
>
> With three runs per version on Debian 12, median end-to-end wall-clock time
> for a --filter=blob:none clone of linux.git decreased from 156 seconds to
> 133 seconds (15%). Trace2 attributed the change to the initial index-pack
> --promisor phase, whose median duration decreased from 121 seconds to 98
> seconds (19%). System CPU time decreased by 46%.
>
> Two paired spot checks against GitHub showed end-to-end reductions of 18%
> and 26%. These measurements include network and server variability and are
> therefore corroborating rather than controlled results. A third pair was not
> interpretable because the baseline request encountered a transport stall.
>
> A full-clone control showed no material change, taking approximately 256
> seconds with either version. This is expected because full clones do not
> exercise promisor-link recording.
>
> t5302-pack-index.sh passed with both SHA-1 and SHA-256, while
> t0410-partial-clone.sh and t5616-partial-clone.sh also passed. New coverage
> checks malformed commit headers, conflicting link types, and mismatched tag
> target types.
>
> Signed-off-by: Arijit Banerjee <arijit@effectiveailabs.com>
> ---
> index-pack: speed up promisor link recording
>
> AI assistance: OpenAI Codex was used to identify the bottleneck and
> assist with the implementation, testing, and benchmark analysis. I
> reviewed the resulting change and take responsibility for this
> submission.
I don't think SubmittingPatches really allows more than trivial changes
written by AI:
The Developer's Certificate of Origin requires contributors to certify
that they know the origin of their contributions to the project and
that they have the right to submit it under the project's license.
It's not yet clear that this can be legally satisfied when submitting
significant amount of content that has been generated by AI tools.
[...]
To avoid these issues, we will reject anything that looks AI
generated, that sounds overly formal or bloated, that looks like AI
slop, that looks good on the surface but makes no sense, or that
senders don’t understand or cannot explain.
This doesn't look like it's a trivial change, so I don't believe this
patch can be accepted.
--
brian m. carlson (they/them)
Toronto, Ontario, CA |
|
User |
|
Arijit Banerjee wrote on the Git mailing list (how to reply to this email): On Sun, Aug 2, 2026, brian m. carlson wrote:
> This doesn't look like it's a trivial change, so I don't believe this
> patch can be accepted.
Thanks, Brian. I am not trying to bypass the project's policy.
I do not claim to be an expert on this topic, but Codex appears to have
found a material performance improvement of about 15% on end-to-end
blobless clone times. Would it be appropriate to treat the current
submission as an RFC for maintainers before deciding if the optimization is
worth getting in? It seems worth trying to preserve the technical
result.
Thanks,
Arijit
On Sun, Aug 2, 2026 at 2:52 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2026-08-02 at 21:33:15, Arijit Banerjee via GitGitGadget wrote:
> > From: Arijit Banerjee <arijit@effectiveailabs.com>
> >
> > When indexing a promisor pack, index-pack parses every reconstructed
> > non-blob object into the shared object model to record its outgoing links.
> > Since parse_object_buffer() runs under read_mutex, worker threads serialize
> > while allocating persistent tree, commit, and tag structures that are only
> > needed to enumerate those links.
> >
> > Read the links directly from the reconstructed object buffers instead. Keep
> > the strict and fsck paths unchanged, use worker-local typed oidmaps during
> > normal promisor indexing, and merge them after the workers exit. Transfer
> > entries during the merge so that it does not temporarily duplicate the
> > complete link set.
> >
> > The typed entries preserve checks previously performed as a side effect of
> > object parsing. Reject malformed commit and tag headers, conflicting
> > expected types, and targets whose actual type disagrees when the target is
> > present in the pack. Preserve commit-graft handling and the existing policy
> > of recording only subtree entries from trees.
> >
> > With three runs per version on Debian 12, median end-to-end wall-clock time
> > for a --filter=blob:none clone of linux.git decreased from 156 seconds to
> > 133 seconds (15%). Trace2 attributed the change to the initial index-pack
> > --promisor phase, whose median duration decreased from 121 seconds to 98
> > seconds (19%). System CPU time decreased by 46%.
> >
> > Two paired spot checks against GitHub showed end-to-end reductions of 18%
> > and 26%. These measurements include network and server variability and are
> > therefore corroborating rather than controlled results. A third pair was not
> > interpretable because the baseline request encountered a transport stall.
> >
> > A full-clone control showed no material change, taking approximately 256
> > seconds with either version. This is expected because full clones do not
> > exercise promisor-link recording.
> >
> > t5302-pack-index.sh passed with both SHA-1 and SHA-256, while
> > t0410-partial-clone.sh and t5616-partial-clone.sh also passed. New coverage
> > checks malformed commit headers, conflicting link types, and mismatched tag
> > target types.
> >
> > Signed-off-by: Arijit Banerjee <arijit@effectiveailabs.com>
> > ---
> > index-pack: speed up promisor link recording
> >
> > AI assistance: OpenAI Codex was used to identify the bottleneck and
> > assist with the implementation, testing, and benchmark analysis. I
> > reviewed the resulting change and take responsibility for this
> > submission.
>
> I don't think SubmittingPatches really allows more than trivial changes
> written by AI:
>
> The Developer's Certificate of Origin requires contributors to certify
> that they know the origin of their contributions to the project and
> that they have the right to submit it under the project's license.
> It's not yet clear that this can be legally satisfied when submitting
> significant amount of content that has been generated by AI tools.
>
> [...]
>
> To avoid these issues, we will reject anything that looks AI
> generated, that sounds overly formal or bloated, that looks like AI
> slop, that looks good on the surface but makes no sense, or that
> senders don’t understand or cannot explain.
>
> This doesn't look like it's a trivial change, so I don't believe this
> patch can be accepted.
> --
> brian m. carlson (they/them)
> Toronto, Ontario, CA |
|
User |
|
"brian m. carlson" wrote on the Git mailing list (how to reply to this email): On 2026-08-02 at 22:12:16, Arijit Banerjee wrote:
> Thanks, Brian. I am not trying to bypass the project's policy.
>
> The investigation is in the same general spirit as the Git performance work
> being tracked here:
> https://openai-git-upstream.openai.chatgpt.site/
>
> I do not claim to be an expert on this topic, but Codex appears to have found
> a material performance improvement of about 15% on end-to-end blobless clone
> times.
>
> Would it be appropriate to treat the current submission as an RFC? It seems
> worth trying to preserve the technical result.
I don't think the project's policy prevents you from doing analysis and
investigation with an LLM, although it does require you to verify the
correctness of the results and be accountable for them. If, based on
the analysis of the performance impact, you write some code without the
use of an LLM that improves things, I think that would be allowed and
probably welcome, assuming it is otherwise acceptable. Some
contributors will be willing to review such a contribution and others
will not, but it is not outside of the policy.
However, writing substantial code with an LLM doesn't appear to be
allowed. The kinds of trivial changes that I think would be allowed to
be generated would be things like fixing spelling errors or adding
include guards to header files that lack them. Of course, these are
also the kinds of things you could mostly fix with a small script, which
is why they are generally considered so trivial as to be
uncopyrightable.
So I think to have a patch accepted in this case, you would need to
totally discard the existing patch and rewrite it by hand without
recourse to the generated code.
I understand that the SubmittingPatches documentation is a bit long, but
I do suggest giving it at least a glance so you know what to expect. I
think reading this sort of contributing documentation is more important
than ever since, in the era of LLMs, projects tend to have strong
opinions on what is and is not acceptable, not only just in terms of LLM
usage, but in how code and documentation are to be written and
formatted.
--
brian m. carlson (they/them)
Toronto, Ontario, CA |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> index-pack: speed up promisor link recording
>>
>> AI assistance: OpenAI Codex was used to identify the bottleneck and
>> assist with the implementation, testing, and benchmark analysis. I
>> reviewed the resulting change and take responsibility for this
>> submission.
>
> I don't think SubmittingPatches really allows more than trivial changes
> written by AI:
>
> The Developer's Certificate of Origin requires contributors to certify
> that they know the origin of their contributions to the project and
> that they have the right to submit it under the project's license.
> It's not yet clear that this can be legally satisfied when submitting
> significant amount of content that has been generated by AI tools.
>
> [...]
>
> To avoid these issues, we will reject anything that looks AI
> generated, that sounds overly formal or bloated, that looks like AI
> slop, that looks good on the surface but makes no sense, or that
> senders don’t understand or cannot explain.
>
> This doesn't look like it's a trivial change, so I don't believe this
> patch can be accepted.
The project we borrowed DCO from has this to say on this topic:
https://docs.kernel.org/process/coding-assistants.html
* All contributions must comply with licensing reuqirements.
* Only humans can attest DCO by Siging off their patches.
The human submitter is responsible for reviewing all AI generated
code, ensuring compliance with licensing requirements, certify
DCO with their sign-off, and taking full responsibility for the
contribution.
Now we are *not* kernel, but I think there is a general concensus in
the community that, while we do not want to outright ban machine
assisted contributions, we generally want to tread very carefully,
especially in the DCO area.
It is very easy for anybody and their dog to say "I reviewed X" and
it is very hard for others to assess how trustworthy such a
statement is, so I am unsure how the kernel project is enforcing the
"human submitter is responsible for these", and more importantly,
even assuming that we would take a similar policy for ourselves, I
am not sure what mechanism we can put in place to detect cases where
these expectations are violated.
So... |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> I understand that the SubmittingPatches documentation is a bit long, but
> I do suggest giving it at least a glance so you know what to expect. I
> think reading this sort of contributing documentation is more important
> than ever since, in the era of LLMs, projects tend to have strong
> opinions on what is and is not acceptable, not only just in terms of LLM
> usage, but in how code and documentation are to be written and
> formatted.
Amen.
Since we seem to be drawn into the AI policy discussion, are there
things that we should consider borrowing from policies battle-tested
by other projects? I kind of like what LLVM has as "extractive
contributions are rejected (whether it is AI or not AI)", as we are
severely review-bandwidth limited these days.
|
|
Arijit Banerjee wrote on the Git mailing list (how to reply to this email): Two suggestions:
- Maybe a karma system?
- A special release train that is more indulgent towards AI generated
code? Brave users can try out features and they get baked into stable
releases after enough soak time
Definitely not trying to be extractive, this one appears to be a
decent size perf improvement!
Still holding on to a SHA1-DC hardware acceleration change, that one
would admittedly be much harder to review ;)
On Sun, Aug 2, 2026 at 3:52 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > I understand that the SubmittingPatches documentation is a bit long, but
> > I do suggest giving it at least a glance so you know what to expect. I
> > think reading this sort of contributing documentation is more important
> > than ever since, in the era of LLMs, projects tend to have strong
> > opinions on what is and is not acceptable, not only just in terms of LLM
> > usage, but in how code and documentation are to be written and
> > formatted.
>
> Amen.
>
> Since we seem to be drawn into the AI policy discussion, are there
> things that we should consider borrowing from policies battle-tested
> by other projects? I kind of like what LLVM has as "extractive
> contributions are rejected (whether it is AI or not AI)", as we are
> severely review-bandwidth limited these days.
> |
|
"brian m. carlson" wrote on the Git mailing list (how to reply to this email): [please avoid top-posting]
On 2026-08-02 at 22:54:27, Arijit Banerjee wrote:
> Maybe software can ship experimental versions which is more indulging
> towards AI generated patches? Brave users get to try the features and they
> can baked into stable releases once there's enough soak time.
>
> I have been holding onto my patch around hardware acceleration for SHA1-DC
> :)
The rationale, as Junio said, is based on the Developer's Certificate of
Origin. That is a legal statement that a person has the legal right to
contribute those changes under the license and if they make a false or
misleading statement to that effect, they are responsible—legally and
otherwise—for it.
Considering the extensive litigation over LLM output at the moment, I
don't think anyone can clearly make that assertion. Most of the
arguments I've heard are that it's fair use, which is a U.S. legal
concept. That does not exist in Canada or the U.K., where there is fair
dealing, which is much more restricted.
If Company X includes LLM-generated code in their proprietary product
and it's found to be infringing in say, Germany, then they can simply
not distribute their code in Germany. Git cannot do that: it's
distributed in Linux distributions around the planet, even in countries
subject to sanctions, such as Russia[0]. We must comply with the
license and the law everywhere in every country or we risk liability for
our contributors and distributors. I, for one, am not willing to be
sued over this project and the project does not have the financial means
to deal with extensive litigation.
There are also concerns about the quality of the code and whether
submitters adequately understand the code well enough to have evaluated
and reviewed it thoroughly. It's well known that when creating code
becomes cheap, the burden shifts to review and review becomes extremely
important. That has been seen in lots of places, but we are an open
source project and we can't force contributors to do review like a
company can. As Junio says, we already have trouble getting reviews
through and we don't want to make the problem worse.
LLM-generated content also has a negative quality reputation (see the
reaction to AI content in video games and books for an example) and
while all software has bugs, I appreciate the reputation that Git has
for quality and wish to retain that.
Those alone are reason enough for the policy, but there are other
concerns about the environmental impact, the impact on electricity and
hardware prices, the ethics of incorporating open source code without so
much as a credit[1], and a lot more.
So I don't think it's likely we're going to accept nontrivial
LLM-generated content in any capacity anytime soon and I don't think
trying to argue this or persuade us to accept it is going to be
productive or well received. Of course, anyone can distribute their own
fork of Git with additional patches if they prefer, but we won't include
them.
[0] Debian, which distributes Git, has mirrors in Russia and Belarus:
https://www.debian.org/mirror/list
[1] For instance, as a member of ACM, I have to follow § 1.5 (Respect
the work required to produce new ideas, inventions, creative works, and
computing artifacts) of the Code of Ethics:
https://www.acm.org/code-of-ethics.
--
brian m. carlson (they/them)
Toronto, Ontario, CA |
|
Collin Funk wrote on the Git mailing list (how to reply to this email): "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> If Company X includes LLM-generated code in their proprietary product
> and it's found to be infringing in say, Germany, then they can simply
> not distribute their code in Germany. Git cannot do that: it's
> distributed in Linux distributions around the planet, even in countries
> subject to sanctions, such as Russia[0]. We must comply with the
> license and the law everywhere in every country or we risk liability for
> our contributors and distributors. I, for one, am not willing to be
> sued over this project and the project does not have the financial means
> to deal with extensive litigation.
I generally agree. But some countries have more respected legal systems
than others, to put it mildly. I certainly hope that being extradited to
one of the sanctioned countries isn't too large of a concern.
Collin |
|
User |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change speeds up the client-side
index-pack --promisorphase used bypartial clones. It avoids parsing reconstructed objects into Git's shared
object model solely to record their outgoing links, allowing worker threads
to collect typed links independently before merging them.
Performance
With three runs per version on Debian 12, median end-to-end wall-clock time
for a
--filter=blob:noneclone of linux.git decreased from 156 seconds to133 seconds (15%). Trace2 attributed the change to the initial
index-pack --promisorphase, whose median duration decreased from 121seconds to 98 seconds (19%). System CPU time decreased by 46%.
Two paired spot checks against GitHub showed end-to-end reductions of 18%
and 26%. These measurements include network and server variability and are
therefore corroborating rather than controlled results. A third pair was not
interpretable because the baseline request encountered a transport stall.
A full-clone control showed no material change, taking approximately 256
seconds with either version. This is expected because full clones do not
exercise promisor-link recording.
Testing
t5302-pack-index.shpassed with both SHA-1 and SHA-256, whilet0410-partial-clone.shandt5616-partial-clone.shalso passed. New testscover malformed commit headers, conflicting link types, and mismatched tag
target types.
AI Assistance
OpenAI Codex was used to identify the bottleneck and assist with the
implementation, testing, and benchmark analysis. I reviewed the resulting
change and take responsibility for this submission.
CC: Jonathan Tan jonathantanmy@fastmail.com, Patrick Steinhardt ps@pks.im, Junio C Hamano gitster@pobox.com
cc: "brian m. carlson" sandals@crustytoothpaste.net
cc: Arijit Banerjee arijit@effectiveailabs.com
cc: Collin Funk collin.funk1@gmail.com