[PATCH] t/perf: use 'test_file_size' in more places

[PATCH] t/perf: use 'test_file_size' in more places

[Taylor Blau]

The perf test suite prefers to use test_file_size over 'wc -c' when
inside of a test_size block. One advantage is that accidentally writign
"wc -c file" (instead of "wc -c <file") does not inadvertently break the
tests (since the former will include the filename in the output of wc).

Both of the two uses of test_size use "wc -c", but let's convert those
to the more conventional test_file_size helper instead.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
I noticed while reviewing Stolee's --full-name-hash series that he added
new test_size tests that use the test_file_size helper instead of "wc
-c". I thought it would be good to clean up the existing uses of "wc -c"
in the perf suite as a result, which is what this patch does.

 t/perf/p5311-pack-bitmaps-fetch.sh | 2 +-
 t/perf/p5332-multi-pack-reuse.sh   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/perf/p5311-pack-bitmaps-fetch.sh b/t/perf/p5311-pack-bitmaps-fetch.sh
index 426fab87e32..047efb995d6 100755
--- a/t/perf/p5311-pack-bitmaps-fetch.sh
+++ b/t/perf/p5311-pack-bitmaps-fetch.sh
@@ -39,7 +39,7 @@ test_fetch_bitmaps () {
 		'

 		test_size "size   $title" '
-			wc -c <tmp.pack
+			test_file_size tmp.pack
 		'

 		test_perf "client $title (lookup=$1)" '
diff --git a/t/perf/p5332-multi-pack-reuse.sh b/t/perf/p5332-multi-pack-reuse.sh
index 5c6c575d62c..d1c89a8b7db 100755
--- a/t/perf/p5332-multi-pack-reuse.sh
+++ b/t/perf/p5332-multi-pack-reuse.sh
@@ -73,7 +73,7 @@ do
 		"

 		test_size "clone size for $nr_packs-pack scenario ($reuse-pack reuse)" '
-			wc -c <result
+			test_file_size result
 		'
 	done
 done

base-commit: 4083a6f05206077a50af7658bedc17a94c54607d
--
2.47.0.237.gc601277f4c4

Re: [PATCH] t/perf: use 'test_file_size' in more places

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> The perf test suite prefers to use test_file_size over 'wc -c' when
> inside of a test_size block. One advantage is that accidentally writign
> "wc -c file" (instead of "wc -c <file") does not inadvertently break the
> tests (since the former will include the filename in the output of wc).

Another advantage is, instead of reading through the file and
counting bytes, the helper uses stat() without reading.  For a
performance test that potentially deals with a large-ish file, it
probably counts (pun intended) more.

Will queue.  Thanks.

Re: [PATCH] t/perf: use 'test_file_size' in more places

[Jeff King]

On Thu, Nov 21, 2024 at 03:29:24PM -0500, Taylor Blau wrote:

> The perf test suite prefers to use test_file_size over 'wc -c' when
> inside of a test_size block. One advantage is that accidentally writign
> "wc -c file" (instead of "wc -c <file") does not inadvertently break the
> tests (since the former will include the filename in the output of wc).

The use of "prefers" here confused me, because I did not think we used
test_file_size at all yet. I am certainly OK with arguing that we
should, but I was just confused on first read. Maybe you mean "s/perf//"?

Also, s/writign/writing/.

> I noticed while reviewing Stolee's --full-name-hash series that he added
> new test_size tests that use the test_file_size helper instead of "wc
> -c". I thought it would be good to clean up the existing uses of "wc -c"
> in the perf suite as a result, which is what this patch does.

Perhaps we should also touch the one in t/perf/README, which points
people using test_size to using "wc -c" in the first place?

-Peff