@matheusbsbs / matheus-bagliano / commits / e4929cdf79

refs: skip collision checks in initial transactions

Reference transactions use `refs_verify_refname_available()` to check for colliding references. This check consists of two parts: - Checks for whether multiple ref updates in the same transaction conflict with each other. - Checks for whether existing refs conflict with any refs part of the transaction. While we generally cannot avoid the first check, the second check is superfluous in cases where the transaction is an initial one in an otherwise empty ref store. The check results in multiple ref reads as well as the creation of a ref iterator for every ref we're checking, which adds up quite fast when performing the check for many refs. Introduce a new flag that allows us to skip this check and wire it up in such that the backends pass it when running an initial transaction. This leads to significant speedups when migrating ref storage backends. From "files" to "reftable": Benchmark 1: migrate files:reftable (refcount = 100000, revision = HEAD~) Time (mean ± σ): 472.4 ms ± 6.7 ms [User: 175.9 ms, System: 285.2 ms] Range (min … max): 463.5 ms … 483.2 ms 10 runs Benchmark 2: migrate files:reftable (refcount = 100000, revision = HEAD) Time (mean ± σ): 86.1 ms ± 1.9 ms [User: 67.9 ms, System: 16.0 ms] Range (min … max): 82.9 ms … 90.9 ms 29 runs Summary migrate files:reftable (refcount = 100000, revision = HEAD) ran 5.48 ± 0.15 times faster than migrate files:reftable (refcount = 100000, revision = HEAD~) And from "reftable" to "files": Benchmark 1: migrate reftable:files (refcount = 100000, revision = HEAD~) Time (mean ± σ): 452.7 ms ± 3.4 ms [User: 209.9 ms, System: 235.4 ms] Range (min … max): 445.9 ms … 457.5 ms 10 runs Benchmark 2: migrate reftable:files (refcount = 100000, revision = HEAD) Time (mean ± σ): 95.2 ms ± 2.2 ms [User: 73.6 ms, System: 20.6 ms] Range (min … max): 91.7 ms … 100.8 ms 28 runs Summary migrate reftable:files (refcount = 100000, revision = HEAD) ran 4.76 ± 0.11 times faster than migrate reftable:files (refcount = 100000, revision = HEAD~) Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Nov 20, 2024 at 08:51 UTC e4929cdf797467b5d55a976ee938f1d4f358900e

5 files changed +36 -27

refs.c

+21 -16

@@ -2324,6 +2324,7 @@ int refs_verify_refname_available(struct ref_store *refs,

2324 const char *refname,

2325 const struct string_list *extras,

2326 const struct string_list *skip,

2327 + int initial_transaction,

2328 struct strbuf *err)

