Re: [PATCH 1/8] trace2: create new combined trace facility

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Sep 5, 2018, 15:51 UTC
Message-ID: <eb77f46b-8d04-a1c7-ae31-7ea5e2456cfd@jeffhostetler.com>
In-Reply-To: <xmqqzhww6gl8.fsf@gitster-ct.c.googlers.com>

On 9/4/2018 6:30 PM, Junio C Hamano wrote:

Show 18 quoted lines

> Stefan Beller <sbeller@google.com> writes:
> 
>>> The API defines both fixed-field and printf-style functions.
>>>
>>> The trace2 performance tracing includes thread-specific function
>>> nesting and timings.
>>
>> So this only adds the new API, and we need to merge the TRACE
>> into the TRACE2 later?
> 
> If this is a rhetorical question implying that it would be best if
> the existing trace() were rewritten to be built on top of trace2()
> while building this series, especially before adding new callsites
> that directly use trace2(), I may share that feeling.  I haven't
> studied this new round deeply enough to see how realistic it would
> be, though.
> 
>

I wanted to come up with a unified API that we liked and was sufficient to handle the default-key, performance-key, the new event-key (currently supporting JSON output), and any other formats/variants that we want (protobufs, syslog, etc).

And hopefully get some agreement on it and see what else we want from it.

And then look at converting the trace_printf() and trace_performance() calls to trace2. Clearly, I could just replace the existing printf style calls to trace2_printf's, but I thought it would be better to look at them and promote them to higher-level functions. For example, the trace_argv_printf() calls are generally used to dump the command line arguments for the current process or spawned child processes. I have trace2_start() and trace2_child_start() that captures the argv and additional information about it. (The "why" if you will.) So the trace_argv_* forms can go away.

Likewise, all of the trace_performance() and trace_performance_since() can be converted to trace2_region_enter/_leave calls. And those forms can be removed from trace.[ch].

The net-net is that trace.[ch] shrinks in a short sequence of commits on top of my initial trace2 commit in a reroll of this patch series. (and replacing some of the demonstration commits in V1)

Then I'll address the trace_printf_key() forms, since they write to alternate stream.

Then I can delete trace.[ch]. And we can consider renaming trace2_* functions and/or GIT_TR2_* env vars if we want.

I wanted to avoid rewriting trace.[ch] just to delete them later in the same patch series.

Jeff