Skip to content

Commit 0eeb077

Browse files
peffgitster
authored andcommitted
index-pack: avoid excessive re-reading of pack directory
Since 45e8a74 (has_sha1_file: re-check pack directory before giving up, 2013-08-30), we spend extra effort for has_sha1_file to give the right answer when somebody else is repacking. Usually this effort does not matter, because after finding that the object does not exist, the next step is usually to die(). However, some code paths make a large number of has_sha1_file checks which are _not_ expected to return 1. The collision test in index-pack.c is such a case. On a local system, this can cause a performance slowdown of around 5%. But on a system with high-latency system calls (like NFS), it can be much worse. This patch introduces a "quick" flag to has_sha1_file which callers can use when they would prefer high performance at the cost of false negatives during repacks. There may be other code paths that can use this, but the index-pack one is the most obviously critical, so we'll start with switching that one. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 282616c commit 0eeb077

File tree

3 files changed

+14
-3
lines changed

3 files changed

+14
-3
lines changed

builtin/index-pack.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
721721
assert(data || obj_entry);
722722

723723
read_lock();
724-
collision_test_needed = has_sha1_file(sha1);
724+
collision_test_needed = has_sha1_file_with_flags(sha1, HAS_SHA1_QUICK);
725725
read_unlock();
726726

727727
if (collision_test_needed && !data) {

cache.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -873,8 +873,17 @@ extern int has_sha1_pack(const unsigned char *sha1);
873873
* Return true iff we have an object named sha1, whether local or in
874874
* an alternate object database, and whether packed or loose. This
875875
* function does not respect replace references.
876+
*
877+
* If the QUICK flag is set, do not re-check the pack directory
878+
* when we cannot find the object (this means we may give a false
879+
* negative answer if another process is simultaneously repacking).
876880
*/
877-
extern int has_sha1_file(const unsigned char *sha1);
881+
#define HAS_SHA1_QUICK 0x1
882+
extern int has_sha1_file_with_flags(const unsigned char *sha1, int flags);
883+
static inline int has_sha1_file(const unsigned char *sha1)
884+
{
885+
return has_sha1_file_with_flags(sha1, 0);
886+
}
878887

879888
/*
880889
* Return true iff an alternate object database has a loose object

sha1_file.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3017,14 +3017,16 @@ int has_sha1_pack(const unsigned char *sha1)
30173017
return find_pack_entry(sha1, &e);
30183018
}
30193019

3020-
int has_sha1_file(const unsigned char *sha1)
3020+
int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
30213021
{
30223022
struct pack_entry e;
30233023

30243024
if (find_pack_entry(sha1, &e))
30253025
return 1;
30263026
if (has_loose_object(sha1))
30273027
return 1;
3028+
if (flags & HAS_SHA1_QUICK)
3029+
return 0;
30283030
reprepare_packed_git();
30293031
return find_pack_entry(sha1, &e);
30303032
}

0 commit comments

Comments
 (0)