[PATCH v4] cat-file: skip expanding default format

["John Cai via GitGitGadget" <gitgitgadget@gmail.com>]

From: John Cai <johncai86@gmail.com>

When format is passed into --batch, --batch-check, --batch-command,
the format gets expanded. When nothing is passed in, the default format
is set and the expand_format() gets called.

We can save on these cycles by hardcoding how to print the
information when nothing is passed as the format, or when the default
format is passed. There is no need for the fully expanded format with
the default. Since batch_object_write() happens on every object provided
in batch mode, we get a nice performance improvement.

git rev-list --all > /tmp/all-obj.txt

git cat-file --batch-check </tmp/all-obj.txt

with HEAD^:

Time (mean ± σ): 57.6 ms ± 1.7 ms [User: 51.5 ms, System: 6.2 ms]
Range (min … max): 54.6 ms … 64.7 ms 50 runs

with HEAD:

Time (mean ± σ): 49.8 ms ± 1.7 ms [User: 42.6 ms, System: 7.3 ms]
Range (min … max): 46.9 ms … 55.9 ms 56 runs

If nothing is provided as a format argument, or if the default format is
passed, skip expanding of the format and print the object info with a
default format.

See https://lore.kernel.org/git/87eecf8ork.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
---
    optimize cat file batch info writing
    
    When cat-file --batch or --batch-check is used, we can skip having to
    expand the format if no format is specified or if the default format is
    specified. In this case we know exactly how to print the objects without
    the full expanded format.
    
    This was first discussed in [1].
    
    We get a little performance boost from this optimization because this
    happens for each objects provided to --batch, --batch-check, or
    --batch-command. Because batch_object_write() is called on every oid
    provided in batch mode, this optimization adds up when a large number of
    oid info is printed.
    
    git rev-list --all >/tmp/all-objs.txt
    
    git cat-file --batch-check </tmp/all-obj.txt (with hyperfine)
    
    run on origin/master:
    
    Time (mean ± σ): 57.6 ms ± 1.7 ms [User: 51.5 ms, System: 6.2 ms] Range
    (min … max): 54.6 ms … 64.7 ms 50 runs
    
    run on jc/optimize-cat-file-batch-default-format:
    
    Time (mean ± σ): 49.8 ms ± 1.7 ms [User: 42.6 ms, System: 7.3 ms] Range
    (min … max): 46.9 ms … 55.9 ms 56 runs
    
    Changes since v3:
    
     * reuse scratch string buffer
     * use --batch-all-objects in test
    
    Changes since v2:
    
     * get rid of type_name string buffer, and call type_name() for each
       object in print_default_format()
    
    Changes since v1:
    
     * set opt->format in batch_objects so that the loop that prints objects
       only has to check if the format is null to know to print the object
       info in the default format
     * fixed up commit trailer to include Ævar as Signed-off-by
    
     1. https://lore.kernel.org/git/87eecf8ork.fsf@evledraar.gmail.com/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1221%2Fjohn-cai%2Fjc%2Foptimize-cat-file-batch-default-format-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1221/john-cai/jc/optimize-cat-file-batch-default-format-v4
Pull-Request: https://github.com/git/git/pull/1221

