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

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Nov 10, 2007, 11:12 UTC
Message-ID: <47359221.7090707@lsrfire.ath.cx>
In-Reply-To: <20071110004635.GA14992@sigill.intra.peff.net>

Jeff King schrieb:

Show 15 quoted lines

> 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. :)

Show 6 quoted lines

>> 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é