On Wed, Apr 15, 2015 at 01:21:15PM -0700, Junio C Hamano wrote:
> > - type[i++] = c;
> > - if (i >= sizeof(type))
> > - return -1;
> > + strbuf_addch(&typename, c);
> > }
> > - type[i] = 0;
>
> This _might_ have some performance impact in that strbuf_addch()
> involves strbuf_grow(*, 1), which does "does it overflow to
> increment sb->len by one?"; I would say it should be unmeasurable
> because the function is expected to be used only on loose objects
> and you shouldn't have very many of them without packing in your
> repository in the first place.
>
> I guess Peff's c1822d4f (strbuf: add an optimized 1-character
> strbuf_grow, 2015-04-04) may want to teach strbuf_addch() to use his
> new strbuf_grow_ch(), and once that happens the performance worry
> would disappear without this code to be changed at all.
I haven't re-rolled that series yet, but the discussion there showed
that strbuf_grow_ch() is unnecessary; call-sites can just check:
if (!strbuf_avail(sb))
strbuf_grow(sb, 1);
to get the fast inline check. Since we go to the trouble to inline
strbuf_addch, we should probably teach it the same trick. It would be
nice to identify a place with a tight strbuf_addch() loop that could
demonstrate the speed increase, though.
-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
