Re: [PATCH] odb/files: be less aggressive with geometric repacking

On 26/08/12 07:44AM, Patrick Steinhardt wrote:
> On Tue, Aug 11, 2026 at 03:44:12PM -0500, Justin Tobler wrote:
> > On 26/08/11 11:04AM, Patrick Steinhardt wrote:
> > > When performing auto-maintenance with geometric repacking we have two
> > > conditions that may trigger a repack:
> > > 
> > >   - Either the geometric sequence of packfiles is invalidated.
> > > 
> > >   - Or we have too many loose objects.
> > > 
> > > The first condition shouldn't trigger all that often: it may be hit when
> > > we fetch a new packfile, but users tend to not do that all the time. The
> > > second condition is what typically triggers more regularly though, as
> > > every command that ends up writing new objects may cause us to cross the
> > > threshold of loose objects. It is thus preferable to not be too
> > > aggressive here, as otherwise we may end up repacking objects quite
> > > often.
> > > 
> > > For the geometric-repacking strategy though we have a default of 100
> > > objects, only. As we're approximating the count of objects by only
> > > reading the "objects/17/" shared, we'd only need 2 objects in there
> > > before we perform a repack by default, which is quite aggressive.
> > > git-gc(1) on the other hand has a default of 6700, so it is quite a bit
> > > more conservative here.
> > 
> > Ok IIUC, the reason two loose objects can potentially trigger repacking
> > is because the heuristic used to estimate the number of loose objects
> > only counts objects in "objects/17/" and multiples it by 256 (the
> > maximum number of directories that are fanned-out). That makes sense and
> > indeed seems like it could lead to repacking processes be spawned more
> > frequently than desired.
> > 
> > My first thought is whether the heuristic itself should be updated to
> > capture a more accurate estimate for the number of objects. That would
> > of course require looking up more objects and thus be more expensive. If
> > the goal here is just for a very rough estimate anyways, maybe it
> > wouldn't be worth it though.
> 
> That wouldn't really solve the problem though. The problem is not really
> that the estimation can be wrong, it's rather that even if it was always
> correct we're still being too aggressive with packing the loose objects.
> Because ultimately, a 100 objects is a comparatively small threshold,
> and leads to 67 times more repacking compared to git-gc(1).

Ok, that makes sense.

> > Increasing the loose object threshold here to be more conservative seems
> > like a reasonable approach. I'm not sure exactly why 6700 was chosen
> > here. 6700 / 256 ~= 26.2 which means "objects/17/" would have to contain
> > at least 27 objects before repacking is triggered. That is certainly
> > much more conservative. I see that 6700 has also been chosen else where
> > in the codebase as the threshold too. It might be nice to explain the
> > reasoning a bit more in the commit message though.
> 
> Hmm, don't I already do that? In the paragraph you're responding to I'm
> saying that git-gc(1) already had that default forever, so I'm adjusting
> our heuristic to match that.

I think I was just curious as to why 6700 was the chosen number for
git-gc(1) as well, but its probably just good to be consistent here. I
think this patch is fine as is.

-Justin