Re: [PATCH] cache-tree: remove use of strbuf_addf in update_one

From: Jeff King <peff@peff.net>
Date: Aug 10, 2017, 19:03 UTC
Message-ID: <20170810190349.jxlp6i7c6q5hpari@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kaB7w+OD_8LOgqHjBJ8gCtXUGWPj7Cw8bWXEEQweqMaFw@mail.gmail.com>

On Thu, Aug 10, 2017 at 11:58:34AM -0700, Stefan Beller wrote:

Show 9 quoted lines

> On Thu, Aug 10, 2017 at 11:47 AM, Kevin Willford <kcwillford@gmail.com> wrote:
> > String formatting can be a performance issue when there are
> > hundreds of thousands of trees.
> 
> When changing this for the sake of performance, could you give
> an example (which kind of repository you need for this to become
> a bottleneck? I presume the large Windows repo? Or can I
> reproduce it with a small repo such as linux.git or even git.git?)
> and some numbers how this improves the performance?

I was about to say the same thing. Normally I don't mind a small optimization without numbers if the result is obviously an improvement.

But in this case the result is a lot less readable, and it's not entirely clear to me that it would always be an improvement (we now always run 3 strbuf calls instead of one, and have to check the length for each one).

What I'm wondering specifically is if vsnprintf() on Kevin's platform (which I'll assume is Windows) is slow, and we would do better to replace it with a faster compat/ routine.

-Peff