{"thread":{"id":"62524","subject":"[PATCH] t/perf: use 'test_file_size' in more places","startedAt":"2024-11-21T20:29:27Z","lastAt":"2024-11-22T08:28:07Z","messageCount":3,"participants":["Taylor Blau","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"507837","messageId":"50c1368630684f235548d2e9a68d4de3745b5fe6.1732220875.git.me@ttaylorr.com","threadId":"62524","inReplyTo":null,"subject":"[PATCH] t/perf: use 'test_file_size' in more places","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-11-21T20:29:24Z","receivedAt":"2024-11-21T20:29:27Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The perf test suite prefers to use test_file_size over 'wc -c' when\ninside of a test_size block. One advantage is that accidentally writign\n\"wc -c file\" (instead of \"wc -c <file\") does not inadvertently break the\ntests (since the former will include the filename in the output of wc).\n\nBoth of the two uses of test_size use \"wc -c\", but let's convert those\nto the more conventional test_file_size helper instead.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\nI noticed while reviewing Stolee's --full-name-hash series that he added\nnew test_size tests that use the test_file_size helper instead of \"wc\n-c\". I thought it would be good to clean up the existing uses of \"wc -c\"\nin the perf suite as a result, which is what this patch does.\n\n t/perf/p5311-pack-bitmaps-fetch.sh | 2 +-\n t/perf/p5332-multi-pack-reuse.sh   | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/perf/p5311-pack-bitmaps-fetch.sh b/t/perf/p5311-pack-bitmaps-fetch.sh\nindex 426fab87e32..047efb995d6 100755\n--- a/t/perf/p5311-pack-bitmaps-fetch.sh\n+++ b/t/perf/p5311-pack-bitmaps-fetch.sh\n@@ -39,7 +39,7 @@ test_fetch_bitmaps () {\n \t\t'\n\n \t\ttest_size \"size   $title\" '\n-\t\t\twc -c <tmp.pack\n+\t\t\ttest_file_size tmp.pack\n \t\t'\n\n \t\ttest_perf \"client $title (lookup=$1)\" '\ndiff --git a/t/perf/p5332-multi-pack-reuse.sh b/t/perf/p5332-multi-pack-reuse.sh\nindex 5c6c575d62c..d1c89a8b7db 100755\n--- a/t/perf/p5332-multi-pack-reuse.sh\n+++ b/t/perf/p5332-multi-pack-reuse.sh\n@@ -73,7 +73,7 @@ do\n \t\t\"\n\n \t\ttest_size \"clone size for $nr_packs-pack scenario ($reuse-pack reuse)\" '\n-\t\t\twc -c <result\n+\t\t\ttest_file_size result\n \t\t'\n \tdone\n done\n\nbase-commit: 4083a6f05206077a50af7658bedc17a94c54607d\n--\n2.47.0.237.gc601277f4c4\n"},{"id":"507870","messageId":"xmqqed34dri0.fsf@gitster.g","threadId":"62524","inReplyTo":"50c1368630684f235548d2e9a68d4de3745b5fe6.1732220875.git.me@ttaylorr.com","subject":"Re: [PATCH] t/perf: use 'test_file_size' in more places","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-22T00:47:03Z","receivedAt":"2024-11-22T00:47:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> The perf test suite prefers to use test_file_size over 'wc -c' when\n> inside of a test_size block. One advantage is that accidentally writign\n> \"wc -c file\" (instead of \"wc -c <file\") does not inadvertently break the\n> tests (since the former will include the filename in the output of wc).\n\nAnother advantage is, instead of reading through the file and\ncounting bytes, the helper uses stat() without reading.  For a\nperformance test that potentially deals with a large-ish file, it\nprobably counts (pun intended) more.\n\nWill queue.  Thanks.\n\n\n"},{"id":"507887","messageId":"20241122082805.GC1179306@coredump.intra.peff.net","threadId":"62524","inReplyTo":"50c1368630684f235548d2e9a68d4de3745b5fe6.1732220875.git.me@ttaylorr.com","subject":"Re: [PATCH] t/perf: use 'test_file_size' in more places","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-11-22T08:28:05Z","receivedAt":"2024-11-22T08:28:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 21, 2024 at 03:29:24PM -0500, Taylor Blau wrote:\n\n> The perf test suite prefers to use test_file_size over 'wc -c' when\n> inside of a test_size block. One advantage is that accidentally writign\n> \"wc -c file\" (instead of \"wc -c <file\") does not inadvertently break the\n> tests (since the former will include the filename in the output of wc).\n\nThe use of \"prefers\" here confused me, because I did not think we used\ntest_file_size at all yet. I am certainly OK with arguing that we\nshould, but I was just confused on first read. Maybe you mean \"s/perf//\"?\n\nAlso, s/writign/writing/.\n\n> I noticed while reviewing Stolee's --full-name-hash series that he added\n> new test_size tests that use the test_file_size helper instead of \"wc\n> -c\". I thought it would be good to clean up the existing uses of \"wc -c\"\n> in the perf suite as a result, which is what this patch does.\n\nPerhaps we should also touch the one in t/perf/README, which points\npeople using test_size to using \"wc -c\" in the first place?\n\n-Peff\n"}]}