On Mon, Nov 18, 2024 at 8:01 PM Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote: > > Contrary to expectations, passing a single candidate tag to "git > describe" is slower than not passing any --match options. > > $ time git describe --debug > ... > traversed 10619 commits > ... > v6.12-rc5-63-g0fc810ae3ae1 > > real 0m0.169s > > $ time git describe --match=v6.12-rc5 --debug > ... > traversed 1310024 commits > v6.12-rc5-63-g0fc810ae3ae1 > > real 0m1.281s > > In fact, the --debug output shows that git traverses all or most of > history. For some repositories and/or git versions, those 1.3s are > actually 10-15 seconds. > > This has been acknowledged as a performance bug in git [1], and a fix > is on its way [2]. However, no solution is yet in git.git, and even > when one lands, it will take quite a while before it finds its way to > a release and for $random_kernel_developer to pick that up. > > So rewrite the logic to use plumbing commands. For each of the > candidate values of $tag, we ask: (1) is $tag even an annotated > tag? (2) Is it eligible to describe HEAD, i.e. an ancestor of > HEAD? (3) If so, how many commits are in $tag..HEAD? > > I have tested that this produces the same output as the current script > for ~700 random commits between v6.9..v6.10. For those 700 commits, > and in my git repo, the 'make -s kernelrelease' command is on average > ~4 times faster with this patch applied (geometric mean of ratios). > > For the commit mentioned in Josh's original report [3], the > time-consuming part of setlocalversion goes from > > $ time git describe --match=v6.12-rc5 c1e939a21eb1 > v6.12-rc5-44-gc1e939a21eb1 > > real 0m1.210s > > to > > $ time git rev-list --count --left-right v6.12-rc5..c1e939a21eb1 > 0 44 > > real 0m0.037s > > [1] https://lore.kernel.org/git/20241101113910.GA2301440@xxxxxxxxxxxxxxxxxxxxxxx/ > [2] https://lore.kernel.org/git/20241106192236.GC880133@xxxxxxxxxxxxxxxxxxxxxxx/ > [3] https://lore.kernel.org/lkml/309549cafdcfe50c4fceac3263220cc3d8b109b2.1730337435.git.jpoimboe@xxxxxxxxxx/ > > Reported-by: Sean Christopherson <seanjc@xxxxxxxxxx> > Closes: https://lore.kernel.org/lkml/ZPtlxmdIJXOe0sEy@xxxxxxxxxx/ > Reported-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx> > Tested-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx> > Closes: https://lore.kernel.org/lkml/309549cafdcfe50c4fceac3263220cc3d8b109b2.1730337435.git.jpoimboe@xxxxxxxxxx/ > Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> > --- > v3: > > - Update trailer tag list, per Masahiro. > - Drop redundant quoutes around the word tag > - Add a shellcheck disable directive > > Masahiro, I decided to keep the changes minimal, in particular not to > change anything around the logic or the (unused) return values, in > order not to invalidate Josh' T-b tag. I think it's more important for > this to make it to 6.13-rc1 (if that is even still possible, given > that the MW is already open). This is not urgent because it has been broken more than one year. Your "|| return 1" may not live long. https://lore.kernel.org/linux-kbuild/20241118231534.1351938-1-masahiroy@xxxxxxxxxx/T/#u If you write try_tag() like you wrote, my patch can become even simpler. > + > + # $2 is the number of commits in the range $tag..HEAD, possibly 0. > + count="$2" count=$2 is enough because double-quotes are not required on the RHS of an assignment. > - # If we are at the tagged commit, we ignore it because the version is > - # well-defined. > - if [ "${tag}" != "${desc}" ]; then > + # If we are at the tagged commit, we ignore it because the > + # version is well-defined. If none of the attempted tags exist > + # or were usable, $count is still empty. > + if [ -z "${count}" ] || [ "${count}" -gt 0 ]; then Is this code equivalent to the following? if [ "${count}" != 0 ]; then -- Best Regards Masahiro Yamada
