Re: [PATCH 2/2] pkt-line: show packets in async processes as "sideband"

From: Stefan Beller <sbeller@google.com>
Date: Sep 1, 2015, 22:23 UTC
Message-ID: <CAGZ79kZKJF2epU=2gb7jFWni2tBu03dYGgttvZeW40ra=r2iiA@mail.gmail.com>
In-Reply-To: <xmqq1tehrdoa.fsf@gitster.mtv.corp.google.com>

On Tue, Sep 1, 2015 at 3:13 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> Jeff King <peff@peff.net> writes:
>
>> What we really need is thread-local storage for
>> packet_trace_identity. But the async code does not provide
>> an interface for that, and it would be messy to add it here
>> (we'd have to care about pthreads, initializing our
>> pthread_key_t ahead of time, etc).
>
> True.
>
>> So instead, let us just assume that any async process is
>> handling sideband data. That's always true now, and is
>> likely to remain so in the future.
>
> Hmm, does Stefan's thread-pool thing interact with this decision in
> any way?

I do not plan to actually fetch from inside the thread pool, but each thread is just a proxy for starting a new process doing the fetch and getting the output in order.

That seems to be the least amount of work to me. Very long term we may want to do the fetch directly in a thread pool worker, but then we can also add the thread local storage interface.

Show 46 quoted lines

>>
>> The output looks like:
>>
>>    packet:  sideband< \1000eunpack ok0019ok refs/heads/master0000
>>    packet:      push< unpack ok
>>    packet:      push< ok refs/heads/master
>>    packet:      push< 0000
>>    packet:  sideband< 0000
>>
>> Signed-off-by: Jeff King <peff@peff.net>
>> ---
>>  pkt-line.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/pkt-line.c b/pkt-line.c
>> index 08a1427..62fdb37 100644
>> --- a/pkt-line.c
>> +++ b/pkt-line.c
>> @@ -1,5 +1,6 @@
>>  #include "cache.h"
>>  #include "pkt-line.h"
>> +#include "run-command.h"
>>
>>  char packet_buffer[LARGE_PACKET_MAX];
>>  static const char *packet_trace_prefix = "git";
>> @@ -11,6 +12,11 @@ void packet_trace_identity(const char *prog)
>>       packet_trace_prefix = xstrdup(prog);
>>  }
>>
>> +static const char *get_trace_prefix(void)
>> +{
>> +     return in_async() ? "sideband" : packet_trace_prefix;
>> +}
>> +
>>  static int packet_trace_pack(const char *buf, unsigned int len, int sideband)
>>  {
>>       if (!sideband) {
>> @@ -57,7 +63,7 @@ static void packet_trace(const char *buf, unsigned int len, int write)
>>       strbuf_init(&out, len+32);
>>
>>       strbuf_addf(&out, "packet: %12s%c ",
>> -                 packet_trace_prefix, write ? '>' : '<');
>> +                 get_trace_prefix(), write ? '>' : '<');
>>
>>       /* XXX we should really handle printable utf8 */
>>       for (i = 0; i < len; i++) {