{"thread":{"id":"36167","subject":"[PATCH 0/2] pack-bitmap progress meters","startedAt":"2014-03-15T02:25:51Z","lastAt":"2014-03-15T02:26:58Z","messageCount":3,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"236771","messageId":"20140315022550.GA875@sigill.intra.peff.net","threadId":"36167","inReplyTo":null,"subject":"[PATCH 0/2] pack-bitmap progress meters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-15T02:25:51Z","receivedAt":"2014-03-15T02:25:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here are patches to make the pack-objects progress meters behave the\nsame both with and without pack reuse. The first one is basically the\npatch I posted earlier.\n\nThe second one drops the \"Reusing existing pack\", and just rolls those\nnumbers into \"Counting objects\". I have mixed feelings on it. _I_ find\nit interesting to know whether the pack was reused. But then I am often\ndebugging bitmaps and packfiles. :) From a regular user's perspective,\nit is an implementation detail that may not be so interesting (git\nshould just magically be faster, and they do not have to care why).\n\nSo I dunno. Opinions welcome.\n\n  [1/2]: pack-objects: show progress for reused packfiles\n  [2/2]: pack-objects: show reused packfile objects in \"Counting objects\"\n\n-Peff\n"},{"id":"236772","messageId":"20140315022620.GA987@sigill.intra.peff.net","threadId":"36167","inReplyTo":"20140315022550.GA875@sigill.intra.peff.net","subject":"[PATCH 1/2] pack-objects: show progress for reused packfiles","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-15T02:26:21Z","receivedAt":"2014-03-15T02:26:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When the \"--all-progress\" option is in effect, pack-objects\nshows a progress report for the \"writing\" phase. If the\nrepository has bitmaps and we are reusing a packfile, the\nuser sees no progress update until the whole packfile is\nsent.  Since this is typically the bulk of what is being\nwritten, it can look like git hangs during this phase, even\nthough the transfer is proceeding.\n\nThis generally only happens with \"git push\" from a\nrepository with bitmaps. We do not use \"--all-progress\" for\nfetch (since the result is going to index-pack on the\nclient, which takes care of progress reporting). And for\nregular repacks to disk, we do not reuse packfiles.\n\nWe already have the progress meter setup during\nwrite_reused_pack; we just need to call display_progress\nwhiel we are writing out the pack. The progress meter is\nattached to our output descriptor, so it automatically\nhandles the throughput measurements.\n\nHowever, we need to update the object count as we go, since\nthat is what feeds the percentage we show. We aren't\nactually parsing the packfile as we send it, so we have no\nidea how many objects we have sent; we only know that at the\nend of N bytes, we will have sent M objects. So we cheat a\nlittle and assume each object is M/N bytes (i.e., the mean\nof the objects we are sending). While this isn't strictly\ntrue, it actually produces a more pleasing progress meter\nfor the user, as it moves smoothly and predictably (and\nnobody really cares about the object count; they care about\nthe percentage, and the object count is a proxy for that).\n\nOne alternative would be to actually show two progress\nmeters: one for the reused pack, and one for the rest of the\nobjects. That would more closely reflect the data we have\n(the first would be measured in bytes, and the second\nmeasured in objects). But it would also be more complex and\nannoying to the user; rather than seeing one progress meter\ncounting up to 100%, they would finish one meter, then start\nanother one at zero.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/pack-objects.c | 19 ++++++++++++++++---\n 1 file changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex f2365a4..12aa94c 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -708,7 +708,7 @@ static struct object_entry **compute_write_order(void)\n static off_t write_reused_pack(struct sha1file *f)\n {\n \tunsigned char buffer[8192];\n-\toff_t to_write;\n+\toff_t to_write, total;\n \tint fd;\n \n \tif (!is_pack_valid(reuse_packfile))\n@@ -725,7 +725,7 @@ static off_t write_reused_pack(struct sha1file *f)\n \tif (reuse_packfile_offset < 0)\n \t\treuse_packfile_offset = reuse_packfile->pack_size - 20;\n \n-\tto_write = reuse_packfile_offset - sizeof(struct pack_header);\n+\ttotal = to_write = reuse_packfile_offset - sizeof(struct pack_header);\n \n \twhile (to_write) {\n \t\tint read_pack = xread(fd, buffer, sizeof(buffer));\n@@ -738,10 +738,23 @@ static off_t write_reused_pack(struct sha1file *f)\n \n \t\tsha1write(f, buffer, read_pack);\n \t\tto_write -= read_pack;\n+\n+\t\t/*\n+\t\t * We don't know the actual number of objects written,\n+\t\t * only how many bytes written, how many bytes total, and\n+\t\t * how many objects total. So we can fake it by pretending all\n+\t\t * objects we are writing are the same size. This gives us a\n+\t\t * smooth progress meter, and at the end it matches the true\n+\t\t * answer.\n+\t\t */\n+\t\twritten = reuse_packfile_objects *\n+\t\t\t\t(((double)(total - to_write)) / total);\n+\t\tdisplay_progress(progress_state, written);\n \t}\n \n \tclose(fd);\n-\twritten += reuse_packfile_objects;\n+\twritten = reuse_packfile_objects;\n+\tdisplay_progress(progress_state, written);\n \treturn reuse_packfile_offset - sizeof(struct pack_header);\n }\n \n-- \n1.9.0.417.gc6bea4f\n"},{"id":"236773","messageId":"20140315022658.GB987@sigill.intra.peff.net","threadId":"36167","inReplyTo":"20140315022550.GA875@sigill.intra.peff.net","subject":"[PATCH 2/2] pack-objects: show reused packfile objects in \"Counting objects\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-15T02:26:58Z","receivedAt":"2014-03-15T02:26:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we are sending a pack for push or fetch, we may reuse a\nchunk of packfile without even parsing it. The progress\nmeter then looks like this:\n\n  Reusing existing pack: 3440489, done.\n  Counting objects: 3, done.\n\nThe first line shows that we are reusing a large chunk of\nobjects, and then we further count any objects not included\nin the reused portion with an actual traversal.\n\nThese are all implementation details that the user does not\nneed to care about. Instead, we can show the reused objects\nin the normal \"counting...\" progress meter (which will\nsimply go much faster than normal), and then continue to add\nto it as we traverse.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/pack-objects.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 12aa94c..4ca3946 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1038,7 +1038,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,\n \t\t\t    exclude, name && no_try_delta(name),\n \t\t\t    index_pos, found_pack, found_offset);\n \n-\tdisplay_progress(progress_state, to_pack.nr_objects);\n+\tdisplay_progress(progress_state, nr_result);\n \treturn 1;\n }\n \n@@ -1054,7 +1054,7 @@ static int add_object_entry_from_bitmap(const unsigned char *sha1,\n \n \tcreate_object_entry(sha1, type, name_hash, 0, 0, index_pos, pack, offset);\n \n-\tdisplay_progress(progress_state, to_pack.nr_objects);\n+\tdisplay_progress(progress_state, nr_result);\n \treturn 1;\n }\n \n@@ -2456,12 +2456,7 @@ static int get_object_list_from_bitmap(struct rev_info *revs)\n \t\t\t&reuse_packfile_offset)) {\n \t\tassert(reuse_packfile_objects);\n \t\tnr_result += reuse_packfile_objects;\n-\n-\t\tif (progress) {\n-\t\t\tfprintf(stderr, \"Reusing existing pack: %d, done.\\n\",\n-\t\t\t\treuse_packfile_objects);\n-\t\t\tfflush(stderr);\n-\t\t}\n+\t\tdisplay_progress(progress_state, nr_result);\n \t}\n \n \ttraverse_bitmap_commit_list(&add_object_entry_from_bitmap);\n-- \n1.9.0.417.gc6bea4f\n"}]}