Re: [PATCH v13 16/16] ls-tree: split up "fast path" callbacks

From: Teng Long <dyroneteng@gmail.com>
Date: Mar 23, 2022, 09:58 UTC
Message-ID: <20220323095854.7787-1-dyroneteng@gmail.com>
In-Reply-To: <220321.865yo7d5ki.gmgdl@evledraar.gmail.com>

On Mon, 21 Mar 2022 10:20:34 +0100, Ævar Arnfjörð Bjarmason wrote:

> FWIW I didn't do any exhaustive benchmarks of this, but I checked a few
> things against origin/master on linux.git and all the reported
> "hyperfine" timings were the same/within the +/- interval.

I sended a reply on gmail but seems not updated on public-inbox, so I sended again use git send-mail this time. If the duplicated message appears, sorry for that.

I tested the performance between fast-paths and non-fast-paths again on linux.git. And I found the default format appeared a performance regression which maybe was brought by this commit:

    $hyperfine --warmup=10 "/opt/git/master/bin/git ls-tree -r HEAD"
    Benchmark 1: /opt/git/master/bin/git ls-tree -r HEAD
      Time (mean ± σ):     111.3 ms ±   2.3 ms    [User: 86.2 ms, System: 25.0 ms]
      Range (min … max):   107.8 ms … 115.1 ms    25 runs

    $hyperfine --warmup=10 "/opt/git/master/bin/git ls-tree -r --format='%(objectmode) %(objecttype) %(objectname)%x09%(path)' HEAD"
    Benchmark 1: /opt/git/master/bin/git ls-tree -r --format='%(objectmode) %(objecttype) %(objectname)%x09%(path)' HEAD
      Time (mean ± σ):     159.1 ms ±   4.7 ms    [User: 131.8 ms, System: 27.3 ms]
      Range (min … max):   152.3 ms … 170.4 ms    19 runs

Further, the other fast-paths except "default format" seem like what we expect, then I made this change:

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index c9eb7f7243..7d784f97c7 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -406,15 +406,17 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
         * The generic show_tree_fmt() is slower than show_tree(), so
         * take the fast path if possible.
         */
-       while (m2f++) {
+       while (m2f) {
                if (!m2f->fmt) {
                        fn = format ? show_tree_fmt : show_tree_default;
                } else if (format && !strcmp(format, m2f->fmt)) {
+                       fprintf(stderr, "[dyrone] format: %s\n", format);
                        cmdmode = m2f->mode;
                        fn = m2f->fn;
                } else if (!format && cmdmode == m2f->mode) {
                        fn = m2f->fn;
                } else {
+                       m2f++;
                        continue;
                }
                break;

----------end diff---------------

After the scenario has been tested agin, the result seems OK:

    $hyperfine --warmup=10 "/opt/git/master/bin/git ls-tree -r --format='%(objectmode) %(objecttype) %(objectname)%x09%(path)' HEAD"
    Benchmark 1: /opt/git/master/bin/git ls-tree -r --format='%(objectmode) %(objecttype) %(objectname)%x09%(path)' HEAD
      Time (mean ± σ):     112.2 ms ±   2.5 ms    [User: 86.3 ms, System: 25.9 ms]
      Range (min … max):   108.8 ms … 117.4 ms    25 runs


Thanks.