@matheusbsbs / matheus-bagliano / commits / 146a946321

reftable: fix quadratic behavior in the presence of tombstones

When many tombstones are present in a reftable, operations that need to look up or iterate over refs exhibit quadratic behavior. With 8000 refs deleted and re-created, update-ref takes ~15s, quadrupling for each doubling of input size. The root cause is the merged iterator's suppress_deletions flag. When set, merged_iter_next_void() silently consumes tombstone records in a tight internal loop before returning to the caller. This prevents higher-level code from checking iteration bounds (such as prefix or refname comparisons) until after all tombstones have been scanned. This affects any code path that seeks into a range containing tombstones, including: - refs_verify_refnames_available() seeks to "refs/tags/foo-1/" to check for D/F conflicts and must scan through all subsequent tombstones before the caller can see that they are past the prefix of interest. - reftable_backend_read_ref() seeks to a specific refname and must scan through all subsequent tombstones before returning "not found", because the merged iterator skips the matching tombstone and searches for the next live record. Fix this by making suppress_deletions configurable via reftable_stack_options instead of unconditionally enabling it. Git no longer sets the flag, so tombstones are now returned to callers in the reftable backend, which skip them after their existing bounds checks. This allows iteration to terminate as soon as a tombstone past the relevant bound is encountered. Downstream users of the reftable library (e.g. libgit2) can still enable suppress_deletions through the stack options to retain the previous behavior. This also requires adding deletion checks to the log iteration paths, since suppress_deletions applied to both ref and log iterators. Both tests in p1401 go from ~13s to ~0.2s with this change. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Kristofer Karlsson committed Jul 10, 2026 at 10:36 UTC 146a94632139fc470af1fc118b5ff3dda2d15b40

3 files changed +46 -12

refs/reftable-backend.c

+43 -11

@@ -84,7 +84,8 @@ static int reftable_backend_read_ref(struct reftable_backend *be,

84 if (ret)

85 goto done;

87 - if (strcmp(ref.refname, refname)) {

87 + if (strcmp(ref.refname, refname) ||

88 + reftable_ref_record_is_deletion(&ref)) {

89 ret = 1;

90 goto done;

91 }

@@ -110,7 +111,6 @@ static int reftable_backend_read_ref(struct reftable_backend *be,

111 oidread(oid, reftable_ref_record_val1(&ref),

112 &hash_algos[hash_id]);

113 } else {

113 - /* We got a tombstone, which should not happen. */

114 BUG("unhandled reference value type %d", ref.value_type);

115 }

116

@@ -652,6 +652,9 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)

652 break;

653 }

654

655 + if (iter->ref.value_type == REFTABLE_REF_DELETION)

656 + continue;

657 +

658 if (iter->exclude_patterns && should_exclude_current_ref(iter))

659 continue;

660

@@ -1532,6 +1535,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data

1535 ret = 0;

1536 break;

1537 }

1538 + if (reftable_log_record_is_deletion(&log))

1539 + continue;

1540

1541 ALLOC_GROW(logs, logs_nr + 1, logs_alloc);

1542 tombstone = &logs[logs_nr++];

@@ -1929,6 +1934,8 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)

1934 ret = 0;

1935 break;

1936 }

1937 + if (reftable_log_record_is_deletion(&old_log))

1938 + continue;

1939

1940 free(old_log.refname);

1941

@@ -2061,6 +2068,9 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)

2068 if (iter->err)

2069 break;

2070

2071 + if (reftable_log_record_is_deletion(&iter->log))

2072 + continue;

2073 +

2074 /*

2075 * We want the refnames that we have reflogs for, so we skip if

2076 * we've already produced this name. This could be faster by

@@ -2220,6 +2230,8 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,

2230 ret = 0;

2231 break;

2232 }

2233 + if (reftable_log_record_is_deletion(&log))

2234 + continue;

2235

2236 ret = yield_log_record(refs, &log, fn, cb_data);

2237 if (ret)

@@ -2272,6 +2284,10 @@ static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,

2284 ret = 0;

2285 break;

2286 }

2287 + if (reftable_log_record_is_deletion(&log)) {

2288 + reftable_log_record_release(&log);

2289 + continue;

2290 + }

2291

2292 ALLOC_GROW(logs, logs_nr + 1, logs_alloc);

2293 logs[logs_nr++] = log;

@@ -2318,18 +2334,26 @@ static int reftable_be_reflog_exists(struct ref_store *ref_store,

2334 goto done;

2335

2336 /*

2321 - * Check whether we get at least one log record for the given ref name.

2322 - * If so, the reflog exists, otherwise it doesn't.

2337 + * Check whether we get at least one non-deleted log record for the

2338 + * given ref name. If so, the reflog exists, otherwise it doesn't.

2339 */

2324 - ret = reftable_iterator_next_log(&it, &log);

2325 - if (ret < 0)

2326 - goto done;

2327 - if (ret > 0) {

2328 - ret = 0;

2329 - goto done;

2340 + while (1) {

2341 + ret = reftable_iterator_next_log(&it, &log);

2342 + if (ret < 0)

2343 + goto done;

2344 + if (ret > 0) {

2345 + ret = 0;

2346 + goto done;

2347 + }

2348 + if (strcmp(log.refname, refname)) {

2349 + ret = 0;

2350 + goto done;

2351 + }

2352 + if (!reftable_log_record_is_deletion(&log))

2353 + break;

2354 }

2355

2332 - ret = strcmp(log.refname, refname) == 0;

2356 + ret = 1;

2357

2358 done:

2359 reftable_iterator_destroy(&it);

@@ -2442,6 +2466,8 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da

2466 ret = 0;

2467 break;

2468 }

2469 + if (reftable_log_record_is_deletion(&log))

2470 + continue;

2471

2472 tombstone.refname = (char *)arg->refname;

2473 tombstone.value_type = REFTABLE_LOG_DELETION;

@@ -2625,6 +2651,10 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,

2651 reftable_log_record_release(&log);

2652 break;

2653 }

2654 + if (reftable_log_record_is_deletion(&log)) {

2655 + reftable_log_record_release(&log);

2656 + continue;

2657 + }

2658

2659 oidread(&old_oid, log.value.update.old_hash,

2660 ref_store->repo->hash_algo);

@@ -2791,6 +2821,8 @@ static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,

2821 report.path = refname.buf;

2822

2823 switch (ref.value_type) {

2824 + case REFTABLE_REF_DELETION:

2825 + continue;

2826 case REFTABLE_REF_VAL1:

2827 case REFTABLE_REF_VAL2: {

2828 struct object_id oid;

reftable/reftable-stack.h

@@ -42,6 +42,8 @@ struct reftable_stack_options {

42 */

43 void (*on_reload)(void *payload);

44 void *on_reload_payload;

45 +

46 + int suppress_deletions;

47 };

49 /* open a new reftable stack. The tables along with the table list will be

reftable/stack.c

+1 -1

@@ -337,7 +337,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st,

337 /* Update the stack to point to the new tables. */

338 if (st->merged)

339 reftable_merged_table_free(st->merged);

340 - new_merged->suppress_deletions = 1;

340 + new_merged->suppress_deletions = st->opts.suppress_deletions;

341 st->merged = new_merged;

342

343 if (st->tables)