Range-diff vs v3:

 1:  56d13da5141 ! 1:  b0c6a18b7ad cat-file: skip expanding default format
     @@ builtin/cat-file.c: static void print_object_or_die(struct batch_options *opt, s
       	}
       }
       
     -+static int print_default_format(char *buf, int len, struct expand_data *data)
     ++static void print_default_format(struct strbuf *scratch, struct expand_data *data)
      +{
     -+	return xsnprintf(buf, len, "%s %s %"PRIuMAX"\n", oid_to_hex(&data->oid),
     -+			 type_name(data->type),
     -+			 (uintmax_t)*data->info.sizep);
     ++	strbuf_addf(scratch, "%s %s %"PRIuMAX"\n", oid_to_hex(&data->oid),
     ++		    type_name(data->type),
     ++		    (uintmax_t)data->size);
      +}
      +
       /*
        * If "pack" is non-NULL, then "offset" is the byte offset within the pack from
        * which the object may be accessed (though note that we may also rely on
      @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,
     - 		}
       	}
       
     --	strbuf_reset(scratch);
     + 	strbuf_reset(scratch);
      -	strbuf_expand(scratch, opt->format, expand_format, data);
      -	strbuf_addch(scratch, '\n');
     --	batch_write(opt, scratch->buf, scratch->len);
     ++
      +	if (!opt->format) {
     -+		char buf[1024];
     -+		int len = print_default_format(buf, 1024, data);
     -+		batch_write(opt, buf, len);
     ++		print_default_format(scratch, data);
      +	} else {
     -+		strbuf_reset(scratch);
      +		strbuf_expand(scratch, opt->format, expand_format, data);
      +		strbuf_addch(scratch, '\n');
     -+		batch_write(opt, scratch->buf, scratch->len);
      +	}
     ++
     + 	batch_write(opt, scratch->buf, scratch->len);
       
     - 	if (opt->print_contents) {
     - 		print_object_or_die(opt, data);
     -@@ builtin/cat-file.c: static int batch_unordered_packed(const struct object_id *oid,
     - 				      data);
     + 	if (opt->batch_mode == BATCH_MODE_CONTENTS) {
     +@@ builtin/cat-file.c: static void batch_objects_command(struct batch_options *opt,
     + 	strbuf_release(&input);
       }
       
     -+
      +#define DEFAULT_FORMAT "%(objectname) %(objecttype) %(objectsize)"
      +
       static int batch_objects(struct batch_options *opt)
     @@ builtin/cat-file.c: static int batch_objects(struct batch_options *opt)
      +		      &data);
       	data.mark_query = 0;
       	strbuf_release(&output);
     - 	if (opt->cmdmode)
     + 	if (opt->transform_mode)
       		data.split_on_whitespace = 1;
       
      +	if (opt->format && !strcmp(opt->format, DEFAULT_FORMAT))
     @@ t/perf/p1006-cat-file.sh (new)
      @@
      +#!/bin/sh
      +
     -+test_description='Basic sort performance tests'
     ++test_description='Tests listing object info performance'
      +. ./perf-lib.sh
      +
      +test_perf_large_repo
      +
     -+test_expect_success 'setup' '
     -+	git rev-list --all >rla
     -+'
     -+
      +test_perf 'cat-file --batch-check' '
     -+	git cat-file --batch-check <rla
     ++	git cat-file --batch-all-objects --batch-check
      +'
      +
      +test_done


 builtin/cat-file.c       | 29 +++++++++++++++++++++++------
 t/perf/p1006-cat-file.sh | 12 ++++++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100755 t/perf/p1006-cat-file.sh

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index e75e110302e..94d5901e9a7 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -357,6 +357,13 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
 	}
 }
 
+static void print_default_format(struct strbuf *scratch, struct expand_data *data)
+{
+	strbuf_addf(scratch, "%s %s %"PRIuMAX"\n", oid_to_hex(&data->oid),
+		    type_name(data->type),
+		    (uintmax_t)data->size);
+}
+
 /*
  * If "pack" is non-NULL, then "offset" is the byte offset within the pack from
  * which the object may be accessed (though note that we may also rely on
@@ -388,8 +395,14 @@ static void batch_object_write(const char *obj_name,
 	}
 
 	strbuf_reset(scratch);
-	strbuf_expand(scratch, opt->format, expand_format, data);
-	strbuf_addch(scratch, '\n');
+
+	if (!opt->format) {
+		print_default_format(scratch, data);
+	} else {
+		strbuf_expand(scratch, opt->format, expand_format, data);
+		strbuf_addch(scratch, '\n');
+	}
+
 	batch_write(opt, scratch->buf, scratch->len);
 
 	if (opt->batch_mode == BATCH_MODE_CONTENTS) {
@@ -643,6 +656,8 @@ static void batch_objects_command(struct batch_options *opt,
 	strbuf_release(&input);
 }
 
+#define DEFAULT_FORMAT "%(objectname) %(objecttype) %(objectsize)"
+
 static int batch_objects(struct batch_options *opt)
 {
 	struct strbuf input = STRBUF_INIT;
@@ -651,9 +666,6 @@ static int batch_objects(struct batch_options *opt)
 	int save_warning;
 	int retval = 0;
 
-	if (!opt->format)
-		opt->format = "%(objectname) %(objecttype) %(objectsize)";
-
 	/*
 	 * Expand once with our special mark_query flag, which will prime the
 	 * object_info to be handed to oid_object_info_extended for each
@@ -661,12 +673,17 @@ static int batch_objects(struct batch_options *opt)
 	 */
 	memset(&data, 0, sizeof(data));
 	data.mark_query = 1;
-	strbuf_expand(&output, opt->format, expand_format, &data);
+	strbuf_expand(&output,
+		      opt->format ? opt->format : DEFAULT_FORMAT,
+		      expand_format,
+		      &data);
 	data.mark_query = 0;
 	strbuf_release(&output);
 	if (opt->transform_mode)
 		data.split_on_whitespace = 1;
 
+	if (opt->format && !strcmp(opt->format, DEFAULT_FORMAT))
+		opt->format = NULL;
 	/*
 	 * If we are printing out the object, then always fill in the type,
 	 * since we will want to decide whether or not to stream.
diff --git a/t/perf/p1006-cat-file.sh b/t/perf/p1006-cat-file.sh
new file mode 100755
index 00000000000..dcd80153796
--- /dev/null
+++ b/t/perf/p1006-cat-file.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+test_description='Tests listing object info performance'
+. ./perf-lib.sh
+
+test_perf_large_repo
+
+test_perf 'cat-file --batch-check' '
+	git cat-file --batch-all-objects --batch-check
+'
+
+test_done

base-commit: b896f729e240d250cf56899e6a0073f6aa469f5d
-- 
gitgitgadget