Re: [PATCH v2] pack-objects: trace pack bytes written
From: Patrick Steinhardt <hidden>
Date: 2026-08-20 05:41:40
On Wed, Aug 19, 2026 at 04:28:10PM -0700, friel@openai.com wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 1ec5b6f206..252530172c 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c@@ -1389,6 +1390,8 @@ static void write_pack_file(void) display_progress(progress_state, written); } + bytes_written += hashfile_total(f) + + the_repository->hash_algo->rawsz; if (pack_to_stdout) { /* * We never fsync when writing to stdout since we may
I guess the addition here accounts for the trailing hash written by the hashfile. If so, shouldn't we also use the algortihm that the hashfile uses in the first place via `f->algop->rawsz`?
quoted hunk ↗ jump to hunk
@@ -1510,6 +1513,8 @@ static void write_pack_file(void) written, nr_result); trace2_data_intmax("pack-objects", the_repository, "write_pack_file/wrote", nr_result); + trace2_data_intmax("pack-objects", the_repository, + "write_pack_file/wrote_bytes", bytes_written); } static int no_try_delta(const char *path)
The "write_pack_file/wrote" event is quite awkwardly named, if you ask me, as it's not immediately obvious what exactly it's counting, and the second metric may make this even more confusing. In retrospect it would've been preferable to call this "wrote_objects" to clarify. I don't really think we guarantee any kind of stability around those traces, so we could in theory change it here, too. But I don't feel like my argument is strong enough to really warrant such a change, so maybe we should just leave it as-is. Thanks! Patrick