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

From: Jeff King <peff@peff.net>
Date: Nov 10, 2007, 00:46 UTC
Message-ID: <20071110004635.GA14992@sigill.intra.peff.net>
In-Reply-To: <4734EA4E.8070405@lsrfire.ath.cx>

On Sat, Nov 10, 2007 at 12:16:30AM +0100, René Scharfe wrote:

Show 6 quoted lines

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

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.

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

> (next)
> $ time git log --pretty=format:"* %cd %cn%n%n%s%n%b" >/dev/null
> real    0m0.631s

[...]

> (next+patch)
> $ time git log --pretty=format:"* %cd %cn%n%n%s%n%b" >/dev/null
> real    0m0.570s

Great, you have sped up the uncommon case. But what is the cost to "git log --pretty=format:" and "git log --pretty=format:%cd"?

-Peff