Re: [PATCH 03/16] refs: implement releasing ref storages

From: Patrick Steinhardt <ps@pks.im>
Date: May 17, 2024, 07:09 UTC
Message-ID: <ZkcCoY8_S2nSXSyU@tanuki>
In-Reply-To: <xmqqjzjtr602.fsf@gitster.g>

On Thu, May 16, 2024 at 11:01:01AM -0700, Junio C Hamano wrote:

Show 16 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
> 
> >> +static void debug_release(struct ref_store *refs)
> >> +{
> >> +	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
> >
> > We should probably add a trace here, using `trace_printf_key()`
> 
> A totally ignorant question.  Should we be adding more traces with
> trace_* API instead of trace2_* API?  If the latter aims to cover
> superset of use cases the former did, I was hoping that we can
> eventually deprecate the former, hence this question.  Of course We
> could add a compatiblity layer that emulates trace_* API with a thin
> wrapper around trace2_* API, but if we do not add new callers, it
> may still be feasible to directly migrate the callers to use trace2_
> API without having to invent such compatibility wrappers.

I cannot really answer this question as I ain't got much of a clue around the tracing APIs. But in this case I think we should indeed add this via `trace_printf_key()` so that we remain consistent with all the other wrappers in the debug store. I'd argue that either all functions here should use trace or trace2, but not a mixture.

Patrick