On 9/4/2018 6:30 PM, Junio C Hamano wrote:
Stefan Beller <sbel...@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

Reply via email to