Re: [PATCH 4/6] introduce a commit metapack

On Thu, Jan 31, 2013 at 06:06:56PM +0700, Nguyen Thai Ngoc Duy wrote:

On Wed, Jan 30, 2013 at 09:16:29PM +0700, Duy Nguyen wrote:

quoted

Perhaps we could store abbrev sha-1 instead of full sha-1. Nice
space/time trade-off.

Following the on-disk format experiment yesterday, I changed the
format to:

 - a list a _short_ SHA-1 of cached commits
 - a list of cache entries, each (5 uint32_t) consists of:
   - uint32_t for the index in .idx sha-1 table to get full SHA-1 of
     the commit
   - uint32_t for timestamp
   - uint32_t for tree, 1st and 2nd parents for the index in .idx
     table

BTW, I needed the minor fixups below to silence some warnings from your
patch. Here are the cold and warm cache timings I got, as compared to
stock git and my implementation:

 Pack  |  Cold Revs   |  Warm Revs
-------+--------------+------------
 stock | 12.54        | 4.14
    me |  4.76 (-62%) | 0.66 (-84%)
   duy |  4.36 (-65%) | 0.55 (-86%)

Not surprising; yours is just a bit faster in terms of CPU, and even
gains a little more in the cold cache case. Nice. Of course that is just
gravy on top of the smaller disk usage, too. :)

---

diff --git a/commit-metapack.c b/commit-metapack.c
index c984b8e..78fd961 100644
--- a/commit-metapack.c
+++ b/commit-metapack.c

@@ -106,7 +106,7 @@ int commit_metapack(unsigned char *sha1,
 	for (p = commit_metapacks; p; p = p->next) {
 		struct commit_entry *data;
 		uint32_t p1, p2;
-		unsigned lo, hi, mi;
+		unsigned lo, hi;
 		int pos;
 
 		/* sha1_entry_pos does not work with abbreviated sha-1 */

@@ -161,7 +161,7 @@ static void get_commits(struct metapack_writer *mw,
 	struct write_cb *write_cb = (struct write_cb *)data;
 	enum object_type type = sha1_object_info(sha1, NULL);
 	struct commit *c;
-	int p1, p2;
+	int p1 = -1, p2 = -1;
 
 	if (type != OBJ_COMMIT)
 		return;

diff --git a/commit.c b/commit.c
index b326201..5b776f8 100644
--- a/commit.c
+++ b/commit.c

@@ -309,7 +309,7 @@ static int parse_commit_metapack(struct commit *item)
 
 static int parse_commit_metapack(struct commit *item)
 {
-	unsigned char *tree, *p1, *p2;
+	const unsigned char *tree, *p1, *p2;
 	uint32_t ts;
 
 	if (commit_metapack(item->object.sha1, &ts, &tree, &p1, &p2) < 0)