From 9892bebafe0865d8f4f3f18d60a1cfa2d1447cd7 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Sat, 20 Feb 2010 23:27:31 -0500 Subject: sha1_file: don't malloc the whole compressed result when writing out objects There is no real advantage to malloc the whole output buffer and deflate the data in a single pass when writing loose objects. That is like only 1% faster while using more memory, especially with large files where memory usage is far more. It is best to deflate and write the data out in small chunks reusing the same memory instead. For example, using 'git add' on a few large files averaging 40 MB ... Before: 21.45user 1.10system 0:22.57elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k 0inputs+828040outputs (0major+142640minor)pagefaults 0swaps After: 21.50user 1.25system 0:22.76elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k 0inputs+828040outputs (0major+104408minor)pagefaults 0swaps While the runtime stayed relatively the same, the number of minor page faults went down significantly. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano diff --git a/sha1_file.c b/sha1_file.c index 657825e..9196b57 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2281,8 +2281,7 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, void *buf, unsigned long len, time_t mtime) { int fd, ret; - size_t size; - unsigned char *compressed; + unsigned char compressed[4096]; z_stream stream; char *filename; static char tmpfile[PATH_MAX]; @@ -2301,12 +2300,8 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, /* Set it up */ memset(&stream, 0, sizeof(stream)); deflateInit(&stream, zlib_compression_level); - size = 8 + deflateBound(&stream, len+hdrlen); - compressed = xmalloc(size); - - /* Compress it */ stream.next_out = compressed; - stream.avail_out = size; + stream.avail_out = sizeof(compressed); /* First header.. */ stream.next_in = (unsigned char *)hdr; @@ -2317,20 +2312,21 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, /* Then the data itself.. */ stream.next_in = buf; stream.avail_in = len; - ret = deflate(&stream, Z_FINISH); + do { + ret = deflate(&stream, Z_FINISH); + if (write_buffer(fd, compressed, stream.next_out - compressed) < 0) + die("unable to write sha1 file"); + stream.next_out = compressed; + stream.avail_out = sizeof(compressed); + } while (ret == Z_OK); + if (ret != Z_STREAM_END) die("unable to deflate new object %s (%d)", sha1_to_hex(sha1), ret); - ret = deflateEnd(&stream); if (ret != Z_OK) die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), ret); - size = stream.total_out; - - if (write_buffer(fd, compressed, size) < 0) - die("unable to write sha1 file"); close_sha1_file(fd); - free(compressed); if (mtime) { struct utimbuf utb; -- cgit v0.10.2-6-g49f6 From 748af44c63ea6fec12690f1693f3dddd963e88d5 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Sun, 21 Feb 2010 15:48:06 -0500 Subject: sha1_file: be paranoid when creating loose objects We don't want the data being deflated and stored into loose objects to be different from what we expect. While the deflated data is protected by a CRC which is good enough for safe data retrieval operations, we still want to be doubly sure that the source data used at object creation time is still what we expected once that data has been deflated and its CRC32 computed. The most plausible data corruption may occur if the source file is modified while Git is deflating and writing it out in a loose object. Or Git itself could have a bug causing memory corruption. Or even bad RAM could cause trouble. So it is best to make sure everything is coherent and checksum protected from beginning to end. To do so we compute the SHA1 of the data being deflated _after_ the deflate operation has consumed that data, and make sure it matches with the expected SHA1. This way we can rely on the CRC32 checked by the inflate operation to provide a good indication that the data is still coherent with its SHA1 hash. One pathological case we ignore is when the data is modified before (or during) deflate call, but changed back before it is hashed. There is some overhead of course. Using 'git add' on a set of large files: Before: real 0m25.210s user 0m23.783s sys 0m1.408s After: real 0m26.537s user 0m25.175s sys 0m1.358s The overhead is around 5% for full data coherency guarantee. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano diff --git a/sha1_file.c b/sha1_file.c index 9196b57..c0214d7 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2283,6 +2283,8 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, int fd, ret; unsigned char compressed[4096]; z_stream stream; + git_SHA_CTX c; + unsigned char parano_sha1[20]; char *filename; static char tmpfile[PATH_MAX]; @@ -2302,18 +2304,22 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, deflateInit(&stream, zlib_compression_level); stream.next_out = compressed; stream.avail_out = sizeof(compressed); + git_SHA1_Init(&c); /* First header.. */ stream.next_in = (unsigned char *)hdr; stream.avail_in = hdrlen; while (deflate(&stream, 0) == Z_OK) /* nothing */; + git_SHA1_Update(&c, hdr, hdrlen); /* Then the data itself.. */ stream.next_in = buf; stream.avail_in = len; do { + unsigned char *in0 = stream.next_in; ret = deflate(&stream, Z_FINISH); + git_SHA1_Update(&c, in0, stream.next_in - in0); if (write_buffer(fd, compressed, stream.next_out - compressed) < 0) die("unable to write sha1 file"); stream.next_out = compressed; @@ -2325,6 +2331,9 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen, ret = deflateEnd(&stream); if (ret != Z_OK) die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), ret); + git_SHA1_Final(parano_sha1, &c); + if (hashcmp(sha1, parano_sha1) != 0) + die("confused by unstable object source data for %s", sha1_to_hex(sha1)); close_sha1_file(fd); -- cgit v0.10.2-6-g49f6