2329 {

2330 const char *slash;

@@ -2332,8 +2333,6 @@ int refs_verify_refname_available(struct ref_store *refs,

2333 struct strbuf referent = STRBUF_INIT;

2334 struct object_id oid;

2335 unsigned int type;

2335 - struct ref_iterator *iter;

2336 - int ok;

2336 int ret = -1;

2337

2338 /*

@@ -2363,7 +2362,8 @@ int refs_verify_refname_available(struct ref_store *refs,

2362 if (skip && string_list_has_string(skip, dirname.buf))

2363 continue;

2364

2366 - if (!refs_read_raw_ref(refs, dirname.buf, &oid, &referent,

2365 + if (!initial_transaction &&

2366 + !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,

2367 &type, &ignore_errno)) {

2368 strbuf_addf(err, _("'%s' exists; cannot create '%s'"),

2369 dirname.buf, refname);

@@ -2388,21 +2388,26 @@ int refs_verify_refname_available(struct ref_store *refs,

2388 strbuf_addstr(&dirname, refname + dirname.len);

2389 strbuf_addch(&dirname, '/');

2390

2391 - iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0,

2392 - DO_FOR_EACH_INCLUDE_BROKEN);

2393 - while ((ok = ref_iterator_advance(iter)) == ITER_OK) {

2394 - if (skip &&

2395 - string_list_has_string(skip, iter->refname))

2396 - continue;

2391 + if (!initial_transaction) {

2392 + struct ref_iterator *iter;

2393 + int ok;

2394

2398 - strbuf_addf(err, _("'%s' exists; cannot create '%s'"),

2399 - iter->refname, refname);

2400 - ref_iterator_abort(iter);

2401 - goto cleanup;

2402 - }

2395 + iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0,

2396 + DO_FOR_EACH_INCLUDE_BROKEN);

2397 + while ((ok = ref_iterator_advance(iter)) == ITER_OK) {

2398 + if (skip &&

2399 + string_list_has_string(skip, iter->refname))

2400 + continue;

2401

2404 - if (ok != ITER_DONE)

2405 - BUG("error while iterating over references");

2402 + strbuf_addf(err, _("'%s' exists; cannot create '%s'"),

2403 + iter->refname, refname);

2404 + ref_iterator_abort(iter);

2405 + goto cleanup;

2406 + }

2407 +

2408 + if (ok != ITER_DONE)

2409 + BUG("error while iterating over references");

2410 + }

2411

2412 extra_refname = find_descendant_ref(dirname.buf, extras, skip);

2413 if (extra_refname)

refs.h

+4 -1

@@ -101,13 +101,16 @@ int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname,

101 * both "foo" and with "foo/bar/baz" but not with "foo/bar" or

102 * "foo/barbados".

103 *

104 + * If `initial_transaction` is truish, then all collision checks with

105 + * preexisting refs are skipped.

106 + *

107 * extras and skip must be sorted.

108 */

106 -

109 int refs_verify_refname_available(struct ref_store *refs,

110 const char *refname,

111 const struct string_list *extras,

112 const struct string_list *skip,

113 + int initial_transaction,

114 struct strbuf *err);

115

116 int refs_ref_exists(struct ref_store *refs, const char *refname);

refs/files-backend.c

+6 -7

@@ -706,7 +706,7 @@ retry:

706 * reason to expect this error to be transitory.

707 */

708 if (refs_verify_refname_available(&refs->base, refname,

709 - extras, NULL, err)) {

709 + extras, NULL, 0, err)) {

710 if (mustexist) {

711 /*

712 * To the user the relevant error is

@@ -813,7 +813,7 @@ retry:

813 REMOVE_DIR_EMPTY_ONLY)) {

814 if (refs_verify_refname_available(

815 &refs->base, refname,

816 - extras, NULL, err)) {

816 + extras, NULL, 0, err)) {

817 /*

818 * The error message set by

819 * verify_refname_available() is OK.

@@ -850,7 +850,7 @@ retry:

850 */

851 if (refs_verify_refname_available(

852 refs->packed_ref_store, refname,

853 - extras, NULL, err)) {

853 + extras, NULL, 0, err)) {

854 ret = TRANSACTION_NAME_CONFLICT;

855 goto error_return;

856 }

@@ -1159,7 +1159,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,

1159 */

1160 if (is_null_oid(&lock->old_oid) &&

1161 refs_verify_refname_available(refs->packed_ref_store, refname,

1162 - NULL, NULL, err))

1162 + NULL, NULL, 0, err))

1163 goto error_return;

1164

1165 lock->ref_name = xstrdup(refname);

@@ -1538,7 +1538,7 @@ static int refs_rename_ref_available(struct ref_store *refs,

1538

1539 string_list_insert(&skip, old_refname);

1540 ok = !refs_verify_refname_available(refs, new_refname,

1541 - NULL, &skip, &err);

1541 + NULL, &skip, 0, &err);

1542 if (!ok)

1543 error("%s", err.buf);

1544

@@ -3043,8 +3043,7 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,

3043 BUG("initial ref transaction with old_sha1 set");

3044

3045 if (refs_verify_refname_available(&refs->base, update->refname,

3046 - &affected_refnames, NULL,

3047 - err)) {

3046 + &affected_refnames, NULL, 1, err)) {

3047 ret = TRANSACTION_NAME_CONFLICT;

3048 goto cleanup;

3049 }

refs/reftable-backend.c

+4 -2

@@ -1097,7 +1097,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,

1097 * at a later point.

1098 */

1099 ret = refs_verify_refname_available(ref_store, u->refname,

1100 - &affected_refnames, NULL, err);

1100 + &affected_refnames, NULL,

1101 + transaction->flags & REF_TRANSACTION_FLAG_INITIAL,

1102 + err);

1103 if (ret < 0)

1104 goto done;

1105

@@ -1584,7 +1586,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)

1586 if (arg->delete_old)

1587 string_list_insert(&skip, arg->oldname);

1588 ret = refs_verify_refname_available(&arg->refs->base, arg->newname,

1587 - NULL, &skip, &errbuf);

1589 + NULL, &skip, 0, &errbuf);

1590 if (ret < 0) {

1591 error("%s", errbuf.buf);

1592 goto done;

t/helper/test-ref-store.c

+1 -1

@@ -199,7 +199,7 @@ static int cmd_verify_ref(struct ref_store *refs, const char **argv)

199 struct strbuf err = STRBUF_INIT;

200 int ret;

201

202 - ret = refs_verify_refname_available(refs, refname, NULL, NULL, &err);

202 + ret = refs_verify_refname_available(refs, refname, NULL, NULL, 0, &err);

203 if (err.len)

204 puts(err.buf);

205 return ret;