Re: [PATCH v2 2/5] builtin/repo: collect largest inflated objects
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 26, 2026, 19:50 UTC
- Message-ID
- <xmqqv7fj1dzg.fsf@gitster.g>
- In-Reply-To
- <20260223174120.2356504-3-jltobler@gmail.com>
Justin Tobler <jltobler@gmail.com> writes:
Show 20 quoted lines
> @@ -485,6 +514,23 @@ static void structure_keyvalue_print(struct repo_structure *stats,
> printf("objects.tags.disk_size%c%" PRIuMAX "%c", key_delim,
> (uintmax_t)stats->objects.disk_sizes.tags, value_delim);
>
> + printf("objects.commits.max_size%c%" PRIuMAX "%c", key_delim,
> + (uintmax_t)stats->objects.largest.commit_size.value, value_delim);
> + printf("objects.commits.max_size_oid%c%s%c", key_delim,
> + oid_to_hex(&stats->objects.largest.commit_size.oid), value_delim);
> + printf("objects.trees.max_size%c%" PRIuMAX "%c", key_delim,
> + (uintmax_t)stats->objects.largest.tree_size.value, value_delim);
> + printf("objects.trees.max_size_oid%c%s%c", key_delim,
> + oid_to_hex(&stats->objects.largest.tree_size.oid), value_delim);
> + printf("objects.blobs.max_size%c%" PRIuMAX "%c", key_delim,
> + (uintmax_t)stats->objects.largest.blob_size.value, value_delim);
> + printf("objects.blobs.max_size_oid%c%s%c", key_delim,
> + oid_to_hex(&stats->objects.largest.blob_size.oid), value_delim);
> + printf("objects.tags.max_size%c%" PRIuMAX "%c", key_delim,
> + (uintmax_t)stats->objects.largest.tag_size.value, value_delim);
> + printf("objects.tags.max_size_oid%c%s%c", key_delim,
> + oid_to_hex(&stats->objects.largest.tag_size.oid), value_delim);The repetition tires reviewers' eyes. I am reasonably sure if there were an intentional copy-and-paste error, I wouldn't be able to spot it. But I tried to be careful and read it over three times ;-).
Show 12 quoted lines
> @@ -553,6 +599,15 @@ struct count_objects_data {
> struct progress *progress;
> };
>
> +static void check_largest(struct object_data *data, struct object_id *oid,
> + size_t value)
> +{
> + if (value > data->value) {
> + oidcpy(&data->oid, oid);
> + data->value = value;
> + }
> +}How important is it for this application to end up with a valid value in data->oid?
If data->value is initialized to a valid value, instead of an impossible sentinel value that is strictly smaller than any valid values, this can leave data->value to a valid value from an existing object without recording its object name. Imagine a repository with a single empty blob, and data->value initialized to zero (it cannot be initialized to a sentinel -1, as use of size_t here makes it impossible to have any reasonable sentinel values).
Show 15 quoted lines
> @@ -138,6 +158,14 @@ test_expect_success SHA1 'keyvalue and nul format' ' > objects.trees.disk_size=$(object_type_disk_usage tree) > objects.blobs.disk_size=$(object_type_disk_usage blob) > objects.tags.disk_size=$(object_type_disk_usage tag) > + objects.commits.max_size=221 > + objects.commits.max_size_oid=de3508174b5c2ace6993da67cae9be9069e2df39 > + objects.trees.max_size=1335 > + objects.trees.max_size_oid=09931deea9d81ec21300d3e13c74412f32eacec5 > + objects.blobs.max_size=11 > + objects.blobs.max_size_oid=eaeeedced46482bd4281fda5a5f05ce24854151f > + objects.tags.max_size=132 > + objects.tags.max_size_oid=1ee0f2b16ea37d895dbe9dbd76cd2ac70446176c > EOF > > git repo structure --format=keyvalue >out 2>err &&