Subject: Re: [RFC PATCH] trace2 API: don't save a copy of constant "thread_name"

From: Junio C Hamano <[email-masked]>
Date: Thu, 06 Oct 2022 18:16:55 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> A cleaned up version of the test code I had on top of "master", RFC
> because I may still be missing some context here. E.g. maybe there's a
> plan to dynamically construct these thread names?

That's nice to learn, indeed.

> +void jw_object_string_thread(struct json_writer *jw, const char *thread_name,
> +			     int thread_id)
> +{
> +	object_common(jw, "thread");
> +	strbuf_addch(&jw->json, '"');
> +	jw_strbuf_add_thread_name(&jw->json, thread_name, thread_id);
> +	strbuf_addch(&jw->json, '"');
> +}

...

> @@ -107,9 +109,11 @@ static void perf_fmt_prepare(const char *event_name,
>  	}
>  
>  	strbuf_addf(buf, "d%d | ", tr2_sid_depth());
> -	strbuf_addf(buf, "%-*s | %-*s | ", TR2_MAX_THREAD_NAME,
> -		    ctx->thread_name.buf, TR2FMT_PERF_MAX_EVENT_NAME,
> -		    event_name);
> +	oldlen = buf->len;
> +	jw_strbuf_add_thread_name(buf, ctx->thread_name, ctx->thread_id);
> +	padlen = TR2_MAX_THREAD_NAME - (buf->len - oldlen);;
> +	strbuf_addf(buf, "%-*s | %-*s | ", padlen, "",
> +		    TR2FMT_PERF_MAX_EVENT_NAME, event_name);

Having to do strbuf_addf() many times may negatively affect perf_*
stuff, if this code is invoked in the hot path.  I however tend to
treat anything that involves an I/O not performance critical, and
this certainly falls into that category.