Re: [PATCH 3/6] strbuf_getwholeline: use getc_unlocked

On Tue, Apr 07, 2015 at 03:48:33PM +0200, Rasmus Villemoes wrote:

quoted

  3. Find some alternative that is more robust than fgets, and faster
     than getc. I don't think there is anything in stdio, but I am not
     above dropping in a faster non-portable call if it is available,
     and then falling back to the current code otherwise.

getdelim is in POSIX 2008
(http://pubs.opengroup.org/stage7tc1/functions/getdelim.html), so should
be available on any half-{d,r}ecent platform. It obviously has the
advantage of having access to the internal stdio buffer, and by
definition handles embedded NULs. No idea if using such modern
interfaces in git is ok, though.

Thanks, that's perfect. I knew about getline(), but not getdelim(), and
I had thought getline() unconditionally malloc'd. But it doesn't; it
behaves exactly as we are already doing here. :-/

Implementation-wise, I think strbuf_getwholeline could be implemented
mostly as a simple wrapper for getdelim. If I'm reading the current code
and the posix spec for getdelim correctly, something like this should do
it (though obviously not meant to be included as-is):

I think it's close to what we want. strbuf_grow calls xrealloc, which
will call try_to_free_routine() and possibly die() for us. So we would
probably want to check errno on failure and respond similarly if we get
ENOMEM.

Your patch performs even faster than my fgets version (about 8%).

I wonder if it is even worth doing the getc_unlocked dance at all. It
would potentially speed up the fallback code, but my hope that would be
that most systems would simply use the getdelim() version.

-Peff