Re: [PATCH v3 2/2] reftable: fix quadratic behavior in the presence of tombstones

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jul 10, 2026 at 05:03:28PM +0200, Kristofer Karlsson wrote:
> On Fri, 10 Jul 2026 at 16:32, Patrick Steinhardt <ps@pks.im> wrote:
> >
> > > This also requires adding deletion checks to the log iteration paths,
> > > since suppress_deletions applied to both ref and log iterators.
> >
> > Nit: s/applied/applies/
> 
> Language and using correct tense is always the tricky part --
> will fix if a reroll is needed for other reasons.
> 
> > > +     int suppress_deletions;
> >
> > A comment would've been nice, but I don't think this warrants a reroll.
> 
> Agreed, the field name felt self-documenting to me, but I will
> add a short comment if there is a reroll.
> Something like this?
> "boolean: filters out tombstoned/deleted refs early if true"

I'd drop the "boolean: " prefix, but other than that this looks sensible
to me.

> > > -     new_merged->suppress_deletions = 1;
> > > +     new_merged->suppress_deletions = st->opts.suppress_deletions;
> >
> > Yup, this looks good to me.
> 
> Thanks for the quick review.
> 
> Another thing I have been thinking about: should we consider
> suppress_deletions a temporary stopgap, with the goal of
> eventually removing it?

Maybe? I'll update libgit2 as soon as both ps/reftable-hardening and
kk/reftable-tombstone-quadratic-fix have been merged to "master". Once
done, feel free to create a pull request against libgit2 to deactivate
`suppress_deletions` there, and once that's happened we can also drop
the code in Git itself.

Thanks!

Patrick