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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html