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