From: Kevin Willford Date: Thu, 10 Aug 2017 19:57:24 GMT Subject: Re: [PATCH] cache-tree: remove use of strbuf_addf in update_one Message-ID: In-Reply-To: <20170810190349.jxlp6i7c6q5hpari@sigill.intra.peff.net> On 8/10/2017 3:03 PM, Jeff King wrote: > On Thu, Aug 10, 2017 at 11:58:34AM -0700, Stefan Beller wrote: > >> On Thu, Aug 10, 2017 at 11:47 AM, Kevin Willford 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 The strbuf_add call is essentially only having to do a memcpy whereas the strbuf_addf will have to parse the string, determine the types, convert the data, and then get it in the buffer. That could be made faster with a better compat/ routine but I fear still far from the length check and memcpy. void strbuf_add(struct strbuf *sb, const void *data, size_t len) { strbuf_grow(sb, len); memcpy(sb->buf + sb->len, data, len); strbuf_setlen(sb, sb->len + len); } Here are some of the performance numbers from the windows repo. I will work on writing a perf test for this change so that we have a better idea on smaller repo what the impact of this change is on them. | w/o | with fix | ----------------------------------- git checkout | 36.08 s | 33.34 s | ----------------------------------- git checkout | 32.54 s | 28.26 s | ----------------------------------- git checkout | 44.10 s | 38.13 s | ----------------------------------- git merge | 32.90 s | 30.56 s | ----------------------------------- git rebase | 46.14 s | 42.18 s |