On 11/04/2012 12:41 PM, Jeff King wrote: > On Sun, Nov 04, 2012 at 07:46:51AM +0100, Michael Haggerty wrote: > >> Use ALLOC_GROW() rather than inline code to manage memory in >> strbuf_split_buf(). Rename "pos" to "nr" because it better describes >> the use of the variable and it better conforms to the "ALLOC_GROW" >> idiom. > > I suspect this was not used originally because ALLOC_GROW relies on > alloc_nr, which does fast growth early on. At (x+16)*3/2, we end up with > 24 slots for the first allocation. We are typically splitting 1 or 2 > values. > > It probably doesn't make a big difference in practice, though, as we're > talking about wasting less than 200 bytes on a 64-bit platform, and we > do not tend to keep large numbers of split lists around. I did a little bit of archeology, and found out that * ALLOC_GROW() did indeed exist when this code was developed, so it *could have* been used. * OTOH, I didn't find any indication on the mailing list that the choice not to use ALLOC_GROW() was a conscious decision. So history doesn't give us much guidance. If the size of the initial allocation is a concern, then I would suggest adding a macro like ALLOC_SET_SIZE(ary,nr,alloc) that could be called to initialize the size to some number less than 24. Such a macro might be useful elsewhere, too. It wouldn't, of course, slow the growth rate *after* the first allocation. FWIW, the "max" parameter of strbuf_split*() is only used in one place, though strbuf_split*() is used in some other places where not too many substrings would be expected. I am working on some patch series that will eliminate even more uses of strbuf_split*(), so I won't work more on optimizing its resource usage unless somebody gives me a stronger nudge. Michael -- Michael Haggerty mhagger@xxxxxxxxxxxx http://softwareswirl.blogspot.com/ -- 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