[PATCH v2 0/2] t: add "test-tool genbytes" to avoid slow tr(1) on macOS

["Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>]

While looking for slow tests in CI, t4205 stood out on macOS: its EXPENSIVE
tests take several times longer there than on Linux. A CI experiment showed
that much of the time was spent in tr(1), which is very slow on macOS for
the 2GB of data that t4205 generates.

Patch 1 adds "test-tool genbytes" and uses it instead of "genzeros | tr",
which is where the speedup comes from. Patch 2 converts the remaining
"genzeros" callers and removes it.

Changes since v1:

 * Squashed the rename of test-genzeros.c into the patch that changes it, as
   suggested by Junio. The file changes enough that the default rename
   detection shows it as deleted and newly created; "git format-patch -M30%"
   shows it as a rename.

 * "genbytes" now takes the byte as two lowercase hexadecimal digits instead
   of a literal character, as suggested by Junio. This also works for NUL
   bytes, so patch 2 now uses "genbytes 00" instead of special-casing "NUL".

Kristofer Karlsson (2):
  t: add "test-tool genbytes" to avoid slow tr(1) on macOS
  t: replace "test-tool genzeros" with "genbytes 00"

 Makefile                          |  2 +-
 t/helper/meson.build              |  2 +-
 t/helper/test-genbytes.c          | 51 +++++++++++++++++++++++++++++++
 t/helper/test-genzeros.c          | 37 ----------------------
 t/helper/test-tool.c              |  2 +-
 t/helper/test-tool.h              |  2 +-
 t/t0613-reftable-write-options.sh |  2 +-
 t/t1007-hash-object.sh            |  8 ++---
 t/t1051-large-conversion.sh       |  4 +--
 t/t1450-fsck.sh                   |  4 +--
 t/t4141-apply-too-large.sh        |  2 +-
 t/t4205-log-pretty-formats.sh     |  2 +-
 t/t5300-pack-object.sh            |  6 ++--
 t/t5318-commit-graph.sh           |  2 +-
 t/t7006-pager.sh                  |  3 +-
 t/t9850-shell.sh                  |  2 +-
 16 files changed, 72 insertions(+), 59 deletions(-)
 create mode 100644 t/helper/test-genbytes.c
 delete mode 100644 t/helper/test-genzeros.c


base-commit: fb255012acbcbe81189cb8014da0280b09c66ecc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2301%2Fspkrka%2Ftest-tool-genbytes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2301/spkrka/test-tool-genbytes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2301

Range-diff vs v1:

 1:  b4378b3527 < -:  ---------- t/helper: rename test-genzeros.c to test-genbytes.c
 2:  4c02ada9b8 ! 1:  54139a3a97 t: add "test-tool genbytes" to avoid slow tr(1) on macOS
     @@ Commit message
          macOS CI runners, tr(1) alone spends 95 to 123 seconds of CPU time on
          it, compared to about 6 seconds on Linux.
      
     -    Fix this by adding "test-tool genbytes <byte> [<count>]", which writes
     -    the requested byte directly, and turn "genzeros" into a thin
     -    wrapper around the same code.  Use the new helper in all tests that
     -    piped "genzeros" through tr(1), not just t4205, so that it is the
     -    obvious choice for new tests.
     +    Fix this by adding "test-tool genbytes <hex-byte> [<count>]", which
     +    writes the requested byte, given as two lowercase hexadecimal
     +    digits, directly.  Turn "genzeros" into a thin wrapper around the
     +    same code, and rename the file to match.  Use the new helper in all
     +    tests that piped "genzeros" through tr(1), not just t4205, so that
     +    it is the obvious choice for new tests.
      
          This makes t4205 much faster on macOS in CI.  The Linux jobs, shown
          for comparison, only save the few seconds that GNU tr(1) took, which
     @@ Commit message
      
          Signed-off-by: Kristofer Karlsson <krka@spotify.com>
      
     - ## t/helper/test-genbytes.c ##
     + ## Makefile ##
     +@@ Makefile: TEST_BUILTINS_OBJS += test-env-helper.o
     + TEST_BUILTINS_OBJS += test-example-tap.o
     + TEST_BUILTINS_OBJS += test-find-pack.o
     + TEST_BUILTINS_OBJS += test-fsmonitor-client.o
     ++TEST_BUILTINS_OBJS += test-genbytes.o
     + TEST_BUILTINS_OBJS += test-genrandom.o
     +-TEST_BUILTINS_OBJS += test-genzeros.o
     + TEST_BUILTINS_OBJS += test-getcwd.o
     + TEST_BUILTINS_OBJS += test-hash-speed.o
     + TEST_BUILTINS_OBJS += test-hash.o
     +
     + ## t/helper/meson.build ##
     +@@ t/helper/meson.build: test_tool_sources = [
     +   'test-example-tap.c',
     +   'test-find-pack.c',
     +   'test-fsmonitor-client.c',
     ++  'test-genbytes.c',
     +   'test-genrandom.c',
     +-  'test-genzeros.c',
     +   'test-getcwd.c',
     +   'test-hash-speed.c',
     +   'test-hash.c',
     +
     + ## t/helper/test-genbytes.c (new) ##
      @@
     - #include "test-tool.h"
     - #include "git-compat-util.h"
     - 
     --int cmd__genzeros(int argc, const char **argv)
     ++#define DISABLE_SIGN_COMPARE_WARNINGS
     ++
     ++#include "test-tool.h"
     ++#include "git-compat-util.h"
     ++#include "hex-ll.h"
     ++
      +/*
      + * Write "count" copies of "byte" to stdout, or an endless stream of them if
      + * "count" is negative.
      + */
      +static int generate_bytes(char byte, intmax_t count)
     - {
     --	/* static, so that it is NUL-initialized */
     --	static const char zeros[256 * 1024];
     --	intmax_t count;
     ++{
      +	char buf[256 * 1024];
     - 	ssize_t n;
     - 
     --	if (argc > 2) {
     --		fprintf(stderr, "usage: %s [<count>]\n", argv[0]);
     --		return 1;
     --	}
     --
     --	count = argc > 1 ? strtoimax(argv[1], NULL, 0) : -1;
     ++	ssize_t n;
     ++
      +	memset(buf, byte, sizeof(buf));
     - 
     --	/* Writing out individual NUL bytes is slow... */
     ++
      +	/* Writing out individual bytes is slow... */
     - 	while (count < 0)
     --		if (xwrite(1, zeros, ARRAY_SIZE(zeros)) < 0)
     ++	while (count < 0)
      +		if (xwrite(1, buf, ARRAY_SIZE(buf)) < 0)
     - 			die_errno("write error");
     - 
     - 	while (count > 0) {
     --		n = xwrite(1, zeros,
     --			   count < ARRAY_SIZE(zeros)
     --			   ? count : ARRAY_SIZE(zeros));
     ++			die_errno("write error");
     ++
     ++	while (count > 0) {
      +		n = xwrite(1, buf,
      +			   count < ARRAY_SIZE(buf)
      +			   ? count : ARRAY_SIZE(buf));
     - 
     - 		if (n < 0)
     - 			die_errno("write error");
     -@@ t/helper/test-genbytes.c: int cmd__genzeros(int argc, const char **argv)
     - 
     - 	return 0;
     - }
     ++
     ++		if (n < 0)
     ++			die_errno("write error");
     ++
     ++		count -= n;
     ++	}
     ++
     ++	return 0;
     ++}
      +
      +static intmax_t parse_count(int argc, const char **argv, int pos)
      +{
     @@ t/helper/test-genbytes.c: int cmd__genzeros(int argc, const char **argv)
      +
      +int cmd__genbytes(int argc, const char **argv)
      +{
     -+	if (argc < 2 || argc > 3 || strlen(argv[1]) != 1) {
     -+		fprintf(stderr, "usage: %s <byte> [<count>]\n", argv[0]);
     ++	if (argc < 2 || argc > 3 || strlen(argv[1]) != 2 ||
     ++	    strspn(argv[1], "0123456789abcdef") != 2) {
     ++		fprintf(stderr, "usage: %s <hex-byte> [<count>]\n", argv[0]);
      +		return 1;
      +	}
      +
     -+	return generate_bytes(argv[1][0], parse_count(argc, argv, 2));
     ++	return generate_bytes(hex2chr(argv[1]), parse_count(argc, argv, 2));
      +}
      +
      +int cmd__genzeros(int argc, const char **argv)
     @@ t/helper/test-genbytes.c: int cmd__genzeros(int argc, const char **argv)
      +	return generate_bytes('\0', parse_count(argc, argv, 1));
      +}
      
     + ## t/helper/test-genzeros.c (deleted) ##
     +@@
     +-#define DISABLE_SIGN_COMPARE_WARNINGS
     +-
     +-#include "test-tool.h"
     +-#include "git-compat-util.h"
     +-
     +-int cmd__genzeros(int argc, const char **argv)
     +-{
     +-	/* static, so that it is NUL-initialized */
     +-	static const char zeros[256 * 1024];
     +-	intmax_t count;
     +-	ssize_t n;
     +-
     +-	if (argc > 2) {
     +-		fprintf(stderr, "usage: %s [<count>]\n", argv[0]);
     +-		return 1;
     +-	}
     +-
     +-	count = argc > 1 ? strtoimax(argv[1], NULL, 0) : -1;
     +-
     +-	/* Writing out individual NUL bytes is slow... */
     +-	while (count < 0)
     +-		if (xwrite(1, zeros, ARRAY_SIZE(zeros)) < 0)
     +-			die_errno("write error");
     +-
     +-	while (count > 0) {
     +-		n = xwrite(1, zeros,
     +-			   count < ARRAY_SIZE(zeros)
     +-			   ? count : ARRAY_SIZE(zeros));
     +-
     +-		if (n < 0)
     +-			die_errno("write error");
     +-
     +-		count -= n;
     +-	}
     +-
     +-	return 0;
     +-}
     +
       ## t/helper/test-tool.c ##
      @@ t/helper/test-tool.c: static struct test_cmd cmds[] = {
       	{ "example-tap", cmd__example_tap },
     @@ t/t0613-reftable-write-options.sh: test_expect_success 'small block size fails w
       		cd repo &&
       		test_commit A &&
      -		test-tool genzeros 500 | tr "\000" "a" >logmsg &&
     -+		test-tool genbytes a 500 >logmsg &&
     ++		test-tool genbytes 61 500 >logmsg &&
       		cat >expect <<-EOF &&
       		fatal: update_ref failed for ref ${SQ}refs/heads/logme${SQ}: reftable: transaction failure: entry too large
       		EOF
     @@ t/t4205-log-pretty-formats.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'lo
       
       test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'set up huge commit' '
      -	test-tool genzeros 2147483649 | tr "\000" "1" >expect &&
     -+	test-tool genbytes 1 2147483649 >expect &&
     ++	test-tool genbytes 31 2147483649 >expect &&
       	huge_commit=$(git commit-tree -F expect HEAD^{tree})
       '
       
     @@ t/t5300-pack-object.sh: test_description='git pack-object'
      -	test-tool genzeros 4096 | tr "\000" "a" >a &&
      -	test-tool genzeros 4096 | tr "\000" "b" >b &&
      -	test-tool genzeros 4096 | tr "\000" "c" >c &&
     -+	test-tool genbytes a 4096 >a &&
     -+	test-tool genbytes b 4096 >b &&
     -+	test-tool genbytes c 4096 >c &&
     ++	test-tool genbytes 61 4096 >a &&
     ++	test-tool genbytes 62 4096 >b &&
     ++	test-tool genbytes 63 4096 >c &&
       	test-tool genrandom "seed a" 2097152 >a_big &&
       	test-tool genrandom "seed b" 2097152 >b_big &&
       	git update-index --add a a_big b b_big c &&
     @@ t/t7006-pager.sh: test_expect_success 'setup trace2' '
       test_expect_success 'setup large log output' '
      -	test-tool genzeros 50000 |
      -	tr "\000" "a" |
     -+	test-tool genbytes a 50000 |
     ++	test-tool genbytes 61 50000 |
       	sed "s/a/this is a long commit message/g" >commit-msg &&
       	git commit --allow-empty -F commit-msg
       '
     @@ t/t9850-shell.sh: test_expect_success 'shell allows interactive command' '
       
       test_expect_success 'shell complains of overlong commands' '
      -	test-tool genzeros | tr "\000" "a" |
     -+	test-tool genbytes a |
     ++	test-tool genbytes 61 |
       	test_must_fail git shell 2>err &&
       	test_grep "too long" err
       '
 3:  169ca5968e ! 2:  f49af0e33f t: replace "test-tool genzeros" with "genbytes NUL"
     @@ Metadata
      Author: Kristofer Karlsson <krka@spotify.com>
      
       ## Commit message ##
     -    t: replace "test-tool genzeros" with "genbytes NUL"
     +    t: replace "test-tool genzeros" with "genbytes 00"
      
     -    Now that "genbytes" exists, "genzeros" is merely a special case of it
     -    for NUL bytes.  It is only kept as a separate command because a NUL
     -    byte cannot be passed as a command-line argument.
     -
     -    Teach "genbytes" to accept "NUL" as the name of that byte instead,
     -    convert the remaining callers of "genzeros", and remove it, so that
     -    there is a single command for generating repeated bytes.
     +    Now that "genbytes" exists, "genzeros" is merely the special case of
     +    it that writes NUL bytes.  Convert its remaining callers to
     +    "genbytes 00" and remove it, so that there is a single command for
     +    generating repeated bytes.
      
          Only some of these callers depend on the data being NUL bytes, like
          t1007, which compares against the object ID of 5GB of zeros, and
     @@ Commit message
          Signed-off-by: Kristofer Karlsson <krka@spotify.com>
      
       ## t/helper/test-genbytes.c ##
     -@@ t/helper/test-genbytes.c: static int generate_bytes(char byte, intmax_t count)
     - 	return 0;
     - }
     +@@ t/helper/test-genbytes.c: int cmd__genbytes(int argc, const char **argv)
       
     -+/*
     -+ * A NUL byte cannot be passed as an argument, so it is spelled out as
     -+ * "NUL".  Returns -1 for anything that is not a single byte.
     -+ */
     -+static int parse_byte(const char *arg)
     -+{
     -+	if (!strcmp(arg, "NUL"))
     -+		return '\0';
     -+	if (strlen(arg) == 1)
     -+		return (unsigned char)arg[0];
     -+	return -1;
     -+}
     -+
     - static intmax_t parse_count(int argc, const char **argv, int pos)
     - {
     - 	return pos < argc ? strtoimax(argv[pos], NULL, 0) : -1;
     -@@ t/helper/test-genbytes.c: static intmax_t parse_count(int argc, const char **argv, int pos)
     - 
     - int cmd__genbytes(int argc, const char **argv)
     - {
     --	if (argc < 2 || argc > 3 || strlen(argv[1]) != 1) {
     --		fprintf(stderr, "usage: %s <byte> [<count>]\n", argv[0]);
     --		return 1;
     --	}
     -+	int byte;
     - 
     --	return generate_bytes(argv[1][0], parse_count(argc, argv, 2));
     --}
     + 	return generate_bytes(hex2chr(argv[1]), parse_count(argc, argv, 2));
     + }
      -
      -int cmd__genzeros(int argc, const char **argv)
      -{
      -	if (argc > 2) {
      -		fprintf(stderr, "usage: %s [<count>]\n", argv[0]);
     -+	if (argc < 2 || argc > 3 || (byte = parse_byte(argv[1])) < 0) {
     -+		fprintf(stderr, "usage: %s (<byte> | NUL) [<count>]\n", argv[0]);
     - 		return 1;
     - 	}
     - 
     +-		return 1;
     +-	}
     +-
      -	return generate_bytes('\0', parse_count(argc, argv, 1));
     -+	return generate_bytes(byte, parse_count(argc, argv, 2));
     - }
     +-}
      
       ## t/helper/test-tool.c ##
      @@ t/helper/test-tool.c: static struct test_cmd cmds[] = {
     @@ t/t1007-hash-object.sh: test_expect_success '--stdin outside of repository (uses
       test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
       		'files over 4GB hash literally' '
      -	test-tool genzeros $((5*1024*1024*1024)) >big &&
     -+	test-tool genbytes NUL $((5*1024*1024*1024)) >big &&
     ++	test-tool genbytes 00 $((5*1024*1024*1024)) >big &&
       	test_oid large5GB >expect &&
       	git hash-object --stdin --literally <big >actual &&
       	test_cmp expect actual
     @@ t/t1007-hash-object.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
       test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
       		'files over 4GB hash correctly via --stdin' '
      -	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
     -+	{ test -f big || test-tool genbytes NUL $((5*1024*1024*1024)) >big; } &&
     ++	{ test -f big || test-tool genbytes 00 $((5*1024*1024*1024)) >big; } &&
       	test_oid large5GB >expect &&
       	git hash-object --stdin <big >actual &&
       	test_cmp expect actual
     @@ t/t1007-hash-object.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
       test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
       		'files over 4GB hash correctly' '
      -	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
     -+	{ test -f big || test-tool genbytes NUL $((5*1024*1024*1024)) >big; } &&
     ++	{ test -f big || test-tool genbytes 00 $((5*1024*1024*1024)) >big; } &&
       	test_oid large5GB >expect &&
       	git hash-object -- big >actual &&
       	test_cmp expect actual
     @@ t/t1007-hash-object.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
       test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
       		'hash filtered files over 4GB correctly' '
      -	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
     -+	{ test -f big || test-tool genbytes NUL $((5*1024*1024*1024)) >big; } &&
     ++	{ test -f big || test-tool genbytes 00 $((5*1024*1024*1024)) >big; } &&
       	test_oid large5GB >expect &&
       	test_config filter.null-filter.clean "cat" &&
       	echo "big filter=null-filter" >.gitattributes &&
     @@ t/t1051-large-conversion.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG
       	small_size=$(test_file_size small) &&
       	test_config filter.makelarge.smudge \
      -		"test-tool genzeros $((5*1024*1024*1024)) && cat" &&
     -+		"test-tool genbytes NUL $((5*1024*1024*1024)) && cat" &&
     ++		"test-tool genbytes 00 $((5*1024*1024*1024)) && cat" &&
       	echo "small filter=makelarge" >.gitattributes &&
       	rm small &&
       	git checkout -- small &&
     @@ t/t1051-large-conversion.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG
       test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
       		'files over 4GB convert on input' '
      -	test-tool genzeros $((5*1024*1024*1024)) >big &&
     -+	test-tool genbytes NUL $((5*1024*1024*1024)) >big &&
     ++	test-tool genbytes 00 $((5*1024*1024*1024)) >big &&
       	test_config filter.checklarge.clean "wc -c >big.size" &&
       	echo "big filter=checklarge" >.gitattributes &&
       	git add big &&
     @@ t/t1450-fsck.sh: test_expect_success 'fsck error on gitattributes with excessive
       
       test_expect_success 'fsck error on gitattributes with excessive size' '
      -	blob=$(test-tool genzeros $((100 * 1024 * 1024 + 1)) | git hash-object -w --stdin) &&
     -+	blob=$(test-tool genbytes NUL $((100 * 1024 * 1024 + 1)) | git hash-object -w --stdin) &&
     ++	blob=$(test-tool genbytes 00 $((100 * 1024 * 1024 + 1)) | git hash-object -w --stdin) &&
       	test_when_finished "remove_object $blob" &&
       	tree=$(printf "100644 blob %s\t%s\n" $blob .gitattributes | git mktree) &&
       	test_when_finished "remove_object $tree" &&
     @@ t/t1450-fsck.sh: test_expect_success 'fsck with blob size equal to core.bigFileT
       	(
       		cd big-blob-fsck &&
      -		test-tool genzeros 1024 >blob &&
     -+		test-tool genbytes NUL 1024 >blob &&
     ++		test-tool genbytes 00 1024 >blob &&
       		git add blob &&
       		git commit -m "blob equal to threshold" &&
       		git repack -ad &&
     @@ t/t4141-apply-too-large.sh: test_expect_success EXPENSIVE 'git apply rejects pat
       		@@ -0,0 +1 @@
       		EOF
      -		test-tool genzeros $((1024 * 1024 * 1023))
     -+		test-tool genbytes NUL $((1024 * 1024 * 1023))
     ++		test-tool genbytes 00 $((1024 * 1024 * 1023))
       	} | test_must_fail git apply 2>err &&
       	test_grep "patch too large" err
       '
     @@ t/t5318-commit-graph.sh: corrupt_graph_and_verify() {
       	printf "$data" | dd of="full/$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc &&
       	dd of="full/$objdir/info/commit-graph" bs=1 seek="$zero_pos" if=/dev/null &&
      -	test-tool genzeros $(($orig_size - $zero_pos)) >>"full/$objdir/info/commit-graph" &&
     -+	test-tool genbytes NUL $(($orig_size - $zero_pos)) >>"full/$objdir/info/commit-graph" &&
     ++	test-tool genbytes 00 $(($orig_size - $zero_pos)) >>"full/$objdir/info/commit-graph" &&
       	corrupt_graph_verify "$grepstr"
       
       }

-- 
gitgitgadget