strbuf_getwholeline calls getc in a tight loop. On modern
libc implementations, the stdio code locks the handle for
every operation, which means we are paying a significant
overhead. We can get around this by locking the handle for
the whole loop and using the unlocked variant.
Running "git rev-parse refs/heads/does-not-exist" on a repo
with an extremely large (1.6GB) packed-refs file went from:
real 0m18.900s
user 0m18.472s
sys 0m0.448s
to:
real 0m10.953s
user 0m10.384s
sys 0m0.580s
for a wall-clock speedup of 42%. All times are best-of-3,
and done on a glibc 2.19 system.
Note that we call into strbuf_grow while holding the lock.
It's possible for that function to call other stdio
functions (e.g., printing to stderr when dying due to malloc
error); however, the POSIX.1-2001 definition of flockfile
makes it clear that the locks are per-handle, so we are fine
unless somebody else tries to read from our same handle.
This doesn't ever happen in the current code, and is
unlikely to be added in the future (we would have to do
something exotic like add a die_routine that tried to read
from stdin).
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
I don't think the complexity is worth it, but if we wanted to be more
careful about the locks, I think it would probably involve growing the
buffer, locking, doing unlocked reads until it's full, and then
unlocking for the next round of growth.
I also considered optimizing the "term == '\n'" case by using fgets, but
it gets rather complex (you have to pick a size, fgets into it, and then
keep going if you didn't get a newline). Also, fgets sucks, because you
have to call strlen() immediately after to find out how many bytes you
got!
strbuf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/strbuf.c b/strbuf.c
index 14f337d..af2bad4 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -443,12 +443,14 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
return EOF;
strbuf_reset(sb);
- while ((ch = getc(fp)) != EOF) {
+ flockfile(fp);
+ while ((ch = getc_unlocked(fp)) != EOF) {
strbuf_grow(sb, 1);
sb->buf[sb->len++] = ch;
if (ch == term)
break;
}
+ funlockfile(fp);
if (ch == EOF && sb->len == 0)
return EOF;
--
2.4.0.rc0.363.gf9f328b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html