{"thread":{"id":"33207","subject":"[PATCH 0/2] minor fast-export speedup","startedAt":"2013-03-17T08:32:35Z","lastAt":"2013-03-17T08:38:57Z","messageCount":3,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"211505","messageId":"20130317083235.GA29907@sigill.intra.peff.net","threadId":"33207","inReplyTo":null,"subject":"[PATCH 0/2] minor fast-export speedup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-17T08:32:35Z","receivedAt":"2013-03-17T08:32:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"While grepping through all of the calls to parse_object (to see how they\nhandled error conditions, for the other series I just posted), I noticed\nthis opportunity for a small speedup in fast-export (5-15%). The first\npatch is a cleanup, the second is the interesting bit.\n\n  [1/2]: fast-export: rename handle_object function\n  [2/2]: fast-export: do not load blob objects twice\n\nA useful third patch on top might be to stream blobs out rather than\nload them into memory, but I didn't want to go there tonight.\n\n-Peff\n"},{"id":"211506","messageId":"20130317083312.GA30183@sigill.intra.peff.net","threadId":"33207","inReplyTo":"20130317083235.GA29907@sigill.intra.peff.net","subject":"[PATCH 1/2] fast-export: rename handle_object function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-17T08:33:12Z","receivedAt":"2013-03-17T08:33:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The handle_object function is rather vaguely named; it only\noperates on blobs, and its purpose is to export the blob to\nthe output stream. Let's call it \"export_blob\" to make it\nmore clear what it does.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/fast-export.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 77dffd1..3eba852 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -113,7 +113,7 @@ static void show_progress(void)\n \t\tprintf(\"progress %d objects\\n\", counter);\n }\n \n-static void handle_object(const unsigned char *sha1)\n+static void export_blob(const unsigned char *sha1)\n {\n \tunsigned long size;\n \tenum object_type type;\n@@ -312,7 +312,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)\n \t/* Export the referenced blobs, and remember the marks. */\n \tfor (i = 0; i < diff_queued_diff.nr; i++)\n \t\tif (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode))\n-\t\t\thandle_object(diff_queued_diff.queue[i]->two->sha1);\n+\t\t\texport_blob(diff_queued_diff.queue[i]->two->sha1);\n \n \tmark_next_object(&commit->object);\n \tif (!is_encoding_utf8(encoding))\n@@ -512,7 +512,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info,\n \t\t\t\tcommit = (struct commit *)tag;\n \t\t\t\tbreak;\n \t\t\tcase OBJ_BLOB:\n-\t\t\t\thandle_object(tag->object.sha1);\n+\t\t\t\texport_blob(tag->object.sha1);\n \t\t\t\tcontinue;\n \t\t\tdefault: /* OBJ_TAG (nested tags) is already handled */\n \t\t\t\twarning(\"Tag points to object of unexpected type %s, skipping.\",\n-- \n1.8.2.rc2.7.gef06216\n"},{"id":"211507","messageId":"20130317083855.GB30183@sigill.intra.peff.net","threadId":"33207","inReplyTo":"20130317083235.GA29907@sigill.intra.peff.net","subject":"[PATCH 2/2] fast-export: do not load blob objects twice","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-17T08:38:57Z","receivedAt":"2013-03-17T08:38:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When fast-export wants to export a blob object, it first\ncalls parse_object to get a \"struct object\" and check\nwhether we have already shown the object.  If we haven't\nshown it, we then use read_sha1_file to pull it from disk\nand write it out.\n\nThat means we load each blob from disk twice: once for\nparse_object to find its type and check its sha1, and a\nsecond time when we actually output it. We can drop this to\na single load by using lookup_object to check the SHOWN\nflag, and then checking the signature on and outputting a\nsingle buffer.\n\nThis provides modest speedups on git.git (best-of-five, \"git\nfast-export HEAD >/dev/null\"):\n\n  [before]                [after]\n  real    0m14.347s       real    0m13.780s\n  user    0m14.084s       user    0m13.620s\n  sys     0m0.208s        sys     0m0.100s\n\nand somewhat more on more blob-heavy repos (this is a\nrepository full of media files):\n\n  [before]                [after]\n  real    0m52.236s       real    0m44.451s\n  user    0m50.568s       user    0m43.000s\n  sys     0m1.536s        sys     0m1.284s\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nWe actually spend a non-trivial amount of time re-checking the sha1 of\nobjects we are loading. This change also makes it easy to drop that\nchecking, though perhaps the additional safety is a good thing to have\nduring an export. The timings without it are:\n\n  git.git (was 14.347s)\n  real    0m11.452s\n  user    0m11.336s\n  sys     0m0.072s\n\n  photos (was 44.451s)\n  real    0m18.383s\n  user    0m17.108s\n  sys     0m1.224s\n\n builtin/fast-export.c | 16 ++++++++++------\n 1 file changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 3eba852..d380155 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -119,6 +119,7 @@ static void export_blob(const unsigned char *sha1)\n \tenum object_type type;\n \tchar *buf;\n \tstruct object *object;\n+\tint eaten;\n \n \tif (no_data)\n \t\treturn;\n@@ -126,16 +127,18 @@ static void export_blob(const unsigned char *sha1)\n \tif (is_null_sha1(sha1))\n \t\treturn;\n \n-\tobject = parse_object(sha1);\n-\tif (!object)\n-\t\tdie (\"Could not read blob %s\", sha1_to_hex(sha1));\n-\n-\tif (object->flags & SHOWN)\n+\tobject = lookup_object(sha1);\n+\tif (object && object->flags & SHOWN)\n \t\treturn;\n \n \tbuf = read_sha1_file(sha1, &type, &size);\n \tif (!buf)\n \t\tdie (\"Could not read blob %s\", sha1_to_hex(sha1));\n+\tif (check_sha1_signature(sha1, buf, size, typename(type)) < 0)\n+\t\tdie(\"sha1 mismatch in blob %s\", sha1_to_hex(sha1));\n+\tobject = parse_object_buffer(sha1, type, size, buf, &eaten);\n+\tif (!object)\n+\t\tdie(\"Could not read blob %s\", sha1_to_hex(sha1));\n \n \tmark_next_object(object);\n \n@@ -147,7 +150,8 @@ static void export_blob(const unsigned char *sha1)\n \tshow_progress();\n \n \tobject->flags |= SHOWN;\n-\tfree(buf);\n+\tif (!eaten)\n+\t\tfree(buf);\n }\n \n static int depth_first(const void *a_, const void *b_)\n-- \n1.8.2.rc2.7.gef06216\n"}]}