Skip to content

Commit

Permalink
index-pack: avoid excessive re-reading of pack directory
Browse files Browse the repository at this point in the history
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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Jun 9, 2015
1 parent 282616c commit 0eeb077
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 3 deletions.
2 changes: 1 addition & 1 deletion builtin/index-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
assert(data || obj_entry);

read_lock();
collision_test_needed = has_sha1_file(sha1);
collision_test_needed = has_sha1_file_with_flags(sha1, HAS_SHA1_QUICK);
read_unlock();

if (collision_test_needed && !data) {
Expand Down
11 changes: 10 additions & 1 deletion cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -873,8 +873,17 @@ extern int has_sha1_pack(const unsigned char *sha1);
* Return true iff we have an object named sha1, whether local or in
* an alternate object database, and whether packed or loose. This
* function does not respect replace references.
*
* If the QUICK flag is set, do not re-check the pack directory
* when we cannot find the object (this means we may give a false
* negative answer if another process is simultaneously repacking).
*/
extern int has_sha1_file(const unsigned char *sha1);
#define HAS_SHA1_QUICK 0x1
extern int has_sha1_file_with_flags(const unsigned char *sha1, int flags);
static inline int has_sha1_file(const unsigned char *sha1)
{
return has_sha1_file_with_flags(sha1, 0);
}

/*
* Return true iff an alternate object database has a loose object
Expand Down
4 changes: 3 additions & 1 deletion sha1_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -3017,14 +3017,16 @@ int has_sha1_pack(const unsigned char *sha1)
return find_pack_entry(sha1, &e);
}

int has_sha1_file(const unsigned char *sha1)
int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
{
struct pack_entry e;

if (find_pack_entry(sha1, &e))
return 1;
if (has_loose_object(sha1))
return 1;
if (flags & HAS_SHA1_QUICK)
return 0;
reprepare_packed_git();
return find_pack_entry(sha1, &e);
}
Expand Down

0 comments on commit 0eeb077

Please sign in to comment.