{"thread":{"id":"13357","subject":"[PATCH 0/7] assorted pack-objects cleanups and improvements","startedAt":"2008-05-02T19:11:44Z","lastAt":"2008-05-02T23:03:30Z","messageCount":10,"participants":["Nicolas Pitre","A Large Angry SCM"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"75864","messageId":"1209755511-7840-1-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":null,"subject":"[PATCH 0/7] assorted pack-objects cleanups and improvements","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:44Z","receivedAt":"2008-05-02T19:11:44Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"This is a series of patches with small fixes, minor cleanups and some\nimprovements especially for threaded repacks.\n\nThe diffstat is rather simple:\n\n builtin-pack-objects.c |  197 +++++++++++++++++++++++++++++-------------------\n 1 files changed, 118 insertions(+), 79 deletions(-)\n\n\nNicolas\n"},{"id":"75865","messageId":"1209755511-7840-2-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":"1209755511-7840-1-git-send-email-nico@cam.org","subject":"[PATCH 1/7] pack-objects: small cleanup","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:45Z","receivedAt":"2008-05-02T19:11:45Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Better encapsulate delta creation for writing.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n builtin-pack-objects.c |   27 +++++++++++++--------------\n 1 files changed, 13 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 777f272..8691c99 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -102,21 +102,24 @@ static uint32_t written, written_delta;\n static uint32_t reused, reused_delta;\n \n \n-static void *delta_against(void *buf, unsigned long size, struct object_entry *entry)\n+static void *get_delta(struct object_entry *entry)\n {\n-\tunsigned long othersize, delta_size;\n+\tunsigned long size, base_size, delta_size;\n+\tvoid *buf, *base_buf, *delta_buf;\n \tenum object_type type;\n-\tvoid *otherbuf = read_sha1_file(entry->delta->idx.sha1, &type, &othersize);\n-\tvoid *delta_buf;\n \n-\tif (!otherbuf)\n+\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n+\tif (!buf)\n+\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n+\tbase_buf = read_sha1_file(entry->delta->idx.sha1, &type, &base_size);\n+\tif (!base_buf)\n \t\tdie(\"unable to read %s\", sha1_to_hex(entry->delta->idx.sha1));\n-        delta_buf = diff_delta(otherbuf, othersize,\n+\tdelta_buf = diff_delta(base_buf, base_size,\n \t\t\t       buf, size, &delta_size, 0);\n-        if (!delta_buf || delta_size != entry->delta_size)\n+\tif (!delta_buf || delta_size != entry->delta_size)\n \t\tdie(\"delta size changed\");\n-        free(buf);\n-        free(otherbuf);\n+\tfree(buf);\n+\tfree(base_buf);\n \treturn delta_buf;\n }\n \n@@ -223,7 +226,6 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t  off_t write_offset)\n {\n \tunsigned long size;\n-\tenum object_type type;\n \tvoid *buf;\n \tunsigned char header[10];\n \tunsigned char dheader[10];\n@@ -281,10 +283,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\tobj_type = (allow_ofs_delta && entry->delta->idx.offset) ?\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t\t} else {\n-\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n-\t\t\tif (!buf)\n-\t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n-\t\t\tbuf = delta_against(buf, size, entry);\n+\t\t\tbuf = get_delta(entry);\n \t\t\tsize = entry->delta_size;\n \t\t\tobj_type = (allow_ofs_delta && entry->delta->idx.offset) ?\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n-- \n1.5.5.1.226.g6f6e8\n"},{"id":"75867","messageId":"1209755511-7840-3-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":"1209755511-7840-2-git-send-email-nico@cam.org","subject":"[PATCH 2/7] pack-objects: remove some double negative logic","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:46Z","receivedAt":"2008-05-02T19:11:46Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Parsing !no_reuse_delta everywhere makes my brain spend extra\ncycles wondering each time.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n builtin-pack-objects.c |   17 +++++++++--------\n 1 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 8691c99..afbf3dd 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -65,7 +65,8 @@ static struct pack_idx_entry **written_list;\n static uint32_t nr_objects, nr_alloc, nr_result, nr_written;\n \n static int non_empty;\n-static int no_reuse_delta, no_reuse_object, keep_unreachable, include_tag;\n+static int reuse_delta = 1, reuse_object = 1;\n+static int keep_unreachable, include_tag;\n static int local;\n static int incremental;\n static int allow_ofs_delta;\n@@ -251,7 +252,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\tcrc32_begin(f);\n \n \tobj_type = entry->type;\n-\tif (no_reuse_object)\n+\tif (!reuse_object)\n \t\tto_reuse = 0;\t/* explicit */\n \telse if (!entry->in_pack)\n \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n@@ -1021,7 +1022,7 @@ static void check_object(struct object_entry *entry)\n \t\t\tunuse_pack(&w_curs);\n \t\t\treturn;\n \t\tcase OBJ_REF_DELTA:\n-\t\t\tif (!no_reuse_delta && !entry->preferred_base)\n+\t\t\tif (reuse_delta && !entry->preferred_base)\n \t\t\t\tbase_ref = use_pack(p, &w_curs,\n \t\t\t\t\t\tentry->in_pack_offset + used, NULL);\n \t\t\tentry->in_pack_header_size = used + 20;\n@@ -1044,7 +1045,7 @@ static void check_object(struct object_entry *entry)\n \t\t\t\tdie(\"delta base offset out of bound for %s\",\n \t\t\t\t    sha1_to_hex(entry->idx.sha1));\n \t\t\tofs = entry->in_pack_offset - ofs;\n-\t\t\tif (!no_reuse_delta && !entry->preferred_base) {\n+\t\t\tif (reuse_delta && !entry->preferred_base) {\n \t\t\t\tstruct revindex_entry *revidx;\n \t\t\t\trevidx = find_pack_revindex(p, ofs);\n \t\t\t\tbase_ref = nth_packed_object_sha1(p, revidx->nr);\n@@ -1232,7 +1233,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \t * We do not bother to try a delta that we discarded\n \t * on an earlier try, but only when reusing delta data.\n \t */\n-\tif (!no_reuse_delta && trg_entry->in_pack &&\n+\tif (reuse_delta && trg_entry->in_pack &&\n \t    trg_entry->in_pack == src_entry->in_pack &&\n \t    trg_entry->in_pack_type != OBJ_REF_DELTA &&\n \t    trg_entry->in_pack_type != OBJ_OFS_DELTA)\n@@ -1687,7 +1688,7 @@ static void prepare_pack(int window, int depth)\n \n \t\tif (entry->delta)\n \t\t\t/* This happens if we decided to reuse existing\n-\t\t\t * delta from a pack.  \"!no_reuse_delta &&\" is implied.\n+\t\t\t * delta from a pack.  \"reuse_delta &&\" is implied.\n \t\t\t */\n \t\t\tcontinue;\n \n@@ -2049,11 +2050,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--no-reuse-delta\", arg)) {\n-\t\t\tno_reuse_delta = 1;\n+\t\t\treuse_delta = 0;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--no-reuse-object\", arg)) {\n-\t\t\tno_reuse_object = no_reuse_delta = 1;\n+\t\t\treuse_object = reuse_delta = 0;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--delta-base-offset\", arg)) {\n-- \n1.5.5.1.226.g6f6e8\n"},{"id":"75869","messageId":"1209755511-7840-4-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":"1209755511-7840-3-git-send-email-nico@cam.org","subject":"[PATCH 3/7] pack-objects: simplify the condition associated with --all-progress","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:47Z","receivedAt":"2008-05-02T19:11:47Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Signed-off-by: Nicolas Pitre <nico@cam.org>\n---\n builtin-pack-objects.c |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex afbf3dd..b4a63d2 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -452,11 +452,10 @@ static void write_pack_file(void)\n \tstruct sha1file *f;\n \toff_t offset, offset_one, last_obj_offset = 0;\n \tstruct pack_header hdr;\n-\tint do_progress = progress >> pack_to_stdout;\n \tuint32_t nr_remaining = nr_result;\n \ttime_t last_mtime = 0;\n \n-\tif (do_progress)\n+\tif (progress > pack_to_stdout)\n \t\tprogress_state = start_progress(\"Writing objects\", nr_result);\n \twritten_list = xmalloc(nr_objects * sizeof(*written_list));\n \n-- \n1.5.5.1.226.g6f6e8\n"},{"id":"75871","messageId":"1209755511-7840-5-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":"1209755511-7840-4-git-send-email-nico@cam.org","subject":"[PATCH 4/7] pack-objects: clean up write_object() a bit","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:48Z","receivedAt":"2008-05-02T19:11:48Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"... for improved readability.  No functional changes.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n builtin-pack-objects.c |   64 ++++++++++++++++++++++++-----------------------\n 1 files changed, 33 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex b4a63d2..257002a 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -226,41 +226,43 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t  struct object_entry *entry,\n \t\t\t\t  off_t write_offset)\n {\n-\tunsigned long size;\n+\tunsigned long size, limit;\n \tvoid *buf;\n-\tunsigned char header[10];\n-\tunsigned char dheader[10];\n+\tunsigned char header[10], dheader[10];\n \tunsigned hdrlen;\n \toff_t datalen;\n-\tenum object_type obj_type;\n-\tint to_reuse = 0;\n-\t/* write limit if limited packsize and not first object */\n-\tunsigned long limit = pack_size_limit && nr_written ?\n-\t\t\t\tpack_size_limit - write_offset : 0;\n-\t\t\t\t/* no if no delta */\n-\tint usable_delta =\t!entry->delta ? 0 :\n-\t\t\t\t/* yes if unlimited packfile */\n-\t\t\t\t!pack_size_limit ? 1 :\n-\t\t\t\t/* no if base written to previous pack */\n-\t\t\t\tentry->delta->idx.offset == (off_t)-1 ? 0 :\n-\t\t\t\t/* otherwise double-check written to this\n-\t\t\t\t * pack,  like we do below\n-\t\t\t\t */\n-\t\t\t\tentry->delta->idx.offset ? 1 : 0;\n+\tenum object_type type;\n+\tint usable_delta, to_reuse;\n \n \tif (!pack_to_stdout)\n \t\tcrc32_begin(f);\n \n-\tobj_type = entry->type;\n+\ttype = entry->type;\n+\n+\t/* write limit if limited packsize and not first object */\n+\tlimit = pack_size_limit && nr_written ?\n+\t\t\tpack_size_limit - write_offset : 0;\n+\n+\tif (!entry->delta) \n+\t\tusable_delta = 0;\t/* no delta */\n+\telse if (!pack_size_limit)\n+\t       usable_delta = 1;\t/* unlimited packfile */\n+\telse if (entry->delta->idx.offset == (off_t)-1)\n+\t\tusable_delta = 0;\t/* base was written to another pack */\n+\telse if(entry->delta->idx.offset)\n+\t\tusable_delta = 1;\t/* base already exists in this pack */\n+\telse\n+\t\tusable_delta = 0;\t/* base could end up in another pack */\n+\n \tif (!reuse_object)\n \t\tto_reuse = 0;\t/* explicit */\n \telse if (!entry->in_pack)\n \t\tto_reuse = 0;\t/* can't reuse what we don't have */\n-\telse if (obj_type == OBJ_REF_DELTA || obj_type == OBJ_OFS_DELTA)\n+\telse if (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA)\n \t\t\t\t/* check_object() decided it for us ... */\n \t\tto_reuse = usable_delta;\n \t\t\t\t/* ... but pack split may override that */\n-\telse if (obj_type != entry->in_pack_type)\n+\telse if (type != entry->in_pack_type)\n \t\tto_reuse = 0;\t/* pack has delta which is unusable */\n \telse if (entry->delta)\n \t\tto_reuse = 0;\t/* we want to pack afresh */\n@@ -274,19 +276,19 @@ static unsigned long write_object(struct sha1file *f,\n \t\tunsigned long maxsize;\n \t\tvoid *out;\n \t\tif (!usable_delta) {\n-\t\t\tbuf = read_sha1_file(entry->idx.sha1, &obj_type, &size);\n+\t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n \t\t\tif (!buf)\n \t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n \t\t} else if (entry->delta_data) {\n \t\t\tsize = entry->delta_size;\n \t\t\tbuf = entry->delta_data;\n \t\t\tentry->delta_data = NULL;\n-\t\t\tobj_type = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t\t} else {\n \t\t\tbuf = get_delta(entry);\n \t\t\tsize = entry->delta_size;\n-\t\t\tobj_type = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t\t}\n \t\t/* compress the data to store and put compressed length in datalen */\n@@ -308,9 +310,9 @@ static unsigned long write_object(struct sha1file *f,\n \t\t * The object header is a byte of 'type' followed by zero or\n \t\t * more bytes of length.\n \t\t */\n-\t\thdrlen = encode_header(obj_type, size, header);\n+\t\thdrlen = encode_header(type, size, header);\n \n-\t\tif (obj_type == OBJ_OFS_DELTA) {\n+\t\tif (type == OBJ_OFS_DELTA) {\n \t\t\t/*\n \t\t\t * Deltas with relative base contain an additional\n \t\t\t * encoding of the relative offset for the delta\n@@ -329,7 +331,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\tsha1write(f, header, hdrlen);\n \t\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n \t\t\thdrlen += sizeof(dheader) - pos;\n-\t\t} else if (obj_type == OBJ_REF_DELTA) {\n+\t\t} else if (type == OBJ_REF_DELTA) {\n \t\t\t/*\n \t\t\t * Deltas with a base reference contain\n \t\t\t * an additional 20 bytes for the base sha1.\n@@ -361,11 +363,11 @@ static unsigned long write_object(struct sha1file *f,\n \t\toff_t offset;\n \n \t\tif (entry->delta) {\n-\t\t\tobj_type = (allow_ofs_delta && entry->delta->idx.offset) ?\n+\t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t\t\treused_delta++;\n \t\t}\n-\t\thdrlen = encode_header(obj_type, entry->size, header);\n+\t\thdrlen = encode_header(type, entry->size, header);\n \t\toffset = entry->in_pack_offset;\n \t\trevidx = find_pack_revindex(p, offset);\n \t\tdatalen = revidx[1].offset - offset;\n@@ -374,7 +376,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\tdie(\"bad packed object CRC for %s\", sha1_to_hex(entry->idx.sha1));\n \t\toffset += entry->in_pack_header_size;\n \t\tdatalen -= entry->in_pack_header_size;\n-\t\tif (obj_type == OBJ_OFS_DELTA) {\n+\t\tif (type == OBJ_OFS_DELTA) {\n \t\t\toff_t ofs = entry->idx.offset - entry->delta->idx.offset;\n \t\t\tunsigned pos = sizeof(dheader) - 1;\n \t\t\tdheader[pos] = ofs & 127;\n@@ -385,7 +387,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\tsha1write(f, header, hdrlen);\n \t\t\tsha1write(f, dheader + pos, sizeof(dheader) - pos);\n \t\t\thdrlen += sizeof(dheader) - pos;\n-\t\t} else if (obj_type == OBJ_REF_DELTA) {\n+\t\t} else if (type == OBJ_REF_DELTA) {\n \t\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit)\n \t\t\t\treturn 0;\n \t\t\tsha1write(f, header, hdrlen);\n-- \n1.5.5.1.226.g6f6e8\n"},{"id":"75870","messageId":"1209755511-7840-6-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":"1209755511-7840-5-git-send-email-nico@cam.org","subject":"[PATCH 5/7] pack-objects: move compression code in a separate function","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:49Z","receivedAt":"2008-05-02T19:11:49Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"A later patch will make use of that code too.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n builtin-pack-objects.c |   53 ++++++++++++++++++++++++++---------------------\n 1 files changed, 29 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 257002a..4319e7b 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -124,6 +124,32 @@ static void *get_delta(struct object_entry *entry)\n \treturn delta_buf;\n }\n \n+static unsigned long do_compress(void **pptr, unsigned long size)\n+{\n+\tz_stream stream;\n+\tvoid *in, *out;\n+\tunsigned long maxsize;\n+\n+\tmemset(&stream, 0, sizeof(stream));\n+\tdeflateInit(&stream, pack_compression_level);\n+\tmaxsize = deflateBound(&stream, size);\n+\n+\tin = *pptr;\n+\tout = xmalloc(maxsize);\n+\t*pptr = out;\n+\n+\tstream.next_in = in;\n+\tstream.avail_in = size;\n+\tstream.next_out = out;\n+\tstream.avail_out = maxsize;\n+\twhile (deflate(&stream, Z_FINISH) == Z_OK)\n+\t\t/* nothing */;\n+\tdeflateEnd(&stream);\n+\n+\tfree(in);\n+\treturn stream.total_out;\n+}\n+\n /*\n  * The per-object header is a pretty dense thing, which is\n  *  - first byte: low four bits are \"size\", then three bits of \"type\",\n@@ -226,11 +252,10 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t  struct object_entry *entry,\n \t\t\t\t  off_t write_offset)\n {\n-\tunsigned long size, limit;\n+\tunsigned long size, limit, datalen;\n \tvoid *buf;\n \tunsigned char header[10], dheader[10];\n \tunsigned hdrlen;\n-\toff_t datalen;\n \tenum object_type type;\n \tint usable_delta, to_reuse;\n \n@@ -272,9 +297,6 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t */\n \n \tif (!to_reuse) {\n-\t\tz_stream stream;\n-\t\tunsigned long maxsize;\n-\t\tvoid *out;\n \t\tif (!usable_delta) {\n \t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n \t\t\tif (!buf)\n@@ -291,20 +313,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t\t}\n-\t\t/* compress the data to store and put compressed length in datalen */\n-\t\tmemset(&stream, 0, sizeof(stream));\n-\t\tdeflateInit(&stream, pack_compression_level);\n-\t\tmaxsize = deflateBound(&stream, size);\n-\t\tout = xmalloc(maxsize);\n-\t\t/* Compress it */\n-\t\tstream.next_in = buf;\n-\t\tstream.avail_in = size;\n-\t\tstream.next_out = out;\n-\t\tstream.avail_out = maxsize;\n-\t\twhile (deflate(&stream, Z_FINISH) == Z_OK)\n-\t\t\t/* nothing */;\n-\t\tdeflateEnd(&stream);\n-\t\tdatalen = stream.total_out;\n+\t\tdatalen = do_compress(&buf, size);\n \n \t\t/*\n \t\t * The object header is a byte of 'type' followed by zero or\n@@ -324,7 +333,6 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\twhile (ofs >>= 7)\n \t\t\t\tdheader[--pos] = 128 | (--ofs & 127);\n \t\t\tif (limit && hdrlen + sizeof(dheader) - pos + datalen + 20 >= limit) {\n-\t\t\t\tfree(out);\n \t\t\t\tfree(buf);\n \t\t\t\treturn 0;\n \t\t\t}\n@@ -337,7 +345,6 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t * an additional 20 bytes for the base sha1.\n \t\t\t */\n \t\t\tif (limit && hdrlen + 20 + datalen + 20 >= limit) {\n-\t\t\t\tfree(out);\n \t\t\t\tfree(buf);\n \t\t\t\treturn 0;\n \t\t\t}\n@@ -346,14 +353,12 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\thdrlen += 20;\n \t\t} else {\n \t\t\tif (limit && hdrlen + datalen + 20 >= limit) {\n-\t\t\t\tfree(out);\n \t\t\t\tfree(buf);\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\tsha1write(f, header, hdrlen);\n \t\t}\n-\t\tsha1write(f, out, datalen);\n-\t\tfree(out);\n+\t\tsha1write(f, buf, datalen);\n \t\tfree(buf);\n \t}\n \telse {\n-- \n1.5.5.1.226.g6f6e8\n"},{"id":"75868","messageId":"1209755511-7840-7-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":"1209755511-7840-6-git-send-email-nico@cam.org","subject":"[PATCH 6/7] pack-objects: allow for early delta deflating","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:50Z","receivedAt":"2008-05-02T19:11:50Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"When the delta data is cached in memory until it is written to a pack\nfile on disk, it is best to compress it right away in find_deltas() for\nthe following reasons:\n\n  - we have to compress that data anyway;\n\n  - this allows for caching more deltas with the same cache size limit;\n\n  - compression is potentially threaded.\n\nThis last point is especially relevant for SMP run time.  For example,\nrepacking the Linux repo on a quad core processor using 4 threads with\nall default settings produce the following results before this change:\n\n\treal    2m27.929s\n\tuser    4m36.492s\n\tsys     0m3.091s\n\nAnd with this change applied:\n\n\treal    2m13.787s\n\tuser    4m37.486s\n\tsys     0m3.159s\n\nSo the actual execution time stayed more or less the same but the\nwall clock time is shorter.\n\nThis is however not a good thing to do when generating a pack for\nnetwork transmission.  In that case, the network is most likely to\nthrottle the data throughput, so it is best to make find_deltas()\nfaster in order to start writing data ASAP since we can afford\nspending more time between writes to compress the data\nat that point.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n builtin-pack-objects.c |   35 ++++++++++++++++++++++++++++++++++-\n 1 files changed, 34 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 4319e7b..f5dcd20 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -43,6 +43,7 @@ struct object_entry {\n \t\t\t\t\t     */\n \tvoid *delta_data;\t/* cached delta (uncompressed) */\n \tunsigned long delta_size;\t/* delta data size (uncompressed) */\n+\tunsigned long z_delta_size;\t/* delta data size (compressed) */\n \tunsigned int hash;\t/* name hint hash */\n \tenum object_type type;\n \tenum object_type in_pack_type;\t/* could be delta */\n@@ -301,6 +302,11 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\tbuf = read_sha1_file(entry->idx.sha1, &type, &size);\n \t\t\tif (!buf)\n \t\t\t\tdie(\"unable to read %s\", sha1_to_hex(entry->idx.sha1));\n+\t\t\t/* make sure no cached delta data remains from a\n+\t\t\t   previous attempt before a pack split occured */\n+\t\t\tfree(entry->delta_data);\n+\t\t\tentry->delta_data = NULL;\n+\t\t\tentry->z_delta_size = 0;\n \t\t} else if (entry->delta_data) {\n \t\t\tsize = entry->delta_size;\n \t\t\tbuf = entry->delta_data;\n@@ -313,7 +319,11 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\ttype = (allow_ofs_delta && entry->delta->idx.offset) ?\n \t\t\t\tOBJ_OFS_DELTA : OBJ_REF_DELTA;\n \t\t}\n-\t\tdatalen = do_compress(&buf, size);\n+\n+\t\tif (entry->z_delta_size)\n+\t\t\tdatalen = entry->z_delta_size;\n+\t\telse\n+\t\t\tdatalen = do_compress(&buf, size);\n \n \t\t/*\n \t\t * The object header is a byte of 'type' followed by zero or\n@@ -1447,6 +1457,29 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t\t\tbest_base = other_idx;\n \t\t}\n \n+\t\t/*\n+\t\t * If we decided to cache the delta data, then it is best\n+\t\t * to compress it right away.  First because we have to do\n+\t\t * it anyway, and doing it here while we're threaded will\n+\t\t * save a lot of time in the non threaded write phase,\n+\t\t * as well as allow for caching more deltas within\n+\t\t * the same cache size limit.\n+\t\t * ...\n+\t\t * But only if not writing to stdout, since in that case\n+\t\t * the network is most likely throttling writes anyway,\n+\t\t * and therefore it is best to go to the write phase ASAP\n+\t\t * instead, as we can afford spending more time compressing\n+\t\t * between writes at that moment.\n+\t\t */\n+\t\tif (entry->delta_data && !pack_to_stdout) {\n+\t\t\tentry->z_delta_size = do_compress(&entry->delta_data,\n+\t\t\t\t\t\t\t  entry->delta_size);\n+\t\t\tcache_lock();\n+\t\t\tdelta_cache_size -= entry->delta_size;\n+\t\t\tdelta_cache_size += entry->z_delta_size;\n+\t\t\tcache_unlock();\n+\t\t}\n+\n \t\t/* if we made n a delta, and if n is already at max\n \t\t * depth, leaving it in the window is pointless.  we\n \t\t * should evict it first.\n-- \n1.5.5.1.226.g6f6e8\n"},{"id":"75866","messageId":"1209755511-7840-8-git-send-email-nico@cam.org","threadId":"13357","inReplyTo":"1209755511-7840-7-git-send-email-nico@cam.org","subject":"[PATCH 7/7] pack-objects: fix early eviction for max depth delta objects","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T19:11:51Z","receivedAt":"2008-05-02T19:11:51Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"The 'depth' variable doesn't reflect the actual maximum depth used\nwhen other objects already depend on the current one.\n\nSigned-off-by: Nicolas Pitre <nico@cam.org>\n---\n builtin-pack-objects.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex f5dcd20..94c9875 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1484,7 +1484,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,\n \t\t * depth, leaving it in the window is pointless.  we\n \t\t * should evict it first.\n \t\t */\n-\t\tif (entry->delta && depth <= n->depth)\n+\t\tif (entry->delta && max_depth <= n->depth)\n \t\t\tcontinue;\n \n \t\t/*\n-- \n1.5.5.1.226.g6f6e8\n"},{"id":"75885","messageId":"481B9963.6050605@gmail.com","threadId":"13357","inReplyTo":"1209755511-7840-7-git-send-email-nico@cam.org","subject":"Re: [PATCH 6/7] pack-objects: allow for early delta deflating","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2008-05-02T22:44:51Z","receivedAt":"2008-05-02T22:44:51Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Nicolas Pitre wrote:\n> When the delta data is cached in memory until it is written to a pack\n> file on disk, it is best to compress it right away in find_deltas() for\n> the following reasons:\n> \n>   - we have to compress that data anyway;\n> \n>   - this allows for caching more deltas with the same cache size limit;\n> \n>   - compression is potentially threaded.\n> \n> This last point is especially relevant for SMP run time.  For example,\n> repacking the Linux repo on a quad core processor using 4 threads with\n> all default settings produce the following results before this change:\n> \n> \treal    2m27.929s\n> \tuser    4m36.492s\n> \tsys     0m3.091s\n> \n> And with this change applied:\n> \n> \treal    2m13.787s\n> \tuser    4m37.486s\n> \tsys     0m3.159s\n> \n> So the actual execution time stayed more or less the same but the\n> wall clock time is shorter.\n> \n> This is however not a good thing to do when generating a pack for\n> network transmission.  In that case, the network is most likely to\n> throttle the data throughput, so it is best to make find_deltas()\n> faster in order to start writing data ASAP since we can afford\n> spending more time between writes to compress the data\n> at that point.\n\n[...]\n\n>  \n> +\t\t/*\n> +\t\t * If we decided to cache the delta data, then it is best\n> +\t\t * to compress it right away.  First because we have to do\n> +\t\t * it anyway, and doing it here while we're threaded will\n> +\t\t * save a lot of time in the non threaded write phase,\n> +\t\t * as well as allow for caching more deltas within\n> +\t\t * the same cache size limit.\n> +\t\t * ...\n> +\t\t * But only if not writing to stdout, since in that case\n> +\t\t * the network is most likely throttling writes anyway,\n> +\t\t * and therefore it is best to go to the write phase ASAP\n> +\t\t * instead, as we can afford spending more time compressing\n> +\t\t * between writes at that moment.\n> +\t\t */\n> +\t\tif (entry->delta_data && !pack_to_stdout) {\n> +\t\t\tentry->z_delta_size = do_compress(&entry->delta_data,\n> +\t\t\t\t\t\t\t  entry->delta_size);\n> +\t\t\tcache_lock();\n> +\t\t\tdelta_cache_size -= entry->delta_size;\n> +\t\t\tdelta_cache_size += entry->z_delta_size;\n> +\t\t\tcache_unlock();\n> +\t\t}\n> +\n>  \t\t/* if we made n a delta, and if n is already at max\n>  \t\t * depth, leaving it in the window is pointless.  we\n>  \t\t * should evict it first.\n\nAlthough I like the idea of changing the behavior if the output is \nlikely to be throttled, I do not like the test for that condition being \n\"is it going to stdout\". This is something better suited to a command \nline argument.\n"},{"id":"75886","messageId":"alpine.LFD.1.10.0805021853410.23581@xanadu.home","threadId":"13357","inReplyTo":"481B9963.6050605@gmail.com","subject":"Re: [PATCH 6/7] pack-objects: allow for early delta deflating","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-05-02T23:03:30Z","receivedAt":"2008-05-02T23:03:30Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 2 May 2008, A Large Angry SCM wrote:\n\n> Although I like the idea of changing the behavior if the output is likely to\n> be throttled, I do not like the test for that condition being \"is it going to\n> stdout\". This is something better suited to a command line argument.\n\nThis is a common assumptions for stdio to go over the network, and other \nbehaviors are already adjusted accordingly.  And not only in \npack-objects but in index-pack too.  So unless you have a really \ncompelling reason to override this, I don't think it is worth having \nanother command line argument just for the sake of it.\n\n\nNicolas\n"}]}