reftable/stack: avoid reloading the stack when already locked

When making modifications to the reftable stack, the stack obtains a
lock to the list file and removes the lock after the commit phase. Since
most operations reload the stack to ensure we have the latest state, any
branched operation during the locked phase could trigger a state reload.

To prevent data loss due to concurrent writes, state reload is necessary
right after obtaining the lock. But any reloads after that are just a
no-op. Now that the struct has access to the lock file status, simply
skip reloading if the lock is present.

Benchmarking with a fixed, non-symbolic target OID in the 'refs/tags/'
namespace (since it triggers a stack reload when checking if reflog
exists for the given tag name), shows a consistent 15-20% improvement
with these patches:

  refcount   master     patch     speedup
  --------   -------    -------   -------
  2,000       18.5 ms    16.6 ms   1.11x
  20,000     120.7 ms   102.8 ms   1.17x
  50,000     296.5 ms   247.1 ms   1.20x

We can also see the improvements in the number of syscall counts. On
master, the number of calls to `newfstatat()` grows linearly with the
number of refs created. With this patch, the number is now a constant:

  refcount   master   patch
  --------   ------   ------
  1,000      1,059       55
  5,000      5,059       55
  10,000     10,059      55
  20,000     20,059      55

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
next
Karthik Nayak 2026-08-24 11:31:02 +02:00 committed by Junio C Hamano
parent 654567cf1b
commit 976644c380
1 changed files with 12 additions and 5 deletions

View File

@ -553,14 +553,21 @@ out:

/*
* Check whether the given stack is up-to-date with what we have in memory.
* If skip_if_locked is set skip stack reloading if the stack is currently
* locked. Stack reloading must _not_ be skipped right after obtaining the
* lock, to check for concurrent updates which may have happened.
*
* Returns 0 if so, 1 if the stack is out-of-date or a negative error code
* otherwise.
*/
static int stack_uptodate(struct reftable_stack *st)
static int stack_uptodate(struct reftable_stack *st, int skip_if_locked)
{
char **names = NULL;
int err;

if (skip_if_locked && st->list_lock.fd != -1)
return 0;

/*
* When we have cached stat information available then we use it to
* verify whether the file has been rewritten.
@ -623,7 +630,7 @@ done:

int reftable_stack_reload(struct reftable_stack *st)
{
int err = stack_uptodate(st);
int err = stack_uptodate(st, 1);
if (err > 0)
return reftable_stack_reload_maybe_reuse(st, 1);
return err;
@ -693,7 +700,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
}
}

err = stack_uptodate(st);
err = stack_uptodate(st, 0);
if (err < 0)
goto done;
if (err > 0) {
@ -1200,7 +1207,7 @@ static int stack_compact_range(struct reftable_stack *st,
* we could check that relevant tables still exist. But for now it's
* good enough to just abort.
*/
err = stack_uptodate(st);
err = stack_uptodate(st, 0);
if (err < 0)
goto done;
if (err > 0) {
@ -1319,7 +1326,7 @@ static int stack_compact_range(struct reftable_stack *st,
* tables with our compacted version. If they don't, then we need to
* abort.
*/
err = stack_uptodate(st);
err = stack_uptodate(st, 0);
if (err < 0)
goto done;
if (err > 0) {