On Thu, Oct 21 2021, Jeff King wrote: > On Thu, Oct 21, 2021 at 02:48:24AM +0200, Ævar Arnfjörð Bjarmason wrote: > >> >> There's subtle dependency issues there as well, and just having a >> >> one-off solution for the the command-list.h doesn't get us closer to >> >> addressing that sibling implementation. >> > >> > So I don't know what "subtle dependency issues" you found here, but this >> > is exactly the kind of complexity it was my goal to avoid. >> >> But how? I don't see how narrowly making the loop in generate-cmdlist.sh >> gets us closer to generating the "cmds_txt" in the >> Documentation/Makefile. > > What I meant is that the work to get everything right in the Makefile to > correctly handle dependencies and a partial rebuild can be tricky. For > instance, you're still stuck with a big wildcard dependency on > Documentation/git*.txt (and a manual list of exclusions in the Makefile) > because it's hard in make to do make new dynamic rules based on an > existing one (i.e., the list _should_ come from what's in > command-list.txt). Or the fact that we apparently need to keep the old > script around or cmake anyway. > > It's also much slower. Here are from-scratch builds before and after > your patch 7: > > $ hyperfine --warmup 1 -p 'make clean' 'make command-list.h' > Benchmark #1: make command-list.h > Time (mean ± σ): 1.527 s ± 0.060 s [User: 1.320 s, System: 0.649 s] > Range (min … max): 1.433 s … 1.625 s 10 runs > > > $ hyperfine --warmup 1 -p 'make clean' 'make command-list.h' > Benchmark #1: make command-list.h > Time (mean ± σ): 2.661 s ± 0.080 s [User: 2.359 s, System: 1.082 s] > Range (min … max): 2.481 s … 2.756 s 10 runs > > I know that partial builds will offset that in some cases, but it still > feels like a step in the wrong direction. Even with a partial build, > swapping out "make clean" for "touch Documentation/git-add.txt" takes > about 200ms for me. Whereas with the faster version of > generate-cmdlist.sh I showed, it takes 150ms. > > Now performance isn't everything, and it's possible these partial > snippets will be useful in other places. But I'm not sure I see any real > advantage in this series, and it seems like we're taking a hit in both > performance and complexity in the meantime. Yes, the same numbers are noted in the 7/8 commit message. I.e. it's slower on -j1, but faster with higher -j<n> numbers. Aside from any changes I'm proposing here it seems rather pointless to me to optimize the runtime of -j1 runs. I think we use those in e.g. CI, so of course if they become *really* slow it will matter, but the purpose of this change is to make hacking on git easier, both in terms of runtime and discovering what the Makefile is doing wih V=1. I think anyone hacking on git is going to be on a system with -j2 at least. So again, separate from these specific changes, if we've got a change that speeds up -jN runs at the cost of a -j1 run that seems like good thing. In terms of the utility of benchmarks this benchmark uses "make" and is meaningful, but e.g. <YXCKqAEwtwFozWk6@nand.local> (and I think some other ones?) in this thread invoke the shellscript directly. Those sorts of benchmarks may or may not matter, and in this case the script is always called in the context of a Makefile, so that's really the only meaningful way to test it. If e.g. its performance changes in a way that won't be noticed in other Makefile noise it probably won't matter to anyone.
