t: add "test-tool genbytes" to avoid slow tr(1) on macOS

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 renames test-genzeros.c so that the actual changes in patch 2 show up as a regular diff. Patch 2 adds "genbytes" and uses it instead of "genzeros | tr", which is where the speedup comes from.

Open question: is patch 3 wanted? It folds "genzeros" into "genbytes", spelling the NUL byte as "NUL", so that there is a single command. Alternatively, we could drop it and keep "genzeros" as a thin wrapper, avoiding changes to its 10 call sites. I kept patch 3 separate so that it is easy to drop, but it could also be squashed into patch 2.

Some other decisions that may be worth pointing out:

* The buffer is no longer static: it used to be static so that it was zero-initialized for free, but now that it is filled with the requested byte on every call, a local buffer avoids shared state. It keeps its size of 256KB. Smaller buffers made no measurable difference in my tests, but there was no reason to change it.

* Invalid counts are still silently treated as 0, as before. That could be tightened, but it is unrelated to this series.

Kristofer Karlsson (3):
t/helper: rename test-genzeros.c to test-genbytes.c
t: add "test-tool genbytes" to avoid slow tr(1) on macOS
t: replace "test-tool genzeros" with "genbytes NUL"

 Makefile                          |  2 +-
 t/helper/meson.build              |  2 +-
 t/helper/test-genbytes.c          | 64 +++++++++++++++++++++++++++++++
 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, 85 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-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2301/spkrka/test-tool-genbytes-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2301

gitgitgadget

From: Kristofer Karlsson <[email protected]>

The next commit adds a "genbytes" command to this file and turns "genzeros" into a thin wrapper around it. Rename the file in a separate step first, so that git can detect the rename and the actual changes show up as a regular diff, which is easier to review.

No functional changes.

