{"thread":{"id":"7130","subject":"[PATCH 6/9] Use uint32_t for pack-objects counters.","startedAt":"2007-03-07T01:44:24Z","lastAt":"2007-03-07T21:52:09Z","messageCount":4,"participants":["Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"36520","messageId":"20070307014424.GG26482@spearce.org","threadId":"7130","inReplyTo":null,"subject":"[PATCH 6/9] Use uint32_t for pack-objects counters.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-03-07T01:44:24Z","receivedAt":"2007-03-07T01:44:24Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"As we technically try to support up to a maximum of 2**32-1 objects\nin a single packfile we should act like it and use unsigned 32 bit\nintegers for all of our object counts and progress output.\n\nThis change does not modify everything in pack-objects that probably\nneeds to change to fully support the maximum of 2**32-1 objects.\nI'm intentionally breaking the improvements into slightly smaller\ncommits to make them easier to follow.\n\nNo logic change should be occuring here, with the exception that\nsome comparsions will now work properly when the number of objects\nexceeds 2**31-1.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n builtin-pack-objects.c |   63 ++++++++++++++++++++++-------------------------\n 1 files changed, 30 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 8cf24f4..8d5c3f1 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -68,7 +68,7 @@ static int allow_ofs_delta;\n \n static struct object_entry **sorted_by_sha, **sorted_by_type;\n static struct object_entry *objects;\n-static int nr_objects, nr_alloc, nr_result;\n+static uint32_t nr_objects, nr_alloc, nr_result;\n static const char *base_name;\n static unsigned char pack_file_sha1[20];\n static int progress = 1;\n@@ -114,10 +114,8 @@ static int pack_revindex_hashsz;\n /*\n  * stats\n  */\n-static int written;\n-static int written_delta;\n-static int reused;\n-static int reused_delta;\n+static uint32_t written, written_delta;\n+static uint32_t reused, reused_delta;\n \n static int pack_revindex_ix(struct packed_git *p)\n {\n@@ -518,7 +516,7 @@ static unsigned long write_one(struct sha1file *f,\n \n static void write_pack_file(void)\n {\n-\tint i;\n+\tuint32_t i;\n \tstruct sha1file *f;\n \tunsigned long offset;\n \tstruct pack_header hdr;\n@@ -533,7 +531,7 @@ static void write_pack_file(void)\n \t\tf = sha1create(\"%s-%s.%s\", base_name,\n \t\t\t       sha1_to_hex(object_list_sha1), \"pack\");\n \tif (do_progress)\n-\t\tfprintf(stderr, \"Writing %d objects.\\n\", nr_result);\n+\t\tfprintf(stderr, \"Writing %u objects.\\n\", nr_result);\n \n \thdr.hdr_signature = htonl(PACK_SIGNATURE);\n \thdr.hdr_version = htonl(PACK_VERSION);\n@@ -558,13 +556,13 @@ static void write_pack_file(void)\n \t\tfputc('\\n', stderr);\n  done:\n \tif (written != nr_result)\n-\t\tdie(\"wrote %d objects while expecting %d\", written, nr_result);\n+\t\tdie(\"wrote %u objects while expecting %u\", written, nr_result);\n \tsha1close(f, pack_file_sha1, 1);\n }\n \n static void write_index_file(void)\n {\n-\tint i;\n+\tuint32_t i;\n \tstruct sha1file *f = sha1create(\"%s-%s.%s\", base_name,\n \t\t\t\t\tsha1_to_hex(object_list_sha1), \"idx\");\n \tstruct object_entry **list = sorted_by_sha;\n@@ -633,7 +631,7 @@ static struct object_entry *locate_object_entry(const unsigned char *sha1)\n \n static void rehash_objects(void)\n {\n-\tint i;\n+\tuint32_t i;\n \tstruct object_entry *oe;\n \n \tobject_ix_hashsz = nr_objects * 3;\n@@ -670,7 +668,7 @@ static unsigned name_hash(const char *name)\n \n static int add_object_entry(const unsigned char *sha1, unsigned hash, int exclude)\n {\n-\tunsigned int idx = nr_objects;\n+\tuint32_t idx = nr_objects;\n \tstruct object_entry *entry;\n \tstruct packed_git *p;\n \tunsigned int found_offset = 0;\n@@ -696,9 +694,8 @@ static int add_object_entry(const unsigned char *sha1, unsigned hash, int exclud\n \t\tgoto already_added;\n \n \tif (idx >= nr_alloc) {\n-\t\tunsigned int needed = (idx + 1024) * 3 / 2;\n-\t\tobjects = xrealloc(objects, needed * sizeof(*entry));\n-\t\tnr_alloc = needed;\n+\t\tnr_alloc = (idx + 1024) * 3 / 2;\n+\t\tobjects = xrealloc(objects, nr_alloc * sizeof(*entry));\n \t}\n \tentry = objects + idx;\n \tnr_objects = idx + 1;\n@@ -718,7 +715,7 @@ static int add_object_entry(const unsigned char *sha1, unsigned hash, int exclud\n \n  already_added:\n \tif (progress_update) {\n-\t\tfprintf(stderr, \"Counting objects...%d\\r\", nr_objects);\n+\t\tfprintf(stderr, \"Counting objects...%u\\r\", nr_objects);\n \t\tprogress_update = 0;\n \t}\n \tif (exclude)\n@@ -1081,7 +1078,7 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)\n \n static void get_object_details(void)\n {\n-\tint i;\n+\tuint32_t i;\n \tstruct object_entry *entry;\n \n \tprepare_pack_ix();\n@@ -1120,7 +1117,7 @@ static int sort_comparator(const void *_a, const void *_b)\n static struct object_entry **create_sorted_list(entry_sort_t sort)\n {\n \tstruct object_entry **list = xmalloc(nr_objects * sizeof(struct object_entry *));\n-\tint i;\n+\tuint32_t i;\n \n \tfor (i = 0; i < nr_objects; i++)\n \t\tlist[i] = objects + i;\n@@ -1137,7 +1134,7 @@ static int sha1_sort(const struct object_entry *a, const struct object_entry *b)\n static struct object_entry **create_final_object_list(void)\n {\n \tstruct object_entry **list;\n-\tint i, j;\n+\tuint32_t i, j;\n \n \tfor (i = nr_result = 0; i < nr_objects; i++)\n \t\tif (!objects[i].preferred_base)\n@@ -1279,20 +1276,20 @@ static void progress_interval(int signum)\n \n static void find_deltas(struct object_entry **list, int window, int depth)\n {\n-\tint i, idx;\n+\tuint32_t i = nr_objects, idx = 0, processed = 0;\n \tunsigned int array_size = window * sizeof(struct unpacked);\n-\tstruct unpacked *array = xmalloc(array_size);\n-\tunsigned processed = 0;\n+\tstruct unpacked *array;\n \tunsigned last_percent = 999;\n \n+\tif (!nr_objects)\n+\t\treturn;\n+\tarray = xmalloc(array_size);\n \tmemset(array, 0, array_size);\n-\ti = nr_objects;\n-\tidx = 0;\n \tif (progress)\n-\t\tfprintf(stderr, \"Deltifying %d objects.\\n\", nr_result);\n+\t\tfprintf(stderr, \"Deltifying %u objects.\\n\", nr_result);\n \n-\twhile (--i >= 0) {\n-\t\tstruct object_entry *entry = list[i];\n+\tdo {\n+\t\tstruct object_entry *entry = list[--i];\n \t\tstruct unpacked *n = array + idx;\n \t\tint j;\n \n@@ -1325,7 +1322,7 @@ static void find_deltas(struct object_entry **list, int window, int depth)\n \n \t\tj = window;\n \t\twhile (--j > 0) {\n-\t\t\tunsigned int other_idx = idx + j;\n+\t\t\tuint32_t other_idx = idx + j;\n \t\t\tstruct unpacked *m;\n \t\t\tif (other_idx >= window)\n \t\t\t\tother_idx -= window;\n@@ -1345,7 +1342,7 @@ static void find_deltas(struct object_entry **list, int window, int depth)\n \t\tidx++;\n \t\tif (idx >= window)\n \t\t\tidx = 0;\n-\t}\n+\t} while (i > 0);\n \n \tif (progress)\n \t\tfputc('\\n', stderr);\n@@ -1386,7 +1383,7 @@ static int reuse_cached_pack(unsigned char *sha1)\n \t}\n \n \tif (progress)\n-\t\tfprintf(stderr, \"Reusing %d objects pack %s\\n\", nr_objects,\n+\t\tfprintf(stderr, \"Reusing %u objects pack %s\\n\", nr_objects,\n \t\t\tsha1_to_hex(sha1));\n \n \tif (pack_to_stdout) {\n@@ -1537,7 +1534,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tstruct object_entry **list;\n \tint use_internal_rev_list = 0;\n \tint thin = 0;\n-\tint i;\n+\tuint32_t i;\n \tconst char **rp_av;\n \tint rp_ac_alloc = 64;\n \tint rp_ac;\n@@ -1670,7 +1667,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (progress)\n-\t\tfprintf(stderr, \"Done counting %d objects.\\n\", nr_objects);\n+\t\tfprintf(stderr, \"Done counting %u objects.\\n\", nr_objects);\n \tsorted_by_sha = create_final_object_list();\n \tif (non_empty && !nr_result)\n \t\treturn 0;\n@@ -1683,7 +1680,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t}\n \tSHA1_Final(object_list_sha1, &ctx);\n \tif (progress && (nr_objects != nr_result))\n-\t\tfprintf(stderr, \"Result has %d objects.\\n\", nr_result);\n+\t\tfprintf(stderr, \"Result has %u objects.\\n\", nr_result);\n \n \tif (reuse_cached_pack(object_list_sha1))\n \t\t;\n@@ -1704,7 +1701,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \tif (progress)\n-\t\tfprintf(stderr, \"Total %d (delta %d), reused %d (delta %d)\\n\",\n+\t\tfprintf(stderr, \"Total %u (delta %u), reused %u (delta %u)\\n\",\n \t\t\twritten, written_delta, reused, reused_delta);\n \treturn 0;\n }\n-- \n1.5.0.3.863.gf0989\n"},{"id":"36566","messageId":"7v1wk0g9mp.fsf@assigned-by-dhcp.cox.net","threadId":"7130","inReplyTo":"20070307014424.GG26482@spearce.org","subject":"Re: [PATCH 6/9] Use uint32_t for pack-objects counters.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-07T19:02:22Z","receivedAt":"2007-03-07T19:02:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> No logic change should be occuring here, with the exception that\n> some comparsions will now work properly when the number of objects\n> exceeds 2**31-1.\n> ...\n>  static void find_deltas(struct object_entry **list, int window, int depth)\n>  {\n> -\tint i, idx;\n> +\tuint32_t i = nr_objects, idx = 0, processed = 0;\n>  \tunsigned int array_size = window * sizeof(struct unpacked);\n> -\tstruct unpacked *array = xmalloc(array_size);\n> -\tunsigned processed = 0;\n> +\tstruct unpacked *array;\n>  \tunsigned last_percent = 999;\n>  \n> +\tif (!nr_objects)\n> +\t\treturn;\n> +\tarray = xmalloc(array_size);\n>  \tmemset(array, 0, array_size);\n> -\ti = nr_objects;\n> -\tidx = 0;\n>  \tif (progress)\n> -\t\tfprintf(stderr, \"Deltifying %d objects.\\n\", nr_result);\n> +\t\tfprintf(stderr, \"Deltifying %u objects.\\n\", nr_result);\n>  \n> -\twhile (--i >= 0) {\n> -\t\tstruct object_entry *entry = list[i];\n> +\tdo {\n> +\t\tstruct object_entry *entry = list[--i];\n>  \t\tstruct unpacked *n = array + idx;\n>  \t\tint j;\n> ...  \n> @@ -1345,7 +1342,7 @@ static void find_deltas(struct object_entry **list, int window, int depth)\n>  \t\tidx++;\n>  \t\tif (idx >= window)\n>  \t\t\tidx = 0;\n> -\t}\n> +\t} while (i > 0);\n\nI may not be reading the patch correctly, but doesn't this make\nfind_deltas() logic to scan the list backwards?  We sort the\nlist by type and size before we enter this, so that we scan from\nbigger to smaller, because we prefer to leave biggest as a base\nand represent smaller ones as deltas (removing deltas do not\nhave to record preimage and tend to be much smaller than adding\ndeltas).  I suspect this change may affect that optimization.\n"},{"id":"36569","messageId":"20070307203721.GD27922@spearce.org","threadId":"7130","inReplyTo":"7v1wk0g9mp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 6/9] Use uint32_t for pack-objects counters.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-03-07T20:37:21Z","receivedAt":"2007-03-07T20:37:21Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> I may not be reading the patch correctly, but doesn't this make\n> find_deltas() logic to scan the list backwards?  We sort the\n> list by type and size before we enter this, so that we scan from\n> bigger to smaller, because we prefer to leave biggest as a base\n> and represent smaller ones as deltas (removing deltas do not\n> have to record preimage and tend to be much smaller than adding\n> deltas).  I suspect this change may affect that optimization.\n\nIts always scanned backwards.  I've read the patch like 8 times\nnow (and I just reread the part you quoted).  The patch implements\nthe same backwards scan that has always been there.  Linus put it\nbackwards for the exact reason you state; bigger objects are near\nthe end of the array (see type_size_sort).\n\n-- \nShawn.\n"},{"id":"36575","messageId":"7vodn4en7a.fsf@assigned-by-dhcp.cox.net","threadId":"7130","inReplyTo":"20070307203721.GD27922@spearce.org","subject":"Re: [PATCH 6/9] Use uint32_t for pack-objects counters.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-07T21:52:09Z","receivedAt":"2007-03-07T21:52:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> Its always scanned backwards.  I've read the patch like 8 times\n> now (and I just reread the part you quoted).  The patch implements\n> the same backwards scan that has always been there.\n\nSorry, false alarm.  Yes we scanned from larger to smaller and\nyou still do.\n"}]}