Re: [PATCH] trace2: log progress time and throughput

From: Junio C Hamano <gitster@pobox.com>
Date: May 12, 2020, 22:14 UTC
Message-ID: <xmqqr1voajii.fsf@gitster.c.googlers.com>
In-Reply-To: <20200512214420.36329-1-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 8 quoted lines

> Rather than teaching only one operation, like 'git fetch', how to write
> down throughput to traces, we can learn about a wide range of user
> operations that may seem slow by adding tooling to the progress library
> itself. Operations which display progress are likely to be slow-running
> and the kind of thing we want to monitor for performance anyways. By
> showing object counts and data transfer size, we should be able to
> make some derived measurements to ensure operations are scaling the way
> we expect.

Excellent observation ;-)

Show 8 quoted lines

>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> One note: by putting trace collection into the progress library, we end
> up with data events which have titles like "Receiving objects" - not
> very machine-parseable. An alternative might be to ask for a
> machine-readable title in the progress struct, but I didn't think it was
> worth the code churn.

Perhaps make it an optional member of the struct, and convert only a selected few callers to set it and see how well it helps, while leaving all the other callers set NULL to the member to keep showing not-very-useful titles?

> CI run here, although it failed on the same error Junio noted today[1]:

I think Dscho's latest patch, even though it didn't mention my message, is to fix that error. So with that queued in the same batch, we'll see a full CI coverage again.

Will queue.  Thanks.