Re: [PATCH v3 14/14] ref-filter: parse objects on demand

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 22, 2025, 15:27 UTC
Message-ID: <xmqqtszryn10.fsf@gitster.g>
In-Reply-To: <20251022-b4-pks-ref-filter-skip-parsing-objects-v3-14-eb9f71985ef0@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 14 quoted lines

> +static int get_or_parse_object(struct expand_data *data, const char *refname,
> +			       struct object **object, struct strbuf *err, int *eaten)
> +{
> +	if (!data->maybe_object) {
> +		data->maybe_object = parse_object_buffer(the_repository, &data->oid, data->type,
> +							 data->size, data->content, eaten);
> +		if (!data->maybe_object)
> +			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
> +					       oid_to_hex(&data->oid), refname);
> +	}
> +
> +	*object = data->maybe_object;
> +	return 0;
> +}

I wonder if the calling convention for this function can use further ergonomic improvements, after seeing that ...

Show 9 quoted lines

> +		if (!tag) {
> +			struct object *object;
> +
> +			ret = get_or_parse_object(data, refname, &object, err, eaten);
> +			if (ret < 0)
> +				return ret;
> +
> +			tag = (struct tag *) object;
> +		}

... this pattern is repeated three times in the patch. If the helper cannot legitimately return 0 (success) while stuffing NULL to the object, then something like this

		if (!tag) {
			tag = (struct tag *) get_or_parse(data,	refname, err, eaten);
			if (!tag)
				return -1;
		}

ought to be sufficient for this caller and the other two. I also wonder if this can further be simplified, but stopping at the above is fine to me.

In any case, the overall idea of this step is very much in line with the original vision of ref-filter.c:populate_value() to try avoiding unnecessary work as much as possible, and it is very much welcome.

Nice performance improvements.

Will queue.  Thanks.