From: Jeff Hostetler <git@jeffhostetler.com>
Date: Wed, 05 Sep 2018 15:51:40 GMT
Subject: Re: [PATCH 1/8] trace2: create new combined trace facility
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:
> 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