Re: [PATCH 4/6] strbuf: add an optimized 1-character strbuf_grow

From: Jeff King <peff@peff.net>
Date: Apr 6, 2015, 05:05 UTC
Message-ID: <20150406050544.GB1932@peff.net>
In-Reply-To: <CAPig+cTK5MJLfwETFfM1Zdw35M2geKKmA6Q9M0kHdMP9t8qLNg@mail.gmail.com>

On Sun, Apr 05, 2015 at 10:13:21PM -0400, Eric Sunshine wrote:

Show 8 quoted lines

> > -               strbuf_grow(sb, 1);
> > +               strbuf_grow_ch(sb);
> 
> strbuf_grow_ch() seems overly special-case. What about instead taking
> advantage of inline strbuf_avail() to do something like this?
> 
>     if (!strbuf_avail())
>         strbuf_grow(sb, 1);

Thanks, I somehow missed that function (despite it being a few line above the one I added!).

I agree that strbuf_avail is a much better generic interface, and it turns out to be just as fast (actually, a tiny bit faster in my tests). I'll use that in the re-roll.

Show 5 quoted lines

> (Minor tangent: The 1 is still slightly magical and potentially
> confusing for someone who doesn't know that the buffer is grown
> aggressively, so changing it to a larger number might make it more
> obvious to the casual reader that the buffer is in fact not being
> grown on every iteration.)

I agree this is slightly confusing (and I had to double-check how strbuf_grow worked while writing this series). OTOH, this is not so much about the "1" here as about how strbufs work. We care about the amortized asymptotic cost. strbuf_add() has the same issue; we add more bytes in each chunk, but we would still want to make sure that there is a sub-linear relationship between the number of adds and the number of allocations).

-Peff