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

[Jeff King <peff@peff.net>]

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

> >   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