Re: [PATCH] index-pack: speed up promisor link recording

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=E2=80=AFPM brian m. carlson
<[email protected]> wrote:
>
> On 2026-08-02 at 21:33:15, Arijit Banerjee via GitGitGadget wrote:
> > From: Arijit Banerjee <[email protected]>
> >
> > When indexing a promisor pack, index-pack parses every reconstructed
> > non-blob object into the shared object model to record its outgoing lin=
ks.
> > Since parse_object_buffer() runs under read_mutex, worker threads seria=
lize
> > while allocating persistent tree, commit, and tag structures that are o=
nly
> > 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 dur=
ing
> > normal promisor indexing, and merge them after the workers exit. Transf=
er
> > 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 po=
licy
> > 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=3Dblob:none clone of linux.git decreased from 156 second=
s to
> > 133 seconds (15%). Trace2 attributed the change to the initial index-pa=
ck
> > --promisor phase, whose median duration decreased from 121 seconds to 9=
8
> > seconds (19%). System CPU time decreased by 46%.
> >
> > Two paired spot checks against GitHub showed end-to-end reductions of 1=
8%
> > and 26%. These measurements include network and server variability and =
are
> > therefore corroborating rather than controlled results. A third pair wa=
s not
> > interpretable because the baseline request encountered a transport stal=
l.
> >
> > A full-clone control showed no material change, taking approximately 25=
6
> > seconds with either version. This is expected because full clones do no=
t
> > 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 cove=
rage
> > checks malformed commit headers, conflicting link types, and mismatched=
 tag
> > target types.
> >
> > Signed-off-by: Arijit Banerjee <[email protected]>
> > ---
> >     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 certif=
y
>     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=E2=80=99t 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