Re: [PATCH v4 4/7] builtin/repo: introduce stats subcommand

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 27, 2025, 16:32 UTC
Message-ID: <xmqqfrc797pk.fsf@gitster.g>
In-Reply-To: <20250927145049.723341-5-jltobler@gmail.com>

Justin Tobler <jltobler@gmail.com> writes:

> The shape of a repository's history can have huge impacts on the
> performance and health of the repository itself. Currently, Git lacks a
> means to surface key stats/information regarding the shape of a
> repository via a single command.

Talking about "shape of a repository's *history*" may negatively affect your goal here. If a project is overly mergy with many octopus merges, it would have huge impacts on the performance to run "git bisect" over its history, so it may be interesting to know the ratio of the merge commits in the total commits, and also the average number of merge parents. But after you obtain such numbers, you cannot do anything about it, as you cannot afford to rewrite its history only to improve the "performance and health".

And that is what makes "key stats" relative to your goal. If your goal is to give stats on the things you can control (e.g., how long a typical delta chain is, how many loose objects there are that can be moved to a packfile, how small would your object database would become if you prune all the unreachable objects), that would cut off some stats that may still be interesting but may not contribute to address "huge impacts on the performance and health".

With Devil's advocate hat on, a single command that gives a set of stats that are "key" to a goal of a single use case may not be as useful as a collection of commands, each of which gives stats on one aspect of the repository, that can be combined to help you address various different goals.

Show 5 quoted lines

> To allow users to more readily identify potential issues for a
> repository, introduce the "stats" subcommand in git-repo(1) to output
> stats for the repository that may be of interest to users. The goal of
> this subcommand is to eventually provide similar functionality to
> git-sizer(1), but natively in Git.

So, it is needless to say that the kind of "stats" obtained by such a single tool needs to be chosen carefully, but more importantly, its output should give users actionable output, as whoever designed such a tool and chose what "key stats" are has a clear idea on various aspects of repository. "stats" measure the health of the repository against certain yardstick, but it should come with a clear instruction to make use of that measurement. The tool may say "the stats indicate that you have commits that touch too many paths at the same time". The users need to be know what consequence of that finding is, and what they can do about it.

For example, what would the user do with the new knowledge that the repository has 100x as many local branches as there are remote-tracking branches? Without breaking down these numerous local branches into those that are still used in active development (hint: peek into their reflog), kept as historical landmarks, past development that has already been merged (hint: "git branch --list --merged origin/master"), or abandoned cruft that hasn't been touched with some changes that are not merged anywhere, the users would not know what to do.

Show 8 quoted lines

> +`stats`::
> +	Retrieve statistics about the current repository. The following kinds
> +	of information are reported:
> ++
> +* Reference counts categorized by type
> +
> ++
> +The table output format may change and is not intended for machine parsing.

Do we eventually want to give another format that is intended for machine parsing?

In a format meant for human consumption, is it still sensible to target fixed-column terminals these days? Rather, would they want prettier-formatted html, or csv that they can easily import to spreadsheet? (these are not objections but genuine questions).

Show 24 quoted lines

> +static void stats_table_vaddf(struct stats_table *table,
> +			      struct stats_table_entry *entry,
> +			      const char *format, va_list ap)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	struct string_list_item *item;
> +	char *formatted_name;
> +	size_t name_width;
> +
> +	strbuf_vaddf(&buf, format, ap);
> +	formatted_name = strbuf_detach(&buf, NULL);
> +	name_width = utf8_strwidth(formatted_name);
> +
> +	item = string_list_append_nodup(&table->rows, formatted_name);
> +	item->util = entry;
> +
> +	if (name_width > table->name_col_width)
> +		table->name_col_width = name_width;
> +	if (entry) {
> +		size_t value_width = utf8_strwidth(entry->value);
> +		if (value_width > table->value_col_width)
> +			table->value_col_width = value_width;
> +	}
> +}

OK, accumulate while measuring, so that you can compute the max width of these things before writing them out, which is quite bog-standard way to collect data.

> +static inline size_t max_size_t(size_t a, size_t b)
> +{
> +	return (a > b) ? a : b;
> +}

Heh.

Show 14 quoted lines

> +static void stats_table_print(const struct stats_table *table)
> +{
> +	const char *name_col_title = _("Repository stats");
> +	const char *value_col_title = _("Value");
> +	size_t name_title_len = utf8_strwidth(name_col_title);
> +	size_t value_title_len = utf8_strwidth(value_col_title);
> +	struct string_list_item *item;
> +	int name_col_width;
> +	int value_col_width;
> +
> +	name_col_width = cast_size_t_to_int(
> +		max_size_t(table->name_col_width, name_title_len));
> +	value_col_width = cast_size_t_to_int(
> +		max_size_t(table->value_col_width, value_title_len));

If table->name_col_width and table->value_col_width were int to begin with, none of these casts would have been necessary. Aren't we overusing size_t to count things that are not memory allocations?

Show 9 quoted lines

> +	printf("| %-*s | %-*s |\n", name_col_width, name_col_title,
> +	       value_col_width, value_col_title);
> +	printf("| ");
> +	for (int i = 0; i < name_col_width; i++)
> +		putchar('-');
> +	printf(" | ");
> +	for (int i = 0; i < value_col_width; i++)
> +		putchar('-');
> +	printf(" |\n");

I wonder if people want to use unicode "Box Drawing" block and other fancier things, as we assume utf8 for names and values, in which case these printf would need to be "translatable", but locale administrators should not have more say than others what kind of line drawing elements are to be used, so perhaps the above is good enough at least for now.