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

[Lars Schneider <larsxschneider@gmail.com>]

> On 29 Jul 2016, at 09:40, Jakub Narębski <jnareb@gmail.com> wrote:
> 
> W dniu 2016-07-28 o 15:29, Jeff King pisze:
>> On Thu, Jul 28, 2016 at 09:16:18AM +0200, Lars Schneider wrote:
>> 
>>> 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.


>> 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