Re: [PATCH] lookup_object: remove hashtable_index() and optimize hash_obj()

From: Jeff King <peff@peff.net>
Date: Sep 11, 2013, 18:48 UTC
Message-ID: <20130911184845.GA25386@sigill.intra.peff.net>
In-Reply-To: <alpine.LFD.2.03.1309101811510.20709@syhkavp.arg>

On Tue, Sep 10, 2013 at 06:17:12PM -0400, Nicolas Pitre wrote:

> hashtable_index() appears to be a close duplicate of hash_obj().
> Keep only the later and make it usable for all cases.

Thanks. This duplication has often bugged me when looking at that hash table, but I just never actually wrote the patch.

Show 6 quoted lines

> Also remove the modulus as this is an expansive operation.
> The size argument is always a power of 2 anyway, so a simple
> mask operation provides the same result.
> 
> On a 'git rev-list --all --objects' run this decreased the time spent
> in lookup_object from 27.5% to 24.1%.

Nice. This is a tiny bit subtle, though, as the power-of-2 growth happens elsewhere, and we may want to tweak it later (the decorate.c hash, for example, grows by 3/2).

Maybe it's worth squashing in one or both of the comments below as a warning to anybody who tries to tweak it.

---

diff --git a/object.c b/object.c
index e2dae22..5f792cb 100644
--- a/object.c
+++ b/object.c
@@ -47,6 +47,7 @@ static unsigned int hash_obj(const unsigned char *sha1, unsigned int n)
 {
 	unsigned int hash;
 	memcpy(&hash, sha1, sizeof(unsigned int));
+	/* Assumes power-of-2 hash sizes in grow_object_hash */
 	return hash & (n - 1);
 }
 
@@ -94,6 +95,10 @@ static void grow_object_hash(void)
 static void grow_object_hash(void)
 {
 	int i;
+	/*
+	 * Note that this size must always be power-of-2 to match hash_obj
+	 * above.
+	 */
 	int new_hash_size = obj_hash_size < 32 ? 32 : 2 * obj_hash_size;
 	struct object **new_hash;