From: Jeff King <peff@peff.net>
Date: Sat, 02 Aug 2025 10:48:16 GMT
Subject: Re: [PATCH 3/4] builtin/remote: rework how remote refs get renamed
Message-ID: <20250802104816.GB1180347@coredump.intra.peff.net>
In-Reply-To: <aIi9mAoAb1v9igQ6@pks.im>

On Tue, Jul 29, 2025 at 02:24:56PM +0200, Patrick Steinhardt wrote:

> > It's hard to bring myself to care, though. This is a stupidly
> > pathological reflog, and the absolute time change is peanuts compared to
> > the per-ref cost you're fixing here.
> 
> For the "files" backend performance is worse, for the "reftable" backend
> I'd expect that this might even be faster. Mostly because there is no
> way to trivially rename a reflog -- we basically do the same on a rename
> as we are doing with this patch series now.
> 
> Overall I don't care too much about this edge case. By default we never
> write reflogs for remote references anyway, and I doubt that you'll ever
> end up with a remote reflog that has thousands of entries. So I'd rather
> make the general case fast even if the esoteric case becomes slower.
> 
> But ideally we're able to lift such limitations in the future if we were
> to do the above rework.

OK. It looks like you did end up with a single transaction in your
re-roll. So in theory _if_ we had a "rename" transaction entry the
backends could be smarter here. But I agree with you that touching the
core of the ref transaction code is tricky and liable to cause
regressions. Given the numbers I produced earlier, I'm fine with leaving
it for later (or maybe never) and just copying the reflog entries as you
do in your series.

-Peff