Re: [PATCH] cat-file: skip expanding default format

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 7, 2022, 05:56 UTC
Message-ID: <xmqqmti2uwzr.fsf@gitster.g>
In-Reply-To: <pull.1221.git.git.1646429845306.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 11 quoted lines

> From: John Cai <johncai86@gmail.com>
>
> When format is passed into --batch, --batch-check, --batch-command,
> the format gets expanded. When nothing is passed in, the default format
> is set and the expand_format() gets called.
>
> We can save on these cycles by hardcoding how to print the
> information when nothing is passed as the format, or when the default
> format is passed. There is no need for the fully expanded format with
> the default. Since batch_object_write() happens on every object provided
> in batch mode, we get a nice performance improvement.

That is OK in principle, but ...

Show 9 quoted lines

> +	if (!opt->format && !opt->print_contents) {
> +		char buf[1024];
> +
> +		print_default_format(buf, 1024, data);
> +		batch_write(opt, buf, strlen(buf));
> +		goto cleanup;
> +	}
> +
> +	fmt = opt->format ? opt->format : default_format;

... instead of doing this, wouldn't it be nicer to base the decision to call print_default_format() on purely the contents of the format, i.e.

	fmt = opt->format ? opt->format : default_format;
	if (!strcmp(fmt, DEFAULT_FORMAT) && !opt->print_contents) {
		... the above print_default_format() call block here ...
		goto cleanup;
	}

where DEFAULT_FORMAT is

#define DEFAULT_FORMAT = "%(objectname) %(objecttype) %(objectsize)"

and

Show 7 quoted lines

> @@ -515,9 +543,7 @@ static int batch_objects(struct batch_options *opt)
>  	struct expand_data data;
>  	int save_warning;
>  	int retval = 0;
> -
> -	if (!opt->format)
> -		opt->format = "%(objectname) %(objecttype) %(objectsize)";

retain the defaulting with

	if (!opt->format)
		opt->format = DEFAULT_FORMAT;

instead of making opt->format == NULL to mean something special?

That way, even if the user-input happens to name the format that is identical to DEFAULT_FORMAT, because we only care what the format is, and not where the format comes from, we will get the same optimization. Wouldn't it make more sense?