On Wed, Jul 14, 2021 at 9:43 AM Jeff King <peff@xxxxxxxx> wrote: > > On Wed, Jul 14, 2021 at 01:12:33AM +0000, Elijah Newren via GitGitGadget wrote: > > > The combined effect of the above is that the file size used in past > > calculations was likely about 5x too large. Combine that with a CPU > > performance improvement of ~30%, and we can increase the limits by > > a factor of sqrt(5/(1-.3)) = 2.67, while keeping the original stated > > time limits. > > It's slightly sad that we only got a 30% CPU improvement in the past 10 > years. Are you just counting clock speed as a short-hand here? I think > that doesn't tell the whole story. But all of this is a side-note > anyway. What I care about is your actual timings. :) I'm using shorthand when discussing file sizes above (though I used actual measurements when picking new values below). But the 30% came from measuring the timings with the exact same sample file as you and using a lightly modified version of your original script (tweaked to also change file basenames) on an AWS c5xlarge instance. My timings showed they were only about 30% faster than what you got when you last bumped the limits. All my laptops and even my work machine are pretty old, so I picked an AWS c5xlarge instance hoping it'd represent a relatively new machine (since your benchmarks when you bumped were using a new machine at the time). Maybe the stock c5xlarge instance I picked wasn't a good choice (wasn't new enough? had a low relative clock speed? affected by Spectre patches? had slow disks? something else?). Or maybe single processor speed really did just hit up against Moore's law and nearly all gains have shifted to having more cores available which don't benefit us here since our algorithm is serial. > (It also seems like this rename detection is ripe for parallelization, > but obviously that's a totally separate topic). True. > > Using the original time limit of 2s for diff.renameLimit, and bumping > > merge.renameLimit from 10s to 60s, I found the following timings using > > the simple script at the end of this commit message (on an AWS c5.xlarge > > which reports as "Intel(R) Xeon(R) Platinum 8124M CPU @ 3.00GHz"): > > > > N Timing > > 1300 1.995s > > 7100 59.973s > > > > So let's round down to nice even numbers and bump the limits from > > 400->1000, and from 1000->7000. > > Sounds good. Thanks for thoroughly measuring and updating.