---
 Makefile                                      | 2 +-
 t/helper/meson.build                          | 2 +-
 t/helper/{test-genzeros.c => test-genbytes.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename t/helper/{test-genzeros.c => test-genbytes.c} (100%)

index c2a6098484..8d214286e0 100644
@@ -825,8 +825,8 @@ 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

index ae513b4cdc..f99688c49b 100644
@@ -22,8 +22,8 @@ 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',

similarity index 100%
rename from t/helper/test-genzeros.c
rename to t/helper/test-genbytes.c

gitgitgadget

From: Kristofer Karlsson <[email protected]>

Several tests create fixture data consisting of a single repeated character with "test-tool genzeros <count> | tr '\000' <char>". This works well on platforms where tr(1) is implemented efficiently.

The implementation of tr(1) on macOS supports multi-byte characters, and processes its input one character at a time through getwchar() and putwchar(), even in the C locale. For small inputs that does not matter, but t4205 generates more than 2GB of data this way, which makes that test much slower on macOS than on Linux: on the 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.

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 is within the usual noise:

  job                before    after   change
  osx-gcc            821.1s   243.3s   -70.4%
  osx-reftable       911.0s   194.0s   -78.7%
  linux-asan-ubsan   226.7s   222.1s    -2.0%
  linux-leaks        320.2s   298.8s    -6.7%

---
 t/helper/test-genbytes.c          | 52 ++++++++++++++++++++++---------
 t/helper/test-tool.c              |  1 +
 t/helper/test-tool.h              |  1 +
 t/t0613-reftable-write-options.sh |  2 +-
 t/t4205-log-pretty-formats.sh     |  2 +-
 t/t5300-pack-object.sh            |  6 ++--
 t/t7006-pager.sh                  |  3 +-
 t/t9850-shell.sh                  |  2 +-
 8 files changed, 46 insertions(+), 23 deletions(-)

index b895436a32..293af78d0d 100644
@@ -3,29 +3,26 @@
 #include "test-tool.h"
 #include "git-compat-util.h"
 
-int cmd__genzeros(int argc, const char **argv)
+/*
+ * 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;
+	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)
+		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));
+		n = xwrite(1, buf,
+			   count < ARRAY_SIZE(buf)
+			   ? count : ARRAY_SIZE(buf));
 
 		if (n < 0)
 			die_errno("write error");

@@ -35,3 +32,28 @@ int cmd__genzeros(int argc, const char **argv)
 
 	return 0;
 }
+
+static intmax_t parse_count(int argc, const char **argv, int pos)
+{
+	return pos < argc ? strtoimax(argv[pos], NULL, 0) : -1;
+}
+
+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;
+	}
+
+	return generate_bytes(argv[1][0], parse_count(argc, argv, 2));
+}
+
+int cmd__genzeros(int argc, const char **argv)
+{
+	if (argc > 2) {
+		fprintf(stderr, "usage: %s [<count>]\n", argv[0]);
+		return 1;
+	}
+
+	return generate_bytes('\0', parse_count(argc, argv, 1));
+}

index b71a22b43b..af70e2bec7 100644
@@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
 	{ "example-tap", cmd__example_tap },
 	{ "find-pack", cmd__find_pack },
 	{ "fsmonitor-client", cmd__fsmonitor_client },
+	{ "genbytes", cmd__genbytes },
 	{ "genrandom", cmd__genrandom },
 	{ "genzeros", cmd__genzeros },
 	{ "getcwd", cmd__getcwd },

index f2885b33d5..c35e7412af 100644
@@ -26,6 +26,7 @@ int cmd__env_helper(int argc, const char **argv);
 int cmd__example_tap(int argc, const char **argv);
 int cmd__find_pack(int argc, const char **argv);
 int cmd__fsmonitor_client(int argc, const char **argv);
+int cmd__genbytes(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
 int cmd__genzeros(int argc, const char **argv);
 int cmd__getcwd(int argc, const char **argv);

index 0b27d75fe3..0b74ff6157 100755
@@ -146,7 +146,7 @@ test_expect_success 'small block size fails with large reflog message' '
 	(
 		cd repo &&
 		test_commit A &&
-		test-tool genzeros 500 | tr "\000" "a" >logmsg &&
+		test-tool genbytes a 500 >logmsg &&
 		cat >expect <<-EOF &&
 		fatal: update_ref failed for ref ${SQ}refs/heads/logme${SQ}: reftable: transaction failure: entry too large
 		EOF

index 01b97c8888..894d88c39b 100755
@@ -1182,7 +1182,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'log --pretty with huge commit mes
 '
 
 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 &&
 	huge_commit=$(git commit-tree -F expect HEAD^{tree})
 '
 

index aac139e6a0..38dc4021b7 100755
@@ -9,9 +9,9 @@ test_description='git pack-object'
 
 test_expect_success 'setup' '
 	rm -f .git/index* &&
-	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 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 &&

index 75ca7a25da..8f1f71bf12 100755
@@ -662,8 +662,7 @@ test_expect_success 'setup trace2' '
 '
 
 test_expect_success 'setup large log output' '
-	test-tool genzeros 50000 |
-	tr "\000" "a" |
+	test-tool genbytes a 50000 |
 	sed "s/a/this is a long commit message/g" >commit-msg &&
 	git commit --allow-empty -F commit-msg
 '

index 27cc49bbd1..d56edebda7 100755
@@ -30,7 +30,7 @@ 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_must_fail git shell 2>err &&
 	test_grep "too long" err
 '

gitgitgadget

From: Kristofer Karlsson <[email protected]>

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.

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 t5318, which zeroes out part of a commit-graph file. The others only care about the size, but keep generating NUL bytes there, too, so that what they test does not change.

---
 t/helper/test-genbytes.c    | 29 +++++++++++++++++------------
 t/helper/test-tool.c        |  1 -
 t/helper/test-tool.h        |  1 -
 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/t5318-commit-graph.sh     |  2 +-
 8 files changed, 27 insertions(+), 24 deletions(-)

index 293af78d0d..ad0fb2d2b4 100644
@@ -33,6 +33,19 @@ static int generate_bytes(char byte, intmax_t count)
 	return 0;
 }
 
+/*
+ * 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;

@@ -40,20 +53,12 @@ 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));
-}
-
-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 generate_bytes('\0', parse_count(argc, argv, 1));
+	return generate_bytes(byte, parse_count(argc, argv, 2));
 }

index af70e2bec7..16571fd702 100644
@@ -35,7 +35,6 @@ static struct test_cmd cmds[] = {
 	{ "fsmonitor-client", cmd__fsmonitor_client },
 	{ "genbytes", cmd__genbytes },
 	{ "genrandom", cmd__genrandom },
-	{ "genzeros", cmd__genzeros },
 	{ "getcwd", cmd__getcwd },
 	{ "hashmap", cmd__hashmap },
 	{ "hash-speed", cmd__hash_speed },

index c35e7412af..1b45c8c37b 100644
@@ -28,7 +28,6 @@ int cmd__find_pack(int argc, const char **argv);
 int cmd__fsmonitor_client(int argc, const char **argv);
 int cmd__genbytes(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
-int cmd__genzeros(int argc, const char **argv);
 int cmd__getcwd(int argc, const char **argv);
 int cmd__hashmap(int argc, const char **argv);
 int cmd__hash_speed(int argc, const char **argv);

index 463b38f990..ea1a1949a1 100755
@@ -263,7 +263,7 @@ test_expect_success '--stdin outside of repository (uses default hash)' '
 
 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_oid large5GB >expect &&
 	git hash-object --stdin --literally <big >actual &&
 	test_cmp expect actual

@@ -271,7 +271,7 @@ 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_oid large5GB >expect &&
 	git hash-object --stdin <big >actual &&
 	test_cmp expect actual

@@ -279,7 +279,7 @@ 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_oid large5GB >expect &&
 	git hash-object -- big >actual &&
 	test_cmp expect actual

@@ -289,7 +289,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
 # We ensure that cleaning doesn't mangle large files on 64-bit Windows.
 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_oid large5GB >expect &&
 	test_config filter.null-filter.clean "cat" &&
 	echo "big filter=null-filter" >.gitattributes &&

index 361afb679b..6364c396fd 100755
@@ -91,7 +91,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
 	test_commit test small "a small file" &&
 	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" &&
 	echo "small filter=makelarge" >.gitattributes &&
 	rm small &&
 	git checkout -- small &&

@@ -103,7 +103,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
 # the actual size, we ensure that cleaning doesn't mangle large files on 64-bit Windows.
 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_config filter.checklarge.clean "wc -c >big.size" &&
 	echo "big filter=checklarge" >.gitattributes &&
 	git add big &&

index 3e9eb251cb..86b01d071c 100755
@@ -1111,7 +1111,7 @@ test_expect_success 'fsck error on gitattributes with excessive line lengths' '
 '
 
 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) &&
 	test_when_finished "remove_object $blob" &&
 	tree=$(printf "100644 blob %s\t%s\n" $blob .gitattributes | git mktree) &&
 	test_when_finished "remove_object $tree" &&

@@ -1157,7 +1157,7 @@ test_expect_success 'fsck with blob size equal to core.bigFileThreshold' '
 	git init big-blob-fsck &&
 	(
 		cd big-blob-fsck &&
-		test-tool genzeros 1024 >blob &&
+		test-tool genbytes NUL 1024 >blob &&
 		git add blob &&
 		git commit -m "blob equal to threshold" &&
 		git repack -ad &&

index 6bc1c7a663..309807859c 100755
@@ -13,7 +13,7 @@ test_expect_success EXPENSIVE 'git apply rejects patches that are too large' '
 		+++ b/file
 		@@ -0,0 +1 @@
 		EOF
-		test-tool genzeros $((1024 * 1024 * 1023))
+		test-tool genbytes NUL $((1024 * 1024 * 1023))
 	} | test_must_fail git apply 2>err &&
 	test_grep "patch too large" err
 '

index 6cecfbeaa1..974bd21f1e 100755
@@ -522,7 +522,7 @@ corrupt_graph_and_verify() {
 	zero_pos=${4:-${orig_size}} &&
 	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" &&
 	corrupt_graph_verify "$grepstr"
 
 }

gitgitgadget

"Kristofer Karlsson via GitGitGadget" <[email protected]>

Just being curious. Would forcing C locale on it help? I.e.,

genzeros | LANG=C LC_ALL=C tr ...

"Kristofer Karlsson via GitGitGadget" <[email protected]>

Needless to say, with this patch applied, the following tests would now fail, unable to find "test-tool genzeros".

    t0613-reftable-write-options.sh
    t1007-hash-object.sh
    t1051-large-conversion.sh
    t1450-fsck.sh
    t4141-apply-too-large.sh
    t4205-log-pretty-formats.sh
    t5300-pack-object.sh
    t5318-commit-graph.sh
    t7006-pager.sh
    t9850-shell.sh

FWIW, if you apply [1/3] and [2/3], squash them together, then give a custom rename factor to the command, e.g.,

    $ git format-patch -M3 -1

to format the resulting single patch, you will see a modification of genzeros while the file gets renamed just fine, and you would not have to break these tests in the middle of a series.

"Kristofer Karlsson via GitGitGadget" <[email protected]>

I would suggest genbytes to take the value of the byte in decimal or hex, instead of av[1][0], so that you do not have to treat NUL any specially. I would actually think it is even better to limit the input format only to [0-9a-f][0-9a-f], i.e., two hexadecimal digits that does not allow uppercase to represent values 0-255 (inclusive), without support for any other input format like decimal.

After all, this is just a test helper, so ergonomics or flexibility do not matter as much as simplicity.

Shouldn't matter.

Shouldn't matter. That's ergonomics and flexibility that we do not value as much as in a normal program.

Thanks.

Not really. Even though the source file is renamed, the dispatcher inside still recognises genzeros and calls cmd__genzeros. In fact, at this stage, "test-tool genbytes" would not even work (and it does not have to work as nobody calls it).

So, sorry for the false alarm, but my point about not split the rename + modification into two separate steps still stands.

Thanks.

Am 07.11.26 um 15:59 schrieb Kristofer Karlsson via GitGitGadget:

All cases that do not need a particular character can use

	printf %0.500d 0

Though, on my Linux box,

	printf %0.2147483649d 0

produces nothing, but

	printf 000000000%0.2147483640d 0

has 2147483649 bytes of output.

If this isn't insanely slow on macOS, use this pattern instead of a highly engineered helper program? ;-)

-- Hannes

[...]

When using the GGG workflow I think that option is not available, though I will see if I can try formatting and sending the patch manually for this one instead. (Or try to submit a patch to GGG to allow for configuring this too.)

I could squash it all into a single patch too, which I think would make sense if we don't need the intermediate state where both genzeros and genbytes exist at the same time.

What's your preference?

Thanks,
Kristofer

No, test-lib-functions.sh already sets those env variables, but the macos implementation of tr always executes the slow path unfortunately.

Thanks,
Kristofer

Yes, I actually had one local prototype that used this approach, but I held off on it since it would mean a semantically larger change. But I agree it would improve the ergonomics, so I can do it. And it's not too many call sites to convert anyway.

Thanks,
Kristofer

Nice trick, and it would work for t4205, which does not care about the byte value. I tried it for the 2147483649 bytes there on Linux:

  printf           time   peak memory
  dash             6.5s   2.1GB
  bash             6.0s   3MB
  coreutils        3.7s   2.1GB

It is definitely fast enough (on the variants I tested). The annoying thing (but not a real blocker) is the extra memory usage required and I am not sure I fully trust that all implementations will behave the same for absurdly large padding.

(Particularly for paddings larger than 2^31, as you noted too.)

It feels a bit like a hack, and tweaking the existing test-tool feels like a more sane option that is easier to reason about.

A gotcha here is that the printf hack only works for producing "0", so for the cases we truly need NUL characters it does not help us, and I think that is also a point in favor of simply generalizing test-tool genzeros instead.

Thanks,
Kristofer

Am 08.11.26 um 10:18 schrieb Kristofer Karlsson:

I don't know how to measure memory requirements, but if it helps,

	printf 000000000%02147483640d 0

(without the fullstop) works, too.

On the other hand, the only case you really care about is the one that takes an insane amount of time. If you convert only that case to printf, then we do not need all this churn in the first place.

-- Hannes

That's a good point -- if we want to make a minimal fix, we could simply switch t4205 to use printf instead of "genzeros | tr".

- test-tool genzeros 2147483649 | tr "\000" "1" >expect &&
+ printf 000000000%02147483640d 0 >expect &&

I tried this out and it works on Linux but fails on macOS[1]. What happens is that printf only outputs the nine literal zeros ("000000000") there, and silently ignores the very large padding.

[1] https://github.com/gitgitgadget/git/actions/runs/42475956442/job/128717309586

The setup and the next test still pass, since the latter compares against the same file, and only the test expecting the exact size in the error message notices. So I do not think we can rely on printf for this, and I still think that generalizing the existing helper is a reasonable way forward.

I think even if printf _would_ have worked, the end state of having a uniform way of creating this fake data is nicer than having three separate ways:

- genzeros when we only need NUL bytes
- genzeros | tr for small inputs that need some other byte
- printf for large inputs where "0" bytes are valid

(And if we ever need to support large inputs with a specific byte value, we'd get stuck again, though anticipating such a need is speculative.)

Thanks,
Kristofer

Am 08.11.26 um 13:24 schrieb Kristofer Karlsson:

Fair enough.

-- Hannes

I have no strong preference between keeping both genzeros() and genbytes() or rewriting users of genzeros() to use genbytes(), and I prefer to leave that out of the decision.

My preference is to optimize for those who consume the resulting history the most over the longer term.

Your split between a "pure rename" and a "modification in place" is optimizing for those who review the patches on the mailing list (arguably due to tool limitations). While they are an important audience of yours (after all, without their support, your change would not end up in the final history), those who will read the history via "git log" or "git show" to see what happened later are the more important readers of your work in the long run. To them, a single rename-and-modify commit is much easier to understand, I would think.

I read this "printf is an easy and efficient way to create a long sequence of bytes if you do not care what value these bytes have" discussion and found it very illuminating (including the mention of glitches that are platform specific).

Thanks.

That's fair -- For v2 I will squash patches 1 and 2 and keep patch 3. I'll also add the lowercase hex as input format. The similarity seems to be ~29%, so running it with -M20% would only mildly reduce the diff, though I think the diff is manageable to consume anyway.

Thanks,
Kristofer

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 <[email protected]>
      
     - ## 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 <[email protected]>
      
       ## 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 <[email protected]>
      
       ## 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

From: Kristofer Karlsson <[email protected]>

Several tests create fixture data consisting of a single repeated character with "test-tool genzeros <count> | tr '\000' <char>". This works well on platforms where tr(1) is implemented efficiently.

The implementation of tr(1) on macOS supports multi-byte characters, and processes its input one character at a time through getwchar() and putwchar(), even in the C locale. For small inputs that does not matter, but t4205 generates more than 2GB of data this way, which makes that test much slower on macOS than on Linux: on the 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 <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 is within the usual noise:

  job                before    after   change
  osx-gcc            821.1s   243.3s   -70.4%
  osx-reftable       911.0s   194.0s   -78.7%
  linux-asan-ubsan   226.7s   222.1s    -2.0%
  linux-leaks        320.2s   298.8s    -6.7%

---
 Makefile                          |  2 +-
 t/helper/meson.build              |  2 +-
 t/helper/test-genbytes.c          | 61 +++++++++++++++++++++++++++++++
 t/helper/test-genzeros.c          | 37 -------------------
 t/helper/test-tool.c              |  1 +
 t/helper/test-tool.h              |  1 +
 t/t0613-reftable-write-options.sh |  2 +-
 t/t4205-log-pretty-formats.sh     |  2 +-
 t/t5300-pack-object.sh            |  6 +--
 t/t7006-pager.sh                  |  3 +-
 t/t9850-shell.sh                  |  2 +-
 11 files changed, 72 insertions(+), 47 deletions(-)
 create mode 100644 t/helper/test-genbytes.c
 delete mode 100644 t/helper/test-genzeros.c

index c2a6098484..8d214286e0 100644
@@ -825,8 +825,8 @@ 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

index ae513b4cdc..f99688c49b 100644
@@ -22,8 +22,8 @@ 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',

deleted file mode 100644
index b895436a32..0000000000
@@ -1,37 +0,0 @@
-#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;
-}

index b71a22b43b..af70e2bec7 100644
@@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
 	{ "example-tap", cmd__example_tap },
 	{ "find-pack", cmd__find_pack },
 	{ "fsmonitor-client", cmd__fsmonitor_client },
+	{ "genbytes", cmd__genbytes },
 	{ "genrandom", cmd__genrandom },
 	{ "genzeros", cmd__genzeros },
 	{ "getcwd", cmd__getcwd },

index f2885b33d5..c35e7412af 100644
@@ -26,6 +26,7 @@ int cmd__env_helper(int argc, const char **argv);
 int cmd__example_tap(int argc, const char **argv);
 int cmd__find_pack(int argc, const char **argv);
 int cmd__fsmonitor_client(int argc, const char **argv);
+int cmd__genbytes(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
 int cmd__genzeros(int argc, const char **argv);
 int cmd__getcwd(int argc, const char **argv);

index 0b27d75fe3..b2032a799d 100755
@@ -146,7 +146,7 @@ test_expect_success 'small block size fails with large reflog message' '
 	(
 		cd repo &&
 		test_commit A &&
-		test-tool genzeros 500 | tr "\000" "a" >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

index 01b97c8888..aa51a70a34 100755
@@ -1182,7 +1182,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'log --pretty with huge commit mes
 '
 
 test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'set up huge commit' '
-	test-tool genzeros 2147483649 | tr "\000" "1" >expect &&
+	test-tool genbytes 31 2147483649 >expect &&
 	huge_commit=$(git commit-tree -F expect HEAD^{tree})
 '
 

index aac139e6a0..3c2f83091c 100755
@@ -9,9 +9,9 @@ test_description='git pack-object'
 
 test_expect_success 'setup' '
 	rm -f .git/index* &&
-	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 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 &&

index 75ca7a25da..f59b3a00a8 100755
@@ -662,8 +662,7 @@ test_expect_success 'setup trace2' '
 '
 
 test_expect_success 'setup large log output' '
-	test-tool genzeros 50000 |
-	tr "\000" "a" |
+	test-tool genbytes 61 50000 |
 	sed "s/a/this is a long commit message/g" >commit-msg &&
 	git commit --allow-empty -F commit-msg
 '

index 27cc49bbd1..cf05944682 100755
@@ -30,7 +30,7 @@ test_expect_success 'shell allows interactive command' '
 '
 
 test_expect_success 'shell complains of overlong commands' '
-	test-tool genzeros | tr "\000" "a" |
+	test-tool genbytes 61 |
 	test_must_fail git shell 2>err &&
 	test_grep "too long" err
 '

gitgitgadget

From: Kristofer Karlsson <[email protected]>

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 t5318, which zeroes out part of a commit-graph file. The others only care about the size, but keep generating NUL bytes there, too, so that what they test does not change.

---
 t/helper/test-genbytes.c    | 10 ----------
 t/helper/test-tool.c        |  1 -
 t/helper/test-tool.h        |  1 -
 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/t5318-commit-graph.sh     |  2 +-
 8 files changed, 10 insertions(+), 22 deletions(-)

index b760326b24..898d73c516 100644
@@ -49,13 +49,3 @@ int cmd__genbytes(int argc, const char **argv)
 
 	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]);
-		return 1;
-	}
-
-	return generate_bytes('\0', parse_count(argc, argv, 1));
-}

index af70e2bec7..16571fd702 100644
@@ -35,7 +35,6 @@ static struct test_cmd cmds[] = {
 	{ "fsmonitor-client", cmd__fsmonitor_client },
 	{ "genbytes", cmd__genbytes },
 	{ "genrandom", cmd__genrandom },
-	{ "genzeros", cmd__genzeros },
 	{ "getcwd", cmd__getcwd },
 	{ "hashmap", cmd__hashmap },
 	{ "hash-speed", cmd__hash_speed },

index c35e7412af..1b45c8c37b 100644
@@ -28,7 +28,6 @@ int cmd__find_pack(int argc, const char **argv);
 int cmd__fsmonitor_client(int argc, const char **argv);
 int cmd__genbytes(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
-int cmd__genzeros(int argc, const char **argv);
 int cmd__getcwd(int argc, const char **argv);
 int cmd__hashmap(int argc, const char **argv);
 int cmd__hash_speed(int argc, const char **argv);

index 463b38f990..9e28bcc4f0 100755
@@ -263,7 +263,7 @@ test_expect_success '--stdin outside of repository (uses default hash)' '
 
 test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
 		'files over 4GB hash literally' '
-	test-tool genzeros $((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

@@ -271,7 +271,7 @@ 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 00 $((5*1024*1024*1024)) >big; } &&
 	test_oid large5GB >expect &&
 	git hash-object --stdin <big >actual &&
 	test_cmp expect actual

@@ -279,7 +279,7 @@ 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 00 $((5*1024*1024*1024)) >big; } &&
 	test_oid large5GB >expect &&
 	git hash-object -- big >actual &&
 	test_cmp expect actual

@@ -289,7 +289,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
 # We ensure that cleaning doesn't mangle large files on 64-bit Windows.
 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 00 $((5*1024*1024*1024)) >big; } &&
 	test_oid large5GB >expect &&
 	test_config filter.null-filter.clean "cat" &&
 	echo "big filter=null-filter" >.gitattributes &&

index 361afb679b..dd5902b3b3 100755
@@ -91,7 +91,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
 	test_commit test small "a small file" &&
 	small_size=$(test_file_size small) &&
 	test_config filter.makelarge.smudge \
-		"test-tool genzeros $((5*1024*1024*1024)) && cat" &&
+		"test-tool genbytes 00 $((5*1024*1024*1024)) && cat" &&
 	echo "small filter=makelarge" >.gitattributes &&
 	rm small &&
 	git checkout -- small &&

@@ -103,7 +103,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT,!LONG_IS_64BIT \
 # the actual size, we ensure that cleaning doesn't mangle large files on 64-bit Windows.
 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 00 $((5*1024*1024*1024)) >big &&
 	test_config filter.checklarge.clean "wc -c >big.size" &&
 	echo "big filter=checklarge" >.gitattributes &&
 	git add big &&

index 3e9eb251cb..ae21cb897f 100755
@@ -1111,7 +1111,7 @@ test_expect_success 'fsck error on gitattributes with excessive line lengths' '
 '
 
 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 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" &&

@@ -1157,7 +1157,7 @@ test_expect_success 'fsck with blob size equal to core.bigFileThreshold' '
 	git init big-blob-fsck &&
 	(
 		cd big-blob-fsck &&
-		test-tool genzeros 1024 >blob &&
+		test-tool genbytes 00 1024 >blob &&
 		git add blob &&
 		git commit -m "blob equal to threshold" &&
 		git repack -ad &&

index 6bc1c7a663..52639a62b0 100755
@@ -13,7 +13,7 @@ test_expect_success EXPENSIVE 'git apply rejects patches that are too large' '
 		+++ b/file
 		@@ -0,0 +1 @@
 		EOF
-		test-tool genzeros $((1024 * 1024 * 1023))
+		test-tool genbytes 00 $((1024 * 1024 * 1023))
 	} | test_must_fail git apply 2>err &&
 	test_grep "patch too large" err
 '

index 6cecfbeaa1..37b903359c 100755
@@ -522,7 +522,7 @@ corrupt_graph_and_verify() {
 	zero_pos=${4:-${orig_size}} &&
 	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 00 $(($orig_size - $zero_pos)) >>"full/$objdir/info/commit-graph" &&
 	corrupt_graph_verify "$grepstr"
 
 }

gitgitgadget

"Kristofer Karlsson via GitGitGadget" <[email protected]>

On Sun, Nov 08, 2026 at 07:15:03PM +0000, Kristofer Karlsson via GitGitGadget wrote:
[snip]

Nice improvement.

Yes, I think we have a few options to choose between:

1. keep the previous static. Technically this means that we should also document it as not being thread-safe. Currently it does not matter, since it's never called from anywhere else, but there is a slight code smell to it since the buffer no longer hosts a constant value.

2. remove static and keep the size. Since the stack is not complex or deep here it should be safe. It would break on platforms with a very small stack, but since it's only use by CI and developers any breakage would be immediately obvious and easy to fix.

3. remove static and shrink the buffer size. From my (limited) local experiment, it seems the buffer is larger than needed. We could shrink it without losing performance.

I don't have a strong opinion here; I went with (2) because it seemed like the smallest change but I'd be happy to change it.

If we go with static, I'd prefer to have a small comment above the function definition:

/* not thread-safe (due to the mutable static 'buf') */

Thanks,
Kristofer

v1 had the rename as a separate commit, and Junio preferred to have it squashed into the change. I do not have a strong opinion either way, so I will wait for a consensus before sending v3.

Good idea. It would also reject invalid counts, which are currently silently treated as 0.

That made me look at what the callers actually need:

  callers                     need
  t1007 (4x), t5318           NUL bytes
  t1051 (2x), t1450 (2x),     any byte, only the size matters
    t4141

t0613, t4205, t7006, t9850 any byte except NUL and newline
t5300 three different non-NUL bytes

So none of them needs a particular byte value, and the only special case is NUL, which "genzeros" already covers. For everything else, a plain character is the most readable:

Given that, I am now leaning towards keeping "genzeros" as it is and adding "genbytes <char> [<count>]" for the other callers, i.e. going back to v1 without its last patch (and without a separate rename patch). That avoids inventing a syntax for NUL, keeps the call sites readable, and also leaves the existing "genzeros" callers alone, which addresses Johannes' concern about the churn.

Junio, would that be acceptable, given your suggestion to use hex digits so that NUL does not need special treatment?

The rename is perhaps less important to do if genzeros is kept (though it also gains the genbytes function).

Thanks,
Kristofer

Am 08.11.26 um 17:34 schrieb Johannes Sixt:

Well, this patch passed for me[2], but I have no idea whether the particular tests were skipped due to the EXPENSIVE prerequisite or not.

[2] https://github.com/j6t/git/actions/runs/42583623620/job/129048078411

index 01b97c8888..5dedb9054e 100755
@@ -1182,7 +1182,8 @@ unfold () {
 '
 
 test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'set up huge commit' '
-	test-tool genzeros 2147483649 | tr "\000" "1" >expect &&
+	# 3 * 715827883 == 2147483649
+	printf %0715827883d 0 0 0 >expect &&
 	huge_commit=$(git commit-tree -F expect HEAD^{tree})
 '
 

Am 09.11.26 um 14:05 schrieb Kristofer Karlsson:

I wouldn't do that (and pay the memory price instead), because the next shell doesn't have printf as a builtin and spawns 2000 printf processes.

Now let's stop painting bikeshes. I'd be fine with your previous suggestion to have both test-tool genzeros and test-tool genbytes.

-- Hannes

I do not think such a comment is not warranted in this case, though. After all this is more or less throw-away code used only by CI and developers ;-)

I've mentioned this idea on coreutils or gnulib's mailing lists before, but I forget where and when. I've wanted to create a printf variant that doesn't allocate much memory and will just use putc_unlocked for each padding character, which would avoid the memory issue you bring up.

Most platforms will be limited to INT_MAX there, since that is what POSIX specifies is supplied with '*' [1]:

A field width, or precision, or both, may be indicated by an <asterisk> ('*'). In this case an argument of type int supplies the field width or precision.

E.g., in an invocation like this where the field width is supplied with another argument:

    $ printf '%0*d\n' 5 1
    00001

I think my original reason for wanting a different printf variant was escaping that limit.

For generating bytes where the values don't matter, in GNU coreutils we frequently use 'yes' for that. As a result, the implementation is very overengineered. E.g., it will use zero-copy since coreutils-9.11. Before that, it would reuse ARGV if it was contiguous and the arguments used to determine the printed string were long enough such that we weren't calling read/write on a tiny string, or it would generate a large buffer before using read and write. Here is the performance from before and after zero-copy was introduced on a beefy Power10 machine, stolen from the commit message [2]:

IBM,9043-MRX $ taskset 1 yes | taskset 2 pv > /dev/null ... [11.6GiB/s]
IBM,9043-MRX $ taskset 1 src/yes | taskset 2 pv > /dev/null ... [175GiB/s]

This was introduced around the time of the copy-fail vulnerability and various zero-copy vulnerabilities, where there was serious consideration given to nerfing splice and vmsplice. Luckily for us, that never happened.

FreeBSD, and therefore, I assume, macOS use something similar to our previous read/write method, which is fast enough. I think OpenBSD and NetBSD use puts on ARGV or the "y" string, so it's relatively slow.

GNU 'tr' doesn't handle multi-byte characters, so you can adjust that output if needed and remain relatively quick:

    $ timeout 10 yes 0 | LC_ALL=C tr -d '\n' | pv -r > /dev/null
    [ 616MiB/s]

I'm not sure if macOS 'tr' handles multibyte characters. But it might be worth checking if they do, and if they keep a fast case for single-byte locales. Doing that without duplicating code is quite tricky, which is part of the reason why we haven't done it in GNU 'tr' yet (but recent releases have improved multi-byte character support for other utilities, so hopefully it is handled soon).

Probably more information than you need, but hopefully it helps you generate slop sometimes needed for testing. :)

