New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow CodedOutputStream to use Array Slices #7839
Conversation
The constructor for `CodedOutputStream` (which takes in a given array offset) is `private`. As it stands, it is only used via one other constructor (which always passes 0 as the offset), making the offset always hard-coded to the initial value of 0. Making this constructor `public` allows for client applications to serialize messages into arrays without superfluous array copies. It is also worth noting that the corresponding constructor on `CodedInputStream` is already public (`public CodedInputStream(byte[] buffer, int offset, int length)`). Looking through the history, it seems that this constructor was originally marked private when the library used static factory methods. When migrating over to using constructors directly, this constructor [was not made public](protocolbuffers@0e0e0c9#diff-edc376d7570e8db3f8d8745ef37719f3R82) (perhaps because there was no corresponding factory method?).
We should understand why this wasn't made public to begin with, or if there is a preferred alternative interface that is public. |
CodedInputStream has the same constructor available. It has some extra parameter checks, I'll add those to this PR. Otherwise, there doesn't seem to be an issue with making this public, and it might be preferred over an alternative in order to stay consistent with CodedInputStream. |
Throwing `ArgumentOutOfRangeException` when bad parameters are given. This is consistent with checks made in corresponding constructor in [CodedInputStream](https://github.com/protocolbuffers/protobuf/blob/master/csharp/src/Google.Protobuf/CodedInputStream.cs#L101).
Hi, this change is still desired. This PR has been here for almost a year with no reply. What next steps can be taken? Thanks! |
I'm not sure if this is still needed as CodedOuputStream is now considered legacy (since the introduction of #7576) - better and faster APIs are now available. You can now write messages directly to a IBufferWriter or a Span, which is faster and saves any unnecessary allocations. Use of those APIs is preferred over using CodedOutputStream.
|
I'm going to close this since according to @jtattermusch's comment there are now better APIs available. |
The constructor for
CodedOutputStream
(which takes in a given array offset) isprivate
. As it stands, it is only used via one other constructor (which always passes 0 as the offset), making the offset always hard-coded to the initial value of 0. Making this constructorpublic
allows for client applications to serialize messages into arrays without superfluous array copies. It is also worth noting that the corresponding constructor onCodedInputStream
is already public (public CodedInputStream(byte[] buffer, int offset, int length)
).Looking through the history, it seems that this constructor was originally marked private when the library used static factory methods. When migrating over to using constructors directly, this constructor was not made public (perhaps because there was no corresponding factory method?).