git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Don't use "sscanf()" for tree mode scanning
@ 2006-05-28 23:16 Linus Torvalds
  0 siblings, 0 replies; only message in thread
From: Linus Torvalds @ 2006-05-28 23:16 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


Doing an oprofile run on the result of my git rev-list memory leak fixes 
and tree parsing cleanups, I was surprised by the third-highest entry 
being

	samples  %        image name               app name                 symbol name
	179751    2.7163  libc-2.4.so              libc-2.4.so              _IO_vfscanf@@GLIBC_2.4

where that 2.7% is actually more than 5% of one CPU, because this was run 
on a dual CPU setup with the other CPU just being idle.

That seems to all be from the use of 'sscanf(tree, "%o", &mode)' for the 
tree buffer parsing.

So do the trivial octal parsing by hand, which also gives us where the 
first space in the string is (and thus where the pathname starts) so we 
can get rid of the "strchr(tree, ' ')" call too.

This brings the "git rev-list --all --objects" time down from 63 seconds 
to 55 seconds on the historical kernel archive for me, so it's quite 
noticeable - tree parsing is a lot of what we end up doing when following 
all the objects.

[ I also see a 5% speedup on a full "git fsck-objects" on the current 
  kernel archive, so that sscanf() really does seem to have hurt our
  performance by a surprising amount ]

Signed-off-by: Linus Torvalds <torvalds@osdl.org>

---
 tree-walk.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tree-walk.c b/tree-walk.c
index 9f7abb7..3922058 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -47,18 +47,33 @@ void update_tree_entry(struct tree_desc 
 	desc->size = size - len;
 }
 
+static const char *get_mode(const char *str, unsigned int *modep)
+{
+	unsigned char c;
+	unsigned int mode = 0;
+
+	while ((c = *str++) != ' ') {
+		if (c < '0' || c > '7')
+			return NULL;
+		mode = (mode << 3) + (c - '0');
+	}
+	*modep = mode;
+	return str;
+}
+
 const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep)
 {
 	void *tree = desc->buf;
 	unsigned long size = desc->size;
 	int len = strlen(tree)+1;
 	const unsigned char *sha1 = tree + len;
-	const char *path = strchr(tree, ' ');
+	const char *path;
 	unsigned int mode;
 
-	if (!path || size < len + 20 || sscanf(tree, "%o", &mode) != 1)
+	path = get_mode(tree, &mode);
+	if (!path || size < len + 20)
 		die("corrupt tree file");
-	*pathp = path+1;
+	*pathp = path;
 	*modep = canon_mode(mode);
 	return sha1;
 }

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2006-05-28 23:16 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-28 23:16 Don't use "sscanf()" for tree mode scanning Linus Torvalds

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).