Collin

[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/printf.html
[2] https://github.com/coreutils/coreutils/commit/2b1c059e6a06eebbb721d010b1221ec54200cc33

Thanks, this was an interesting read (even if it may not solve the problem at hand).

To answer your question about macOS tr: it does not seem to have a single-byte fast path. Apple's tr (text_cmds tr/tr.c) goes through getwchar()/putwchar() for every byte even with LC_ALL=C, which is why "genzeros | tr" takes about 2 minutes for 2GB there, compared to a few seconds with GNU tr.

"yes" is nicely optimized, but without an option to drop the newlines we would still need to filter its output, and filtering is exactly what is slow on macOS. I suppose the primitive I would ideally want is something like "repeat <string> [<count>]".

In the end portability is the main problem: with tr and printf behaving this differently across platforms, a tiny test-tool helper is easier to reason about than any combination of shell tools.

Thanks,
Kristofer

From: Kristofer Karlsson <[email protected]>

Several tests create fixture data consisting of a single repeated character with "test-tool genzeros <count> | tr '\000' <char>". This works well on platforms where tr(1) is implemented efficiently.

The implementation of tr(1) on macOS supports multi-byte characters, and processes its input one character at a time through getwchar() and putwchar(), even in the C locale. For small inputs that does not matter, but t4205 generates more than 2GB of data this way, which makes that test much slower on macOS than on Linux: on the 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 <char> [<count>]" next to "genzeros", sharing the same code, which writes the requested character directly. Use it in all tests that piped "genzeros" through tr(1), not just t4205, so that it is the obvious choice for new tests. Callers that want NUL bytes keep using "genzeros".

While at it, parse the count with git_parse_signed(), so that an invalid count is reported as a usage error instead of being silently treated as 0.

This makes t4205 much faster on macOS in CI. The Linux jobs, shown for comparison, show smaller differences, which may partly be due to normal run-to-run variation:

  job                before    after   change
  osx-gcc            821.1s   243.3s   -70.4%
  osx-reftable       911.0s   194.0s   -78.7%
  linux-asan-ubsan   226.7s   222.1s    -2.0%
  linux-leaks        320.2s   298.8s    -6.7%

---
t: add "test-tool genbytes" to avoid slow tr(1) on macOS

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.

This adds "test-tool genbytes []" and uses it instead of "genzeros | tr", which is where the speedup comes from.

Changes since v2:

* "genbytes" takes a literal character again ("genbytes a") instead of two hex digits ("genbytes 61"), which Patrick found hard to read at the call sites.

* "genzeros" and its callers that want NUL bytes are kept as they are, and the patch that converted them to "genbytes 00" is dropped. This avoids churn in tests that do not need to change, as discussed with Johannes. Keeping "genzeros" also means that "genbytes" does not need special handling for NUL bytes.

* test-genzeros.c is no longer renamed, and "genbytes" lives next to "genzeros" in the same file. Since "genzeros" stays, the current file name is still valid and not worth churning.

* The buffer is static again instead of 256 KiB on the stack, as suggested by Junio, to avoid using a large part of the limited stack space. There is no risk of concurrent use in a run-once helper.

* The count is parsed with git_parse_signed(), so that an invalid count is a usage error instead of being silently treated as 0, as suggested by Patrick. This applies to "genzeros" as well.

Changes in v2 (subsequently reverted in v3):

* Squashed the rename of test-genzeros.c into the patch that changes it, as suggested by Junio.

* "genbytes" took the byte as two lowercase hexadecimal digits instead of a literal character, as suggested by Junio. This also worked for NUL bytes, so "genbytes 00" replaced special-casing "NUL".

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

Range-diff vs v2:

1: 54139a3a97 ! 1: 17e5baacdb 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 <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.
     +    Fix this by adding "test-tool genbytes <char> [<count>]" next to
     +    "genzeros", sharing the same code, which writes the requested
     +    character directly.  Use it in all tests that piped "genzeros"
     +    through tr(1), not just t4205, so that it is the obvious choice for
     +    new tests.  Callers that want NUL bytes keep using "genzeros".
     +
     +    While at it, parse the count with git_parse_signed(), so that an
     +    invalid count is reported as a usage error instead of being silently
     +    treated as 0.
      
          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
     -    is within the usual noise:
     +    for comparison, show smaller differences, which may partly be due to
     +    normal run-to-run variation:
      
            job                before    after   change
            osx-gcc            821.1s   243.3s   -70.4%
     @@ Commit message
      
          Signed-off-by: Kristofer Karlsson <[email protected]>
      
     - ## 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) ##
     + ## t/helper/test-genzeros.c ##
      @@
     -+#define DISABLE_SIGN_COMPARE_WARNINGS
     -+
     -+#include "test-tool.h"
     -+#include "git-compat-util.h"
     -+#include "hex-ll.h"
     -+
     + 
     + #include "test-tool.h"
     + #include "git-compat-util.h"
     ++#include "parse.h"
     + 
     +-int cmd__genzeros(int argc, const char **argv)
      +/*
      + * 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)
     -+{
     -+	char buf[256 * 1024];
     -+	ssize_t n;
     -+
     ++static int generate_bytes(unsigned char byte, intmax_t count)
     + {
     +-	/* static, so that it is NUL-initialized */
     +-	static const char zeros[256 * 1024];
     +-	intmax_t count;
     ++	static 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;
      +	memset(buf, byte, sizeof(buf));
     -+
     + 
     +-	/* Writing out individual NUL bytes is slow... */
      +	/* Writing out individual bytes is slow... */
     -+	while (count < 0)
     + 	while (count < 0)
     +-		if (xwrite(1, zeros, ARRAY_SIZE(zeros)) < 0)
      +		if (xwrite(1, buf, ARRAY_SIZE(buf)) < 0)
     -+			die_errno("write error");
     -+
     -+	while (count > 0) {
     + 			die_errno("write error");
     + 
     + 	while (count > 0) {
     +-		n = xwrite(1, zeros,
     +-			   count < ARRAY_SIZE(zeros)
     +-			   ? count : ARRAY_SIZE(zeros));
      +		n = xwrite(1, buf,
      +			   count < ARRAY_SIZE(buf)
      +			   ? count : ARRAY_SIZE(buf));
     -+
     -+		if (n < 0)
     -+			die_errno("write error");
     -+
     -+		count -= n;
     -+	}
     -+
     -+	return 0;
     -+}
     -+
     -+static intmax_t parse_count(int argc, const char **argv, int pos)
     -+{
     -+	return pos < argc ? strtoimax(argv[pos], NULL, 0) : -1;
     -+}
     + 
     + 		if (n < 0)
     + 			die_errno("write error");
     +@@ t/helper/test-genzeros.c: int cmd__genzeros(int argc, const char **argv)
     + 
     + 	return 0;
     + }
      +
      +int cmd__genbytes(int argc, const char **argv)
      +{
     -+	if (argc < 2 || argc > 3 || strlen(argv[1]) != 2 ||
     -+	    strspn(argv[1], "0123456789abcdef") != 2) {
     -+		fprintf(stderr, "usage: %s <hex-byte> [<count>]\n", argv[0]);
     ++	intmax_t count = -1;
     ++
     ++	if (argc < 2 || argc > 3 || strlen(argv[1]) != 1 ||
     ++	    (argc == 3 && !git_parse_signed(argv[2], &count, INTMAX_MAX))) {
     ++		fprintf(stderr, "usage: %s <char> [<count>]\n", argv[0]);
      +		return 1;
      +	}
      +
     -+	return generate_bytes(hex2chr(argv[1]), parse_count(argc, argv, 2));
     ++	return generate_bytes(argv[1][0], count);
      +}
      +
      +int cmd__genzeros(int argc, const char **argv)
      +{
     -+	if (argc > 2) {
     ++	intmax_t count = -1;
     ++
     ++	if (argc > 2 ||
     ++	    (argc == 2 && !git_parse_signed(argv[1], &count, INTMAX_MAX))) {
      +		fprintf(stderr, "usage: %s [<count>]\n", argv[0]);
      +		return 1;
      +	}
      +
     -+	return generate_bytes('\0', parse_count(argc, argv, 1));
     ++	return generate_bytes('\0', count);
      +}
      
     - ## 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 61 500 >logmsg &&
     ++		test-tool genbytes a 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 31 2147483649 >expect &&
     ++	test-tool genbytes 1 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 61 4096 >a &&
     -+	test-tool genbytes 62 4096 >b &&
     -+	test-tool genbytes 63 4096 >c &&
     ++	test-tool genbytes a 4096 >a &&
     ++	test-tool genbytes b 4096 >b &&
     ++	test-tool genbytes c 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 61 50000 |
     ++	test-tool genbytes a 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 61 |
     ++	test-tool genbytes a |
       	test_must_fail git shell 2>err &&
       	test_grep "too long" err
       '

2: f49af0e33f < -: ---------- t: replace "test-tool genzeros" with "genbytes 00"

 t/helper/test-genzeros.c          | 54 ++++++++++++++++++++++---------
 t/helper/test-tool.c              |  1 +
 t/helper/test-tool.h              |  1 +
 t/t0613-reftable-write-options.sh |  2 +-
 t/t4205-log-pretty-formats.sh     |  2 +-
 t/t5300-pack-object.sh            |  6 ++--
 t/t7006-pager.sh                  |  3 +-
 t/t9850-shell.sh                  |  2 +-
 8 files changed, 48 insertions(+), 23 deletions(-)

index b895436a32..e8d83d5012 100644
@@ -2,30 +2,28 @@
 
 #include "test-tool.h"
 #include "git-compat-util.h"
+#include "parse.h"
 
-int cmd__genzeros(int argc, const char **argv)
+/*
+ * Write "count" copies of "byte" to stdout, or an endless stream of them if
+ * "count" is negative.
+ */
+static int generate_bytes(unsigned char byte, intmax_t count)
 {
-	/* static, so that it is NUL-initialized */
-	static const char zeros[256 * 1024];
-	intmax_t count;
+	static 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;
+	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)
+		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));
+		n = xwrite(1, buf,
+			   count < ARRAY_SIZE(buf)
+			   ? count : ARRAY_SIZE(buf));
 
 		if (n < 0)
 			die_errno("write error");

