Re: [PATCH] Optimize prefixcmp()

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Optimize prefixcmp()

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:02

René Scharfe [off-list ref] writes:
prefixcmp() was already optimized before -- only for a different use
case.  At a number of callsites the prefix is a string literal, which
allowed the compiler to perform the strlen() call at compile time.

The patch increases the text size considerably: the file "git" is
2,620,938 without and 2,640,450 with the patch in my build (there are
136 callsites in builtin*.c).  The new version of prefixcmp() shouldn't
be inlined any more, as the benefit of doing so is gone.
Yuck, you are absolutely right.  The late thread may have been
well intentioned but resulted in this regression.  Sorry about
that.

I presume that all callers with constant prefix are outside
performance critical parts?  Can we simply uninline the function
in that case?

Re: [PATCH] Optimize prefixcmp()

From: René Scharfe <hidden>
Date: 2016-06-15 22:44:02

Junio C Hamano schrieb:
René Scharfe [off-list ref] writes:
quoted
prefixcmp() was already optimized before -- only for a different use
case.  At a number of callsites the prefix is a string literal, which
allowed the compiler to perform the strlen() call at compile time.

The patch increases the text size considerably: the file "git" is
2,620,938 without and 2,640,450 with the patch in my build (there are
136 callsites in builtin*.c).  The new version of prefixcmp() shouldn't
be inlined any more, as the benefit of doing so is gone.
Yuck, you are absolutely right.  The late thread may have been
well intentioned but resulted in this regression.  Sorry about
that.

I presume that all callers with constant prefix are outside
performance critical parts?  Can we simply uninline the function
in that case?
Most of them seem to be non-critical performance-wise.  They are part of
code to parse parameters or config files.  Exceptions are the commit
message parsing code used for --pretty=format (which can't be an issue
given that prefixcmp() was made the way it's now to speed up this code
path) and half of the callsites in fast-import.c.

René
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help