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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 7, 2022, 01:16 UTC
Message-ID: <xmqqo7uoh1q0.fsf@gitster.g>
In-Reply-To: <RFC-patch-1.1-8563d017137-20221007T010829Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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.

Show 8 quoted lines

> +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, '"');
> +}

...

Show 12 quoted lines

> @@ -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.