* [RFC/PATCH] Revert "check_packed_git_idx(): check integrity of the idx file itself."
@ 2006-01-19 4:34 Junio C Hamano
0 siblings, 0 replies; only message in thread
From: Junio C Hamano @ 2006-01-19 4:34 UTC (permalink / raw)
To: git; +Cc: romieu, Andrew Morton
Andrew noticed that this format-patch in the linux-2.6
repository runs very very very slowly. I tried it myself and
found it to be 4x slowdown in a repository where I have both
commits still not packed:
$ git format-patch 7c5c220e 7c4d3367
0.99.9m ran this with 20 seconds wallclock on a Duron 850, while
1.1.3 (and the master) took 47 seconds wallclock. Comparing
user time is worse: 10 seconds vs 37 seconds.
It turns out that this is due to the pack idx file integrity
check I added about a month ago (bisect is cool -- thanks
Linus):
commit c5ced64578a82b9d172aceb2f67c6fb9e639f6d9
Author: Junio C Hamano <junkio@cox.net>
Date: Thu Dec 22 22:57:20 2005 -0800
check_packed_git_idx(): check integrity of the idx file itself.
Although pack-check.c had routine to verify the checksum for the
pack index file itself, the core did not check it before using
it.
The potential problem I saw with the core before this fix was
that after mapping the pack idx file and did some sanity
checks, we did not check the checksum we had in the idx file
that made sure the idx file itself is not corrupt. Doing it
everytime turned out to be quite expensive as illustrated by
this problem. With the revert, the same test runs as fast as
0.99.9m used to.
git-fsck-objects should catch it if idx file is corrupt, so this
is removing suspender while we still have belt.
So I am going to propose reverting this "safety measure";
objections?
-- >8 --
Date: Wed Jan 18 20:26:14 2006 -0800
This reverts c5ced64578a82b9d172aceb2f67c6fb9e639f6d9 commit.
It turns out that doing this check every time we map the idx file
is quite expensive. A corrupt idx file is caught by git-fsck-objects,
so this check is not strictly necessary.
In one unscientific test, 0.99.9m spent 10 seconds usertime for
the same task 1.1.3 takes 37 seconds usertime. Reverting this gives
us the performance of 0.99.9 back.
---
sha1_file.c | 16 +---------------
1 files changed, 1 insertions(+), 15 deletions(-)
a53f669407a76d01c7271124d06366a2dbd5a12d
diff --git a/sha1_file.c b/sha1_file.c
index 8bebbb2..20f6419 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -349,16 +349,12 @@ struct packed_git *packed_git;
static int check_packed_git_idx(const char *path, unsigned long *idx_size_,
void **idx_map_)
{
- SHA_CTX ctx;
- unsigned char sha1[20];
void *idx_map;
unsigned int *index;
unsigned long idx_size;
int nr, i;
- int fd;
+ int fd = open(path, O_RDONLY);
struct stat st;
-
- fd = open(path, O_RDONLY);
if (fd < 0)
return -1;
if (fstat(fd, &st)) {
@@ -396,16 +392,6 @@ static int check_packed_git_idx(const ch
if (idx_size != 4*256 + nr * 24 + 20 + 20)
return error("wrong index file size");
- /*
- * File checksum.
- */
- SHA1_Init(&ctx);
- SHA1_Update(&ctx, idx_map, idx_size-20);
- SHA1_Final(sha1, &ctx);
-
- if (memcmp(sha1, idx_map + idx_size - 20, 20))
- return error("index checksum mismatch");
-
return 0;
}
--
1.1.3.gce7b
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2006-01-19 4:34 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-19 4:34 [RFC/PATCH] Revert "check_packed_git_idx(): check integrity of the idx file itself." Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).