C#: Allow message parsing from an array slice #3858
Conversation
jtattermusch
changed the title
| /// <param name="length">The length of the slice to merge.</param> | ||
| public static void MergeFrom(this IMessage message, byte[] data, int offset, int length) | ||
| { | ||
| ProtoPreconditions.CheckNotNull(message, "message"); |
There was a problem hiding this comment.
At some point I should do a nameof tidy-up... but I understand the point of sticking to the convention of the rest of the file here.
| /// <returns>The newly parsed message.</returns> | ||
| public IMessage ParseFrom(byte[] data, int offset, int length) | ||
| { | ||
| ProtoPreconditions.CheckNotNull(data, "data"); |
There was a problem hiding this comment.
I'd ditch this given that it'll be validated in the call to message.MergeFrom.
(If you change the parameter names to buffer in both cases, you could rely on the CodedInputStream constructor validation, but I prefer data instead of buffer here...)
|
A few more corrections:
PTAL. |
|
@anandolee , what's the timeline for the next protobut 3.5 release? I'd like to make sure this change will be included. |
|
I won't be able to look again until tomorrow - about to board a flight. |
|
Jisi has already did a cut off and the change logs are ready. I assume the release will be in next week or two. |
| @@ -64,7 +64,6 @@ internal IMessage CreateTemplate() | |||
| /// <returns>The newly parsed message.</returns> | |||
| public IMessage ParseFrom(byte[] data) | |||
| { | |||
| ProtoPreconditions.CheckNotNull(data, "data"); | |||
There was a problem hiding this comment.
The downside of removing this is that the parameter name will be wrong in the exception (buffer rather than data). But I'm not massively worried.
Backport #3858 to 3.5.x branch
In gRPC C# we would like to enable more efficient buffer management and it seems as a prerequisite
to be able to parse protobuf messages from array slices (when pooling, we cannot ensure that the size of the array will be the same as the length of the payload, and for some techniques, we can't even rely on the offset being 0).
Two reasons why I'd like to add the methods to protobuf directly:
Context: grpc/grpc#13236.