From 41c92c36f04dadb2c9c1854235b26c5f479a8f12 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Nov 2019 18:55:24 +0100 Subject: [PATCH 1/6] add span_based_serialization design --- csharp/span_based_serialization.md | 63 ++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 csharp/span_based_serialization.md diff --git a/csharp/span_based_serialization.md b/csharp/span_based_serialization.md new file mode 100644 index 000000000000..f7d98b7f416c --- /dev/null +++ b/csharp/span_based_serialization.md @@ -0,0 +1,63 @@ +# More efficient Protobuf C# serialization/deserialization API + +## Background +Currently serialization of protobuf C# messages is done (much like java) through instances of CodedInputStream and CodedOutputStream. +These APIs work well, but there is some inherent overhead due to the fact that they are basically `byte[]` based. +Recently a set of new types has been added to the .NET Framework (Span, ReadOnlySequence, IBufferWriter) +and these new types allow defining new serialization/deserialization APIs, ones that would be much more efficient especially from +the perspective of memory management. + +Both gRPC C# implementations (Grpc.Core and grpc-dotnet) already contain functionality that allows exposing request/response payloads +in terms of the new .NET Span APIs in a way that requires no copying of binary payloads and basically allows deserialization directly +from the buffer segments that were received from the network (and serializing directly into the buffer that is going to be sent out +over the network). + +## Objective +The goals for this effort: +- keep the existing CodedInputStream and CodedOutputStream (no plans to remove them in the future), serialization/deserialization speed of these APIs should stay unaffected. +- all the changes made need to be backward compatible +- introduce a set of new public APIs for serialization/deserialization that utilize the Span type +- allow future incremental performance optimizations and make them as easy as possible +- ensure the are microbenchmark that allow comparing the two implementations +- as support for the new serialization/deserialization APIs requires generating code that is not compatible with some older .NET frameworks, figure out a schema under which messages with "new serialization API support" and without it can coexist (without causing user friction). +- the effort to maintain the Protobuf C# project should remain approximately the same (should not be negatively affected by introduction of new serialization/deserialization APIs). + +Non-goals +- over-optimize the initial version of new serializer/deserializer (we should rather focus on enabling future improvements) +- remove or deprecate the existing serialization/deserialization API + +## The Proposal + +Key elements: +- Add CodedInputReader and CodedOutputWriter types (both are `ref structs` and thus can hold an instance of `Span`). These two types implement the low-level wire format semantics in terms of the `Span` type (as opposed to `byte[]` used by CodedInputStream and CodedOutputStream). +- Introduce `IBufferMessage` interface (exact name TBD) which inherits from `IMessage` and marks protobuf messages that support the new serialization/deserialization API. +- Introduce a new codegen option (exact name TBD) to enable/disable generating code required by CodedInputReader/CodedInputWriter (the MergeFrom and WriteTo methods). + +A prototype implementation is https://github.com/protocolbuffers/protobuf/pull/5888 + +## Platform support + +- the new serialization API will be supported for projects that support at least `netstandard2.0` and allow use of C# 7.2. On newer .NET frameworks, there might be (and likely will be) further performance benefits. + +## Backwards compatibility + +## Code duplication + +TODO: testing (how to test both APIs without twice the effort). + +## Other considerations + +TODO: design for codegen option +TODO: will the generated code contain an #ifdef to enable multi-targeted projects? + +TODO: design for Grpc.Tools and whether the span-based parsing will be enabled by default + +TODO: how will the old generated code coexist with the span-enabled generated code? how will it work across library boundaries (e.g. a client library contains code generated without the support for fast parsing and we want to use that client library). + +## References + +This proposal is based on previous work / discussions: +- https://github.com/grpc/grpc-dotnet/issues/30 +- https://github.com/protocolbuffers/protobuf/issues/3431 +- https://github.com/protocolbuffers/protobuf/pull/4988 +- https://github.com/jtattermusch/protobuf/pull/8 \ No newline at end of file From 1e8182376af8829b3681a6ba37492e7ec45268a0 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 19 Nov 2019 14:42:42 +0100 Subject: [PATCH 2/6] Apply suggestions from code review Co-Authored-By: James Newton-King --- csharp/span_based_serialization.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/span_based_serialization.md b/csharp/span_based_serialization.md index f7d98b7f416c..680c5d973b5c 100644 --- a/csharp/span_based_serialization.md +++ b/csharp/span_based_serialization.md @@ -31,7 +31,7 @@ Non-goals Key elements: - Add CodedInputReader and CodedOutputWriter types (both are `ref structs` and thus can hold an instance of `Span`). These two types implement the low-level wire format semantics in terms of the `Span` type (as opposed to `byte[]` used by CodedInputStream and CodedOutputStream). - Introduce `IBufferMessage` interface (exact name TBD) which inherits from `IMessage` and marks protobuf messages that support the new serialization/deserialization API. -- Introduce a new codegen option (exact name TBD) to enable/disable generating code required by CodedInputReader/CodedInputWriter (the MergeFrom and WriteTo methods). +- Introduce a new codegen option (exact name TBD) to enable/disable generating code required by CodedInputReader/CodedInputWriter (the MergeFrom and WriteTo methods). This option will default to disabled. gRPC frameworks will likely provide the ability to explicitly control it, e.g. ``, and/or detect the version of .NET being targeted and automatically enable the option. A prototype implementation is https://github.com/protocolbuffers/protobuf/pull/5888 @@ -60,4 +60,4 @@ This proposal is based on previous work / discussions: - https://github.com/grpc/grpc-dotnet/issues/30 - https://github.com/protocolbuffers/protobuf/issues/3431 - https://github.com/protocolbuffers/protobuf/pull/4988 -- https://github.com/jtattermusch/protobuf/pull/8 \ No newline at end of file +- https://github.com/jtattermusch/protobuf/pull/8 From 8ae06c953f4bde59155f2f69f19bf86c68f81610 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 6 Jan 2020 18:28:50 +0100 Subject: [PATCH 3/6] Apply suggestions from code review Co-Authored-By: James Newton-King --- csharp/span_based_serialization.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/span_based_serialization.md b/csharp/span_based_serialization.md index 680c5d973b5c..794967f9c317 100644 --- a/csharp/span_based_serialization.md +++ b/csharp/span_based_serialization.md @@ -2,7 +2,7 @@ ## Background Currently serialization of protobuf C# messages is done (much like java) through instances of CodedInputStream and CodedOutputStream. -These APIs work well, but there is some inherent overhead due to the fact that they are basically `byte[]` based. +These APIs work well, but there is some inherent overhead due to the fact that they are basically `byte[]` based. The way .NET gRPC uses these types involves allocating an array the size of the message, which for larger messages means allocations to the [large object heap](https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/large-object-heap#loh-performance-implications). Recently a set of new types has been added to the .NET Framework (Span, ReadOnlySequence, IBufferWriter) and these new types allow defining new serialization/deserialization APIs, ones that would be much more efficient especially from the perspective of memory management. From bb28883b6a4a275d288436d3de9f841742488b33 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 6 Jan 2020 19:13:09 +0100 Subject: [PATCH 4/6] Update span_based_serialization.md --- csharp/span_based_serialization.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/csharp/span_based_serialization.md b/csharp/span_based_serialization.md index 794967f9c317..bec6b4d2957e 100644 --- a/csharp/span_based_serialization.md +++ b/csharp/span_based_serialization.md @@ -18,7 +18,7 @@ The goals for this effort: - all the changes made need to be backward compatible - introduce a set of new public APIs for serialization/deserialization that utilize the Span type - allow future incremental performance optimizations and make them as easy as possible -- ensure the are microbenchmark that allow comparing the two implementations +- ensure there are microbenchmark that allow comparing the two implementations - as support for the new serialization/deserialization APIs requires generating code that is not compatible with some older .NET frameworks, figure out a schema under which messages with "new serialization API support" and without it can coexist (without causing user friction). - the effort to maintain the Protobuf C# project should remain approximately the same (should not be negatively affected by introduction of new serialization/deserialization APIs). @@ -29,9 +29,10 @@ Non-goals ## The Proposal Key elements: -- Add CodedInputReader and CodedOutputWriter types (both are `ref structs` and thus can hold an instance of `Span`). These two types implement the low-level wire format semantics in terms of the `Span` type (as opposed to `byte[]` used by CodedInputStream and CodedOutputStream). -- Introduce `IBufferMessage` interface (exact name TBD) which inherits from `IMessage` and marks protobuf messages that support the new serialization/deserialization API. -- Introduce a new codegen option (exact name TBD) to enable/disable generating code required by CodedInputReader/CodedInputWriter (the MergeFrom and WriteTo methods). This option will default to disabled. gRPC frameworks will likely provide the ability to explicitly control it, e.g. ``, and/or detect the version of .NET being targeted and automatically enable the option. +- Add `CodedInputReader` and `CodedOutputWriter` types (both are `ref structs` and thus can hold an instance of `Span`). These two types implement the low-level wire format semantics in terms of the `Span` type (as opposed to `byte[]` used by CodedInputStream and CodedOutputStream). +- Introduce `IBufferMessage` interface which inherits from `IMessage` and marks protobuf messages that support the new serialization/deserialization API. +- Introduce a new codegen option `--use_buffer_serialization` to enable/disable generating code required by CodedInputReader/CodedInputWriter (the `MergeFrom(ref CodedInputReader)` and `WriteTo(ref CodedOutputWriter output)` methods). This option will default to disabled. gRPC frameworks will likely provide the ability to explicitly control it, e.g. ``, and/or detect the version of .NET being targeted and automatically enable the option. +- Generated code that uses the new IBufferMessage/CodedInputReader/CodedOutputWriter types will be surrounded by `#if !PROTOBUF_DISABLE_BUFFER_SERIALIZATION` defines. That will allow potentially incompatible generated code to be excluded by a project to support multi-targeting scenarios. A prototype implementation is https://github.com/protocolbuffers/protobuf/pull/5888 @@ -39,21 +40,27 @@ A prototype implementation is https://github.com/protocolbuffers/protobuf/pull/5 - the new serialization API will be supported for projects that support at least `netstandard2.0` and allow use of C# 7.2. On newer .NET frameworks, there might be (and likely will be) further performance benefits. +- TODO: add a note that on `net45` the new serialization API will be supported too, but it's mostly for convenience. `netstandard2.0` and higher are the primary targets. +- TODO: add a note that the new serialization API won't be supported on netstandard1.0 + ## Backwards compatibility +TODO: + ## Code duplication TODO: testing (how to test both APIs without twice the effort). ## Other considerations -TODO: design for codegen option -TODO: will the generated code contain an #ifdef to enable multi-targeted projects? +TODO: design for codegen option (--use_buffer_serialization, PROTOBUF_DISABLE_BUFFER_SERIALIZATION ifdef, describe possible scenarios and how the design solves them) TODO: design for Grpc.Tools and whether the span-based parsing will be enabled by default TODO: how will the old generated code coexist with the span-enabled generated code? how will it work across library boundaries (e.g. a client library contains code generated without the support for fast parsing and we want to use that client library). +TODO: handling of `bytes` fields efficiently? + ## References This proposal is based on previous work / discussions: From 47bc28123fe293fadaf8b4d1536f7d813b94e4bc Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 7 Jan 2020 16:08:16 +0100 Subject: [PATCH 5/6] Update span_based_serialization.md --- csharp/span_based_serialization.md | 49 ++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/csharp/span_based_serialization.md b/csharp/span_based_serialization.md index bec6b4d2957e..9da7cb587a0f 100644 --- a/csharp/span_based_serialization.md +++ b/csharp/span_based_serialization.md @@ -36,31 +36,62 @@ Key elements: A prototype implementation is https://github.com/protocolbuffers/protobuf/pull/5888 +## Efficiency improvements + +`MergeFrom(ref CodedInputReader)` allows parsing protobuf messages from a `ReadOnlySequence`, which offers these benefits over the existing `MergeFrom(ref CodedInputStream)` and/or `MergeFrom(byte[] input)`. + +- The input buffer does not have to be continguous, but can be formed from several segment/slices. That's a format that naturally matches the format in which the data are received from the network card (= we can parse directly from the memory slices we received from network) + +- Allows parsing directly from both managed and native memory buffer in a safe manner with a single API (this is beneficial for Grpc.Core implementation that uses a native layer, so we don't have to copy buffers into managed memory first). + +- In grpc-dotnet, the requests can be exposed as `ReadOnlySequence` in a very efficient manner. + +- Buffer segments can be reused once read so that there are no extra memory allocations. + +- Access to segments of read only sequence via `ReadOnlySpan<>` is heavily optimized in recent .NET Core releases. + +`WriteTo(ref CodedOutputWriter output)` allows serializing protobuf messages to `IBufferWriter` which has these benefits: + +- The output doesn't have to be written to a single continguous chunk of memory, but into smaller buffer segments, which +avoids allocating large blocks of memory, resulting in slow LOH GC collections. + +- Allows serializing to both managed and native memory in a safe manner with the same API. + +- Buffer segments can be reused once read so that there are no extra memory allocations. + +- Allows avoiding any additional copies of data when serializing (in both Grpc.Core and grpc-dotnet). + ## Platform support - the new serialization API will be supported for projects that support at least `netstandard2.0` and allow use of C# 7.2. On newer .NET frameworks, there might be (and likely will be) further performance benefits. -- TODO: add a note that on `net45` the new serialization API will be supported too, but it's mostly for convenience. `netstandard2.0` and higher are the primary targets. -- TODO: add a note that the new serialization API won't be supported on netstandard1.0 +- the new serialization API will be supported on `net45`, but it's mostly for convenience and to minimize the differences between different builds of the library. `netstandard2.0` and higher are the primary targets. + +- the new serialization API won't be supported on netstandard1.0 (as it doesn't support the `System.Memory` package which contains several required types). Users will still be able to use the original serialization APIs as always. ## Backwards compatibility -TODO: +API changes to the runtime library are purely additive. -## Code duplication +By default (without `--use_buffer_serialization` options), the generated code will stay the same and won't depend on any of the newly introduced types, so existing users that don't opt-in won't be affected by the new APIs at all. -TODO: testing (how to test both APIs without twice the effort). +When `--use_buffer_serialization` is used when generating the code, the code requiring the newly introduces types will +be protected by `PROTOBUF_DISABLE_BUFFER_SERIALIZATION` if-define, so that it can be disabled (needs to be manually set in a project) when needed. -## Other considerations +## Concern: Code duplication and maintainability -TODO: design for codegen option (--use_buffer_serialization, PROTOBUF_DISABLE_BUFFER_SERIALIZATION ifdef, describe possible scenarios and how the design solves them) +While the logical structure of the (de)serialization code for both old and new approaches is very similar, due to the different nature of the object holding the parsing state (`class` vs `ref struct`), the parsing primitives need to be defined twice (e.g. ReadRawVarint64 exist in two copies, one under CodedInputStream and one user CodedInputReader) and that adds some extra overhead when: + +- optimizing the (de)serialization code (two locations need to be updated) +- fixing bugs (some bug might need to be fixed in two locations) +- writing tests (in some cases, tests need to be duplicated, because behavior with both Stream/Buffer serialization need to be tested) + +## Other considerations TODO: design for Grpc.Tools and whether the span-based parsing will be enabled by default TODO: how will the old generated code coexist with the span-enabled generated code? how will it work across library boundaries (e.g. a client library contains code generated without the support for fast parsing and we want to use that client library). -TODO: handling of `bytes` fields efficiently? - ## References This proposal is based on previous work / discussions: From d4242bc73e9b02ec799c343eee8f267eb3c37ab9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 7 Jan 2020 19:05:24 +0100 Subject: [PATCH 6/6] Update span_based_serialization.md --- csharp/span_based_serialization.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/csharp/span_based_serialization.md b/csharp/span_based_serialization.md index 9da7cb587a0f..816b3b1e10aa 100644 --- a/csharp/span_based_serialization.md +++ b/csharp/span_based_serialization.md @@ -86,6 +86,19 @@ While the logical structure of the (de)serialization code for both old and new a - fixing bugs (some bug might need to be fixed in two locations) - writing tests (in some cases, tests need to be duplicated, because behavior with both Stream/Buffer serialization need to be tested) +## Integration with generated gRPC stubs + +To use the new (de)serialization API from gRPC, gRPC C# will need to modify the codegen plugin to generate +grpc stubs that utilize the new API. + +TODO: add the design for changes to grpc_csharp_plugin codegen plugin. + +TODO: what is going to be the flag to select the new serialization API in gRPC stubs? + +TODO: add example snippet of the generated marshaller code (current state: https://github.com/grpc/grpc/blob/b07d7f14857d09f05b58ccd0e5f15a6992618f3a/src/csharp/Grpc.Examples/MathGrpc.cs#L30) + +TODO: is there a way to autodetect that IBufferMessage serialization API is available and use it automatically (and fallback if it's not available?) + ## Other considerations TODO: design for Grpc.Tools and whether the span-based parsing will be enabled by default