{"thread":{"id":"34889","subject":"packv4: current performance","startedAt":"2013-09-09T19:14:02Z","lastAt":"2013-09-09T19:14:02Z","messageCount":1,"participants":["Nicolas Pitre"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"227247","messageId":"alpine.LFD.2.03.1309091454450.20709@syhkavp.arg","threadId":"34889","inReplyTo":null,"subject":"packv4: current performance","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2013-09-09T19:14:02Z","receivedAt":"2013-09-09T19:14:02Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Of course pack v4, while significantly better on disk space, is still \nlagging behind pack v2 in terms of runtime performance.  This is because \nright now the reading side is implemented as a compatibility layer that \nparses the packv4 data to regenerate a canonical object representation, \nwhich is then parsed again by the core code.  The full runtime benefit \nof pack v4 will be apparent when the core code is optimized to use pack \nv4 data directly (hint hint to potential contributors).\n\nStill, I've looked a bit closer at the performance issue, and one thing \nwas clear: stdio string functions are very costly.  So I've added the \npatch below to my tree which provided a big boost in performance.\n\n    packv4-parse.c: get rid of snprintf()\n    \n    Profiling 'git rev-list --all --objects' shows snprintf() related\n    functions topping the total execution time chart.\n    \n    Let's replace that with specialized string generation functions.\n    This reduced the total runtime by a wopping 40% on a Core2 processor.\n    \n    Signed-off-by: Nicolas Pitre <nico@fluxnic.net>\n\ndiff --git a/packv4-parse.c b/packv4-parse.c\nindex 63bba03..c62c4ae 100644\n--- a/packv4-parse.c\n+++ b/packv4-parse.c\n@@ -163,6 +163,77 @@ const unsigned char *get_pathref(struct packed_git *p, unsigned int index)\n \treturn p->path_dict->data + p->path_dict->offsets[index];\n }\n \n+static int tree_line(unsigned char *buf, unsigned long size,\n+\t\t     const char *label, int label_len,\n+\t\t     const unsigned char *sha1)\n+{\n+\tstatic const char hex[] = \"0123456789abcdef\";\n+\tint i;\n+\n+\tif (label_len + 1 + 40 + 1 > size)\n+\t\treturn 0;\n+\n+\tmemcpy(buf, label, label_len);\n+\tbuf += label_len;\n+\t*buf++ = ' ';\n+\n+\tfor (i = 0; i < 20; i++) {\n+\t\tunsigned int val = *sha1++;\n+\t\t*buf++ = hex[val >> 4];\n+\t\t*buf++ = hex[val & 0xf];\n+\t}\n+\n+\t*buf = '\\n';\n+\n+\treturn label_len + 1 + 40 + 1;\n+}\n+\n+static int ident_line(unsigned char *buf, unsigned long size,\n+\t\t      const char *label, int label_len,\n+\t\t      const unsigned char *ident, unsigned long time, int tz)\n+{\n+\tint ident_len = strlen((const char *)ident);\n+\tint len = label_len + 1 + ident_len + 1 + 1 + 5 + 1;\n+\tint time_len = 0;\n+\tunsigned char time_buf[16];\n+\n+\tdo {\n+\t\ttime_buf[time_len++] = '0' + time % 10;\n+\t\ttime /= 10;\n+\t} while (time);\n+\tlen += time_len;\n+\n+\tif (len > size)\n+\t\treturn 0;\n+\n+\tmemcpy(buf, label, label_len);\n+\tbuf += label_len;\n+\t*buf++ = ' ';\n+\n+\tmemcpy(buf, ident, ident_len);\n+\tbuf += ident_len;\n+\t*buf++ = ' ';\n+\n+\tdo {\n+\t\t*buf++ = time_buf[--time_len];\n+\t} while (time_len);\n+\t*buf++ = ' ';\n+\n+\tif (tz < 0) {\n+\t\ttz = -tz;\n+\t\t*buf++ = '-';\n+\t} else\n+\t\t*buf++ = '+';\n+\t*buf++ = '0' + tz / 1000; tz %= 1000;\n+\t*buf++ = '0' + tz / 100;  tz %= 100;\n+\t*buf++ = '0' + tz / 10;   tz %= 10;\n+\t*buf++ = '0' + tz;\n+\n+\t*buf = '\\n';\n+\n+\treturn len;\n+}\n+\n void *pv4_get_commit(struct packed_git *p, struct pack_window **w_curs,\n \t\t     off_t offset, unsigned long size)\n {\n@@ -182,15 +253,17 @@ void *pv4_get_commit(struct packed_git *p, struct pack_window **w_curs,\n \tscp = src;\n \n \tsha1 = get_sha1ref(p, &scp);\n-\tlen = snprintf((char *)dcp, size, \"tree %s\\n\", sha1_to_hex(sha1));\n+\tlen = tree_line(dcp, size, \"tree\", strlen(\"tree\"), sha1);\n+\tif (!len)\n+\t\tdie(\"overflow in %s\", __func__);\n \tdcp += len;\n \tsize -= len;\n \n \tnb_parents = decode_varint(&scp);\n \twhile (nb_parents--) {\n \t\tsha1 = get_sha1ref(p, &scp);\n-\t\tlen = snprintf((char *)dcp, size, \"parent %s\\n\", sha1_to_hex(sha1));\n-\t\tif (len >= size)\n+\t\tlen = tree_line(dcp, size, \"parent\", strlen(\"parent\"), sha1);\n+\t\tif (!len)\n \t\t\tdie(\"overflow in %s\", __func__);\n \t\tdcp += len;\n \t\tsize -= len;\n@@ -211,16 +284,16 @@ void *pv4_get_commit(struct packed_git *p, struct pack_window **w_curs,\n \telse\n \t\tauthor_time = commit_time - (author_time >> 1);\n \n-\tlen = snprintf((char *)dcp, size, \"author %s %lu %+05d\\n\",\n-\t\t\tauthor, author_time, author_tz);\n-\tif (len >= size)\n+\tlen = ident_line(dcp, size, \"author\", strlen(\"author\"), \n+\t\t\t author, author_time, author_tz);\n+\tif (!len)\n \t\tdie(\"overflow in %s\", __func__);\n \tdcp += len;\n \tsize -= len;\n \n-\tlen = snprintf((char *)dcp, size, \"committer %s %lu %+05d\\n\",\n-\t\t\tcommitter, commit_time, commit_tz);\n-\tif (len >= size)\n+\tlen = ident_line(dcp, size, \"committer\", strlen(\"committer\"),\n+\t\t\t committer, commit_time, commit_tz);\n+\tif (!len)\n \t\tdie(\"overflow in %s\", __func__);\n \tdcp += len;\n \tsize -= len;\n@@ -248,6 +321,32 @@ void *pv4_get_commit(struct packed_git *p, struct pack_window **w_curs,\n \treturn dst;\n }\n \n+static int tree_entry_prefix(unsigned char *buf, unsigned long size,\n+\t\t\t     const unsigned char *path, unsigned mode)\n+{\n+\tint path_len = strlen((const char *)path) + 1;\n+\tint mode_len = 0;\n+\tint len;\n+\tunsigned char mode_buf[8];\n+\n+\tdo {\n+\t\tmode_buf[mode_len++] = '0' + (mode & 7);\n+\t\tmode >>= 3;\n+\t} while (mode);\n+\n+\tlen = mode_len + 1 + path_len;\n+\tif (len > size)\n+\t\treturn 0;\n+\n+\tdo {\n+\t\t*buf++ = mode_buf[--mode_len];\n+\t} while (mode_len);\n+\t*buf++ = ' ';\n+\tmemcpy(buf, path, path_len);\n+\n+\treturn len;\n+}\n+\n static int decode_entries(struct packed_git *p, struct pack_window **w_curs,\n \t\t\t  off_t offset, unsigned int start, unsigned int count,\n \t\t\t  unsigned char **dstp, unsigned long *sizep, int hdr)\n@@ -315,9 +414,8 @@ static int decode_entries(struct packed_git *p, struct pack_window **w_curs,\n \t\t\tif (!path || !sha1)\n \t\t\t\treturn -1;\n \t\t\tmode = (path[0] << 8) | path[1];\n-\t\t\tlen = snprintf((char *)*dstp, *sizep, \"%o %s%c\",\n-\t\t\t\t\t   mode, path+2, '\\0');\n-\t\t\tif (len + 20 > *sizep)\n+\t\t\tlen = tree_entry_prefix(*dstp, *sizep, path + 2, mode);\n+\t\t\tif (!len || len + 20 > *sizep)\n \t\t\t\treturn -1;\n \t\t\thashcpy(*dstp + len, sha1);\n \t\t\t*dstp += len + 20;\n"}]}