{"thread":{"id":"4324","subject":"Don't use \"sscanf()\" for tree mode scanning","startedAt":"2006-05-28T23:16:15Z","lastAt":"2006-05-28T23:16:15Z","messageCount":1,"participants":["Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"20850","messageId":"Pine.LNX.4.64.0605281559100.5623@g5.osdl.org","threadId":"4324","inReplyTo":null,"subject":"Don't use \"sscanf()\" for tree mode scanning","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-28T23:16:15Z","receivedAt":"2006-05-28T23:16:15Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nDoing an oprofile run on the result of my git rev-list memory leak fixes \nand tree parsing cleanups, I was surprised by the third-highest entry \nbeing\n\n\tsamples  %        image name               app name                 symbol name\n\t179751    2.7163  libc-2.4.so              libc-2.4.so              _IO_vfscanf@@GLIBC_2.4\n\nwhere that 2.7% is actually more than 5% of one CPU, because this was run \non a dual CPU setup with the other CPU just being idle.\n\nThat seems to all be from the use of 'sscanf(tree, \"%o\", &mode)' for the \ntree buffer parsing.\n\nSo do the trivial octal parsing by hand, which also gives us where the \nfirst space in the string is (and thus where the pathname starts) so we \ncan get rid of the \"strchr(tree, ' ')\" call too.\n\nThis brings the \"git rev-list --all --objects\" time down from 63 seconds \nto 55 seconds on the historical kernel archive for me, so it's quite \nnoticeable - tree parsing is a lot of what we end up doing when following \nall the objects.\n\n[ I also see a 5% speedup on a full \"git fsck-objects\" on the current \n  kernel archive, so that sscanf() really does seem to have hurt our\n  performance by a surprising amount ]\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n\n---\n tree-walk.c |   21 ++++++++++++++++++---\n 1 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 9f7abb7..3922058 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -47,18 +47,33 @@ void update_tree_entry(struct tree_desc \n \tdesc->size = size - len;\n }\n \n+static const char *get_mode(const char *str, unsigned int *modep)\n+{\n+\tunsigned char c;\n+\tunsigned int mode = 0;\n+\n+\twhile ((c = *str++) != ' ') {\n+\t\tif (c < '0' || c > '7')\n+\t\t\treturn NULL;\n+\t\tmode = (mode << 3) + (c - '0');\n+\t}\n+\t*modep = mode;\n+\treturn str;\n+}\n+\n const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep)\n {\n \tvoid *tree = desc->buf;\n \tunsigned long size = desc->size;\n \tint len = strlen(tree)+1;\n \tconst unsigned char *sha1 = tree + len;\n-\tconst char *path = strchr(tree, ' ');\n+\tconst char *path;\n \tunsigned int mode;\n \n-\tif (!path || size < len + 20 || sscanf(tree, \"%o\", &mode) != 1)\n+\tpath = get_mode(tree, &mode);\n+\tif (!path || size < len + 20)\n \t\tdie(\"corrupt tree file\");\n-\t*pathp = path+1;\n+\t*pathp = path;\n \t*modep = canon_mode(mode);\n \treturn sha1;\n }\n"}]}