| Message ID | 50c1368630684f235548d2e9a68d4de3745b5fe6.1732220875.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | t/perf: use 'test_file_size' in more places | expand |
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.
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
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
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(-) base-commit: 4083a6f05206077a50af7658bedc17a94c54607d -- 2.47.0.237.gc601277f4c4