@@ -35,3 +33,29 @@ int cmd__genzeros(int argc, const char **argv)
 
 	return 0;
 }
+
+int cmd__genbytes(int argc, const char **argv)
+{
+	intmax_t count = -1;
+
+	if (argc < 2 || argc > 3 || strlen(argv[1]) != 1 ||
+	    (argc == 3 && !git_parse_signed(argv[2], &count, INTMAX_MAX))) {
+		fprintf(stderr, "usage: %s <char> [<count>]\n", argv[0]);
+		return 1;
+	}
+
+	return generate_bytes(argv[1][0], count);
+}
+
+int cmd__genzeros(int argc, const char **argv)
+{
+	intmax_t count = -1;
+
+	if (argc > 2 ||
+	    (argc == 2 && !git_parse_signed(argv[1], &count, INTMAX_MAX))) {
+		fprintf(stderr, "usage: %s [<count>]\n", argv[0]);
+		return 1;
+	}
+
+	return generate_bytes('\0', count);
+}

index b71a22b43b..af70e2bec7 100644
@@ -33,6 +33,7 @@ static struct test_cmd cmds[] = {
 	{ "example-tap", cmd__example_tap },
 	{ "find-pack", cmd__find_pack },
 	{ "fsmonitor-client", cmd__fsmonitor_client },
+	{ "genbytes", cmd__genbytes },
 	{ "genrandom", cmd__genrandom },
 	{ "genzeros", cmd__genzeros },
 	{ "getcwd", cmd__getcwd },

index f2885b33d5..c35e7412af 100644
@@ -26,6 +26,7 @@ int cmd__env_helper(int argc, const char **argv);
 int cmd__example_tap(int argc, const char **argv);
 int cmd__find_pack(int argc, const char **argv);
 int cmd__fsmonitor_client(int argc, const char **argv);
+int cmd__genbytes(int argc, const char **argv);
 int cmd__genrandom(int argc, const char **argv);
 int cmd__genzeros(int argc, const char **argv);
 int cmd__getcwd(int argc, const char **argv);

index 0b27d75fe3..0b74ff6157 100755
@@ -146,7 +146,7 @@ test_expect_success 'small block size fails with large reflog message' '
 	(
 		cd repo &&
 		test_commit A &&
-		test-tool genzeros 500 | tr "\000" "a" >logmsg &&
+		test-tool genbytes a 500 >logmsg &&
 		cat >expect <<-EOF &&
 		fatal: update_ref failed for ref ${SQ}refs/heads/logme${SQ}: reftable: transaction failure: entry too large
 		EOF

index 01b97c8888..894d88c39b 100755
@@ -1182,7 +1182,7 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'log --pretty with huge commit mes
 '
 
 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 &&
 	huge_commit=$(git commit-tree -F expect HEAD^{tree})
 '
 

index e5ce478484..4fb6a22145 100755
@@ -9,9 +9,9 @@ test_description='git pack-object'
 
 test_expect_success 'setup' '
 	rm -f .git/index* &&
-	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 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 &&

index 75ca7a25da..8f1f71bf12 100755
@@ -662,8 +662,7 @@ test_expect_success 'setup trace2' '
 '
 
 test_expect_success 'setup large log output' '
-	test-tool genzeros 50000 |
-	tr "\000" "a" |
+	test-tool genbytes a 50000 |
 	sed "s/a/this is a long commit message/g" >commit-msg &&
 	git commit --allow-empty -F commit-msg
 '

index 27cc49bbd1..d56edebda7 100755
@@ -30,7 +30,7 @@ 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_must_fail git shell 2>err &&
 	test_grep "too long" err
 '

base-commit: fbd2d1e765da4d2931cadc4c62d3ca5d5b590e7e

gitgitgadget

Yes, it is a great realization that the file will have both genbytes and genzeros in it in the end, and there is no strong reason why it needs to be renamed.