Re: [PATCH v4 2/5] unpack-trees: add performance tracing

7 messages, 5 authors, 2018-08-14 · open the first message on its own page

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

From: Junio C Hamano <hidden>
Date: 2018-08-13 22:41:55

Jeff King [off-list ref] writes:
I can buy the argument that it's nice to have some form of profiling
that works everywhere, even if it's lowest-common-denominator. I just
wonder if we could be investing effort into tooling around existing
solutions that will end up more powerful and flexible in the long run.
Another thing I noticed is that the codepaths we would find
interesting to annotate with trace_performance_* stuff often
overlaps with the "slog" thing.  If the latter aims to eventually
replace GIT_TRACE (and if not, I suspect there is not much point
adding it in the first place), perhaps we can extend it to also
cover the need of these trace_performance_* calls, so that we do not
have to carry three different tracing mechanisms.

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

From: Jeff Hostetler <hidden>
Date: 2018-08-14 18:19:35


On 8/13/2018 6:41 PM, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
I can buy the argument that it's nice to have some form of profiling
that works everywhere, even if it's lowest-common-denominator. I just
wonder if we could be investing effort into tooling around existing
solutions that will end up more powerful and flexible in the long run.
Another thing I noticed is that the codepaths we would find
interesting to annotate with trace_performance_* stuff often
overlaps with the "slog" thing.  If the latter aims to eventually
replace GIT_TRACE (and if not, I suspect there is not much point
adding it in the first place), perhaps we can extend it to also
cover the need of these trace_performance_* calls, so that we do not
have to carry three different tracing mechanisms.
I'm looking at adding code to my SLOG (better name suggestions welcome)
patch series to eventually replace the existing git_trace facility.
And I would like to have a set of nested messages like Duy has proposed
be a part of that.

In an independent effort I've found the nested messages being very
helpful in certain contexts.  They are not a replacement for the
various platform tools, like PerfView and friends as discussed earlier
on this thread, but then again I can ask a customer to turn a knob and
run it again and send me the output and hopefully get a rough idea of
the problem -- without having them install a bunch of perf tools.

Jeff

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

From: Duy Nguyen <hidden>
Date: 2018-08-14 18:32:40

On Tue, Aug 14, 2018 at 8:19 PM Jeff Hostetler [off-list ref] wrote:
I'm looking at adding code to my SLOG (better name suggestions welcome)
patch series to eventually replace the existing git_trace facility.
Complement maybe. Replace, please no. I'd rather not stare at json messages.
-- 
Duy

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

From: Stefan Beller <hidden>
Date: 2018-08-14 18:44:23

On Tue, Aug 14, 2018 at 11:32 AM Duy Nguyen [off-list ref] wrote:
On Tue, Aug 14, 2018 at 8:19 PM Jeff Hostetler [off-list ref] wrote:
quoted
I'm looking at adding code to my SLOG (better name suggestions welcome)
patch series to eventually replace the existing git_trace facility.
Complement maybe. Replace, please no. I'd rather not stare at json messages.
From the sidelines: We'd only need one logging infrastructure in place, as the
formatting would be done as a later step? For local operations we'd certainly
find better formatting than json, and we figured that we might end up desiring
ProtocolBuffers[1] instead of JSon, so if it would be easy to change
the output of
the structured logging easily that would be great.

But AFAICT these series are all about putting the sampling points into the
code base, so formatting would be orthogonal to it?

Stefan

[1] https://developers.google.com/protocol-buffers/

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

From: Duy Nguyen <hidden>
Date: 2018-08-14 18:52:10

On Tue, Aug 14, 2018 at 8:44 PM Stefan Beller [off-list ref] wrote:
On Tue, Aug 14, 2018 at 11:32 AM Duy Nguyen [off-list ref] wrote:
quoted
On Tue, Aug 14, 2018 at 8:19 PM Jeff Hostetler [off-list ref] wrote:
quoted
I'm looking at adding code to my SLOG (better name suggestions welcome)
patch series to eventually replace the existing git_trace facility.
Complement maybe. Replace, please no. I'd rather not stare at json messages.
From the sidelines: We'd only need one logging infrastructure in place, as the
formatting would be done as a later step? For local operations we'd certainly
find better formatting than json, and we figured that we might end up desiring
ProtocolBuffers[1] instead of JSon, so if it would be easy to change
the output of
the structured logging easily that would be great.
These trace messages are made for human consumption. Granted
occasionally we need some processing but I find one liners mostly
suffice. Now we turn these into something made for machines, turning
people to second citizens. I've read these messages reformatted for
human, it's usually too verbose even if it's reformatted.
But AFAICT these series are all about putting the sampling points into the
code base, so formatting would be orthogonal to it?
It's not just sampling points. There's things like index id being
shown in the message for example. I prefer to keep free style format
to help me read. There's also things like indentation I do here to
help me read. Granted you could do all that with scripts and stuff,
but will we pass around in mail  dumps of json messages to be decoded
locally?
Stefan

[1] https://developers.google.com/protocol-buffers/


-- 
Duy

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

From: Jeff King <hidden>
Date: 2018-08-14 19:54:59

On Tue, Aug 14, 2018 at 08:51:41PM +0200, Duy Nguyen wrote:
quoted
But AFAICT these series are all about putting the sampling points into the
code base, so formatting would be orthogonal to it?
It's not just sampling points. There's things like index id being
shown in the message for example. I prefer to keep free style format
to help me read. There's also things like indentation I do here to
help me read. Granted you could do all that with scripts and stuff,
but will we pass around in mail  dumps of json messages to be decoded
locally?
I think you could have both forms using the same entry points sprinkled
through the code.

At GitHub we have a similar telemetry-ish thing, where we collect some
data points and then the resulting JSON is stored for every operation
(for a few weeks for read ops, and indefinitely attached to every ref
write).

And I've found that the storage and the trace-style "just show a
human-readable message to stderr" interface complement each other in
both directions:

 - you can output a human readable message that is sent immediately to
   the trace mechanism but _also_ becomes part of the telemetry. E.g.,
   imagine that one item in the json blob is "this is the last message
   from GIT_TRACE_FOO". Now you can push tracing messages into whatever
   plan you're using to store SLOG. We do this less with TRACE, and much
   more with error() and die() messages.

 - when a structured telemetry item is updated, we can still output a
   human-readable trace message with just that item. E.g., with:

     trace_performance(n, "foo");

   we could either store a json key (perf.foo=n) or output a nicely
   formatted string like we do now, depending on what the user has
   configured (or even both, of course).

It helps if the sampling points give enough information to cover both
cases (as in the trace_performance example), but you can generally
shoe-horn unstructured data into the structured log, and pretty-print
structured data.

-Peff

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

From: Jeff Hostetler <hidden>
Date: 2018-08-14 20:14:04


On 8/14/2018 2:44 PM, Stefan Beller wrote:
On Tue, Aug 14, 2018 at 11:32 AM Duy Nguyen [off-list ref] wrote:
quoted
On Tue, Aug 14, 2018 at 8:19 PM Jeff Hostetler [off-list ref] wrote:
quoted
I'm looking at adding code to my SLOG (better name suggestions welcome)
patch series to eventually replace the existing git_trace facility.
Complement maybe. Replace, please no. I'd rather not stare at json messages.
 From the sidelines: We'd only need one logging infrastructure in place, as the
formatting would be done as a later step? For local operations we'd certainly
find better formatting than json, and we figured that we might end up desiring
ProtocolBuffers[1] instead of JSon, so if it would be easy to change
the output of
the structured logging easily that would be great.

But AFAICT these series are all about putting the sampling points into the
code base, so formatting would be orthogonal to it?

Stefan

[1] https://developers.google.com/protocol-buffers/
Last time I checked, protocol-buffers has a C++ binding but not
a C binding.

I've not had a chance to use pbuffers, so I have to ask what advantages
would they have over JSON or some other similar self-describing format?
And/or would it be possible for you to tail the json log file and
convert it to whatever format you preferred?

It seems like the important thing is to capture structured data
(whatever the format) to disk first.

Jeff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help