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

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

> (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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to