Skip to content

Commit 5437bc4

Browse files
mhaggerdscho
authored andcommitted
files-backend: cheapen refname_available check when locking refs
When locking references in preparation for updating them, we need to check that none of the newly added references D/F conflict with existing references (e.g., we don't allow `refs/foo` to be added if `refs/foo/bar` already exists, or vice versa). Prior to 524a9fd (refs_verify_refname_available(): use function in more places, 2017-04-16), conflicts with existing loose references were checked by looking directly in the filesystem, and then conflicts with existing packed references were checked by running `verify_refname_available_dir()` against the packed-refs cache. But that commit changed the final check to call `refs_verify_refname_available()` against the *whole* files ref-store, including both loose and packed references, with the following comment: > This means that those callsites now check for conflicts with all > references rather than just packed refs, but the performance cost > shouldn't be significant (and will be regained later). That comment turned out to be too sanguine. User [email protected] reported that fetches involving a very large number of references in neighboring directories were slowed down by that change. The problem is that when fetching, each reference is updated individually, within its own reference transaction. This is done because some reference updates might succeed even though others fail. But every time a reference update transaction is finished, `clear_loose_ref_cache()` is called. So when it is time to update the next reference, part of the loose ref cache has to be repopulated for the `refs_verify_refname_available()` call. If the references are all in neighboring directories, then the cost of repopulating the reference cache increases with the number of references, resulting in O(N²) effort. The comment above also claims that the performance cost "will be regained later". The idea was that once the packed-refs were finished being split out into a separate ref-store, we could limit the `refs_verify_refname_available()` call to the packed references again. That is what we do now. Signed-off-by: Michael Haggerty <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent fdc9913 commit 5437bc4

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

refs/files-backend.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -632,11 +632,11 @@ static int lock_raw_ref(struct files_ref_store *refs,
632632

633633
/*
634634
* If the ref did not exist and we are creating it,
635-
* make sure there is no existing ref that conflicts
636-
* with refname:
635+
* make sure there is no existing packed ref that
636+
* conflicts with refname:
637637
*/
638638
if (refs_verify_refname_available(
639-
&refs->base, refname,
639+
refs->packed_ref_store, refname,
640640
extras, skip, err))
641641
goto error_return;
642642
}
@@ -939,7 +939,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
939939
* our refname.
940940
*/
941941
if (is_null_oid(&lock->old_oid) &&
942-
refs_verify_refname_available(&refs->base, refname,
942+
refs_verify_refname_available(refs->packed_ref_store, refname,
943943
extras, skip, err)) {
944944
last_errno = ENOTDIR;
945945
goto error_return;

0 commit comments

Comments
 (0)