On 3/26/2020 3:54 AM, Jeff King wrote: > Commit 645c432d61 (pack-objects: use reachability bitmap index when > generating non-stdout pack, 2016-09-10) added two timing tests for > packing to an on-disk file, both with and without bitmaps. However, the > non-bitmap one isn't interesting to have as part of p5310's regression > suite. It _could_ be used as a baseline to show off the improvement in > the bitmap case, but: > > - the point of the t/perf suite is to find performance regressions, > and it won't help with that. We don't compare the numbers between > two tests (which the perf suite has no idea are even related), and > any change in its numbers would have nothing to do with bitmaps. This does make me think if there is a way to adjust "./run" to test two different config settings or command-line options instead of just two different build versions. Perhaps something like ./run "HEAD -c core.commitGraph=true" "HEAD -c core.commitGraph=false" -- p4200-line-log.sh But that's just musing on my part. > - it did show off the improvement in the commit message of 645c432d61, > but it wasn't even necessary there. The bitmap case already shows an > improvement (because before the patch, it behaved the same as the > non-bitmap case), and the perf suite is even able to show the > difference between the before and after measurements. > > On top of that, it's one of the most expensive tests in the suite, > clocking in around 60s for linux.git on my machine (as compared to 16s > for the bitmapped version). And by default when using "./run", we'd run > it three times! > > So let's just drop it. It's not useful and is adding minutes to perf > runs. I agree with your reasoning. This is not a critical path for clients, and all servers should be using bitmaps. Thanks, -Stolee
