[csharp] [discussion] Buffer allocation considerations - Marshaller-T

[mgravell]

I'm looking at the gRPC implementation with a view to .NET performance, and it is of keen note that the Marshaller<T> API is very much focused around "allocate a buffer per job". It looks like there was some interest in ReadOnlySequence<byte>, but that this has now petered out (as far as I can see it is neither implemented nor consumed).

Would there be any interest in trying to resurrect this topic? I'm happy to offer some input here, but I don't want to bore people with that if this is a "done and done" dead topic. For context, I have lots of experience with all of the current memory APIs and how to use them to optimize memory throughput in high volume systems, including extensive knowledge of pipelines (ROS-T), buffers (Memory-T, Span-T), the array pool, etc. I'm also intimately familiar with protobuf and the impact on serializers.

Reading between the lines, I'm guessing (on a hunch) that the ROS-T work was deemed not worth the trouble for the serializer, but if that's the case there may still be scope for large gains by using the array-pool or buffer-pool, with minimal API changes or serializer changes.

So: let me know if my input would be useful here. Also let me know if it wouldn't! Thanks.

[jtattermusch]

Hey Marc, summary of what has been done recently: We've added a new "contextual" serialization/deserialization API, where SerializationContext and DeserializationContext is passed to the function and allows adding more interesting interactions than just reading from a newly allocated array etc. https://github.com/grpc/grpc/blob/3891e03ece195810a521ec379a512bdc2e4262cd/src/csharp/Grpc.Core.Api/Marshaller.cs#L58 The idea is that we can over time add more methods to (De)SerializationContext. Some of it has been already done for DeserializationContext, which allows reading the entire payload as ReadOnlySequence and it does that without doing any copying (the segments in the sequence are native grpc_slice items as received by C-core). https://github.com/grpc/grpc/blob/3891e03ece195810a521ec379a512bdc2e4262cd/src/csharp/Grpc.Core.Api/DeserializationContext.cs#L62 We still want to implement writing payload to IBufferWriter by adding a new method to SerializationContext (to avoid copying and array allocations) - this still needs to be done. Your input is definitely welcome (as well as pull requests :-) ).

…

On Sun, Jun 23, 2019 at 4:20 PM Marc Gravell ***@***.***> wrote: I'm looking at the gRPC implementation with a view to .NET performance, and it is of keep note that the Marshaller<T> API is very much focused around "allocate a buffer per job". It looks like there was some interest in ReadOnlySequence<byte>, but that this has now petered out (as far as I can see it is neither implemented nor consumed). Would there be any interest in trying to resurrect this topic? I'm happy to offer some input here, but I don't want to bore people with that if this is a "done and done" dead topic. For context, I have lots of experience with all of the current memory APIs and how to use them to optimize memory throughput in high volume systems, including extensive knowledge of pipelines (ROS-T), buffers (Memory-T, Span-T), the array pool, etc. I'm also intimately familiar with protobuf and the impact on serializers. Reading between the lines, I'm guessing (on a hunch) that the ROS-T work was deemed not worth the trouble for the serializer, but if that's the case there may still be scope for large gains by using the array-pool or buffer-pool, with minimal API changes or serializer changes. So: let me know if my input would be useful here. Also let me know if it wouldn't! Thanks. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#19447?email_source=notifications&email_token=ACL2VZDVTQYXAD5BI24BZDLP36BBTA5CNFSM4H2ZAOO2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G3EU4OA>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACL2VZA7JGM2P75WOHBUL2LP36BBTANCNFSM4H2ZAOOQ> .

-- Jan

[jtattermusch]

protocolbuffers/protobuf#5888 is currently deprioritized because of feature work on grpc-dotnet, but we do plan to get back to it. If you wanted to take a look at it in the meantime, you're welcome to do so.

[mgravell]

That's encouraging - I couldn't see what is implementing the zero-copy non-contiguous variant (ROS-byte) but I'll take a deeper look, especially since it sounds like you're open to input. I didn't want to dive in too deep without checking the water, so to speak!

[jtattermusch]

@mgravell see this PR #18865 for details.

Just to set the expectations - performance work is not the priority right now, but that said, we're happy to have conversations about future designs and accept contributions with perf improvements.

[mgravell]

Just to set the expectations

That's fine - it means I can do some thinking and make some suggestions (I like to talk before coding) without trampling on any else's toes or risking a merge conflict, and the project might get some free perf gains without distracting the core team from their current priorities.

[mgravell]

@jtattermusch I've started my initial dabblings here; marked "draft" at the moment as I'm still having difficulty with the Windows build, so it isn't fully tested yet etc; just wanted to highlight the ideas I had in mind

[mgravell]

(closing; moving to discreet issues)