Subject: Re: [PATCH 11/26] serve: introduce git-serve

From: Brandon Williams <[email-masked]>
Date: Thu, 1 Feb 2018 12:05:59 -0800

On 02/01, Jeff Hostetler wrote:
> 
> 
> On 2/1/2018 1:57 PM, Stefan Beller wrote:
> > On Thu, Feb 1, 2018 at 10:48 AM, Jeff Hostetler <[email-masked]> wrote:
> > > 
> > > 
> > > On 1/2/2018 7:18 PM, Brandon Williams wrote:
> > > > 
> > > > Introduce git-serve, the base server for protocol version 2.
> [...]
> > > > + Special Packets
> > > > +-----------------
> > > > +
> > > > +In protocol v2 these special packets will have the following semantics:
> > > > +
> > > > +  * '0000' Flush Packet (flush-pkt) - indicates the end of a message
> > > > +  * '0001' Delimiter Packet (delim-pkt) - separates sections of a message
> > > 
> > > 
> > > Previously, a 0001 pkt-line meant that there was 1 byte of data
> > > following, right?
> > 
> > No, the length was including the length field, so 0005 would indicate that
> > there is one byte following, (+4 bytes of "0005" included)
> 
> d'oh.  right.  thanks!
> 
> > > Should we also consider increasing the pkt-line limit to 5 hex-digits
> > > while we're at it ?   That would let us have 1MB buffers if that would
> > > help with large packfiles.
> > 
> > AFAICT there is a static allocation of one pkt-line (of maximum size),
> > such that the code can read in a full packet and then process it.
> > If we'd increase the packet size we'd need the static buffer to be 1MB,
> > which sounds good for my developer machine. But I suspect it may be
> > too much for people using git on embedded devices?
> 
> I got burned by that static buffer once upon a time when I wanted
> to have 2 streams going at the same time.  Hopefully, we can move
> that into the new reader structure at some point (if it isn't already).

Yeah the reader struct could easily be extended to take in the
buffer to read the data into.  Because I'm not trying to do any of that
atm I decided to have it default to using the static buffer, but it
would be as simple as changing the reader->buffer variable to use a
different buffer.

> 
> > 
> > pack files larger than 64k are put into multiple pkt-lines, which is
> > not a big deal, as the overhead of 4bytes per 64k is negligible.
> > (also there is progress information in the side channel, which
> > would come in as a special packet in between real packets,
> > such that every 64k transmitted you can update your progress
> > meter; Not sure I feel strongly on fewer progress updates)
> > 
> > >   Granted, we're throttled by the network,
> > > so it might not matter.  Would it be interesting to have a 5 digit
> > > prefix with parts of the high bits of first digit being flags ?
> > > Or is this too radical of a change?
> > 
> > What would the flags be for?
> > 
> > As an alternative we could put the channel number in one byte,
> > such that we can have a side channel not just while streaming the
> > pack but all the time. (Again, not sure if that buys a lot for us)
> > 
> 
> Delimiters like the 0001 and the side channel are a couple of
> ideas, but I was just thinking out loud.  And right, I'm not sure
> it gets us much right now.
> 
> Jeff

-- 
Brandon Williams