Re: [PATCH 2/2] --pretty=format: on-demand format expansion

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Jeff King schrieb:
> On Sat, Nov 10, 2007 at 12:16:30AM +0100, René Scharfe wrote:
> 
>>> A partial patch on top of yours is below (it caches commit and tree 
>>> abbreviations; parent abbreviations and person-parsing are probably 
>>> worth doing). Some timings:
>> ... but I object to the choice of items to cache.  Are there real-world
>> formats containing the same placeholder twice or even more often?
> 
> My choice of items was more "here is what I am talking about" and not
> "this is the best set of items."
> 
> As for what real-world workloads are like, part of the _point_ of
> --pretty=format: is for one-off formats that users use in their
> workflow. So yes, I have used formats that repeat specifiers, but they
> are probably not common.

Hmm, OK.

> The point of my timings was to show not only that we sped up that
> uncommon case, but that there was negligible cost to the common case.
> And since we don't know what formats users will provide, it makes sense
> not to have lousy performance on the uncommon.

My suspicion was that it's more like "never used" rather than
"uncommon".  Since you're a duplicate placeholder user, it's not "never"
any more. :)

>> There is probably more to gain from the interdependencies of different
>> placeholders.  The patch below attempts to avoid parsing the commit
>> twice, by saving pointers to the different parts.
> 
> Looks sane, although I don't see any reason this couldn't just go on top
> of my patch, and then we can speed up both cases.

Yes, the two are independent.  I don't like the malloc()'s in your
patch, though, and have cooked up a different one on top of a cleaned up
version of mine.  It plays the dirty trick of reading expansions of
repeated placeholders from the strbuf..

René