Re: [PATCH v2 0/5] Git filter protocol

On 29 Jul 2016, at 09:40, Jakub Narębski [off-list ref] wrote:

W dniu 2016-07-28 o 15:29, Jeff King pisze:

quoted

On Thu, Jul 28, 2016 at 09:16:18AM +0200, Lars Schneider wrote:

quoted

But Peff ($gmane/299902), Duy, and Eric, seemed to prefer the pkt-line
solution (gmane is down - otherwise I would have given you the links).

FWIW, I think there are arguments for transmitting size + content
(namely, that it is simpler); the downside is that it doesn't allow
streaming.

And that it requires for the filter to know the size of its output
upfront (which, as I wrote, might be easy to do based on size of input
and data stored elsewhere, or might need generating whole output to
know).

I don't know how parallel Git is, but if it is parallel enough,
and other limits do not apply (limited amount of CPU cores, I/O limits),
without streaming new filter protocol might be slower, unless startup
time dominates (MS Windows?):

Current parallel:

  |   startup   | processing 1 |
   |  startup    | processing 2  |
  | startup |  processing 3 |
    |  startup  |  processing 4  |

Protocol v2:

  |  startup  | processing 1 | processing 2 | processing 3 | processing 4 |

Based on the current filter design the "single-shot" invocations are
not executed in parallel.

quoted

So I think there are two viable alternatives:

 1. Total size of data in ASCII decimal, newline, then that many bytes
    of content.

 2. No size header, then a series of pkt-lines followed by a flush
    packet.

   3. Optional size header[2][3], then a series of pkt-lines followed
      by a flush packet[4].

[2] Git should always provide size, because it is easy to do, and
   I think quite cheap (stored with blob, stored in index, or stat()
   on file away).  Filter can provide size if it is easy to calculate,
   or approximation of size / size hint[5] - it helps to avoid
   reallocation.

Agreed!

[3] It is also a place where filter can pass error conditions that
   are known before starting processing a file.

I am not sure I understand what you mean. Can you think of an example?

[4] On one hand you need to catch cases where real size is larger than
   size sent upfront, or smaller than size sent upfront; on the
   other hand it might be a place where to send warnings and errors...
   unless we utilize stderr of a process (but then there is a problem
   of deadlocking, I think).
[5] I suggest

       <size as ascii decimal>
       "approx" SPC <size as ascii decimal>
       "unknown"
       "fail"

My current implementation supports only two cases. Either the filter
knows the size and sends it back. Or the filter doesn't know the size
and Git reads until the flush packet (your "unknown" case). "Approx" is 
probably hard to do and fail shouldn't be part of the size, no?

That being said a "fail" response is a very good idea! This allows
the filter to communicate to git that a non required filter process
failed. I will add that to the protocol. Thanks :) 

- Lars