[protobuf] Re: Issue 120 in protobuf: Request to allow extending CodedInputStream and CodeOutputStream
Comment #8 on issue 120 by ken...@google.com: Request to allow extending CodedInputStream and CodeOutputStream http://code.google.com/p/protobuf/issues/detail?id=120 It's true that because Coded*Stream are public, we cannot contract these interfaces. However, we *do* currently have the ability to *expand* these interfaces without breaking anyone, whereas with your proposed change this would be considerably more difficult. Moreover, it is much easier for us to change the pattern of calls made by the protobuf implementation *to* these interfaces when there is only one implementation. If there are multiple implementations, we don't know the affect our changes might have. For an extreme example, imagine if in the future we discover that we can make encoding much faster by generating code which writes directly to CodedOutputStream's underlying byte array rather than calling its various methods. So we add a new method like byte[] getBuffer(), and the encoder now calls only that. Your custom implementation of CodedOutputStream would be thoroughly broken by this, probably requiring a complete re-design and rewrite of your entire project. Since it's hard to tell how many people would be broken by such a change, we'd probably avoid making it in the first place, meaning everyone remains stuck with an inferior implementation. So, you see, this kind of decision is not as clear-cut as it at first seems. Adding any sort of abstraction to the library requires careful thought and design. If you want to pursue this, you'll probably need to start a discussion thread on the mailing list where you and the protobuf maintainers (which no longer include me) can discuss the pros and cons in more detail. Regarding benchmarks, I think there are some Java benchmarks in the SVN repo that you can run fairly easily. -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to protobuf@googlegroups.com. To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.
[protobuf] Re: Issue 120 in protobuf: Request to allow extending CodedInputStream and CodeOutputStream
Comment #7 on issue 120 by owen.oma...@gmail.com: Request to allow extending CodedInputStream and CodeOutputStream http://code.google.com/p/protobuf/issues/detail?id=120 I understand about not wanting to get locked into an API, although I'm sure you'll already find that if you change the Coded*Stream API you'll break many users. It is already part of your public API and you'll need to deprecate old methods and gradually move your interfaces if you want to make changes. I certainly do understand that inheritance is a brittle dependency, which is why I separated out the interface from the implementation, but fundamentally I'm looking to replace the serialization black box with a different implementation, so it is the right abstraction. What benchmarks would you want to see? Writing and reading a million objects from a file stream? I hadn't considered creating a custom protoc plugin and that is an interesting thought. It is a lot more involved than just implementing the two classes and naively seems more dependent on the protobuf internals. It would also mean that the generated classes would be twice as large with both my serialization code and the original serialization code. -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to protobuf@googlegroups.com. To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.
[protobuf] Re: Issue 120 in protobuf: Request to allow extending CodedInputStream and CodeOutputStream
Updates: Owner: ken...@google.com Comment #6 on issue 120 by ken...@google.com: Request to allow extending CodedInputStream and CodeOutputStream http://code.google.com/p/protobuf/issues/detail?id=120 The problem with this is that if we ever modify the API of Coded*Stream (e.g. to add new methods to support new features), we will have to worry about breaking people like you who have written custom implementations. In fact, we might even break you if we merely change the way Coded*Stream is called, without changing its implementation. Worrying about this creates obstacles to development. Also, abstraction has a performance cost which may be particularly acute in the case of Coded*Stream, which are used within the inner loops of parsing and serialization, performance-critical applications. Have you checked to see how this change affects the benchmarks? While you technically can subclass the C++ Coded*Stream, it won't do any good because none of the methods are virtual. In Python, while you can technically override anything via monkey-patching, the encoding/decoding code is all marked private and it's generally accepted that you shouldn't fiddle with private internals of other packages. So, it is not really possible to override things in this way in either of these languages. It would be better if you wrote a protoc plugin which generates custom parsing/serialization code that calls your own APIs. Or, if performance is not critical, just write some code that iterates through all the fields using getField()/setField() (on the Message and Message.Builder interfaces). -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to protobuf@googlegroups.com. To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.
[protobuf] Re: Issue 120 in protobuf: Request to allow extending CodedInputStream and CodeOutputStream
Comment #5 on issue 120 by owen.oma...@gmail.com: Request to allow extending CodedInputStream and CodeOutputStream http://code.google.com/p/protobuf/issues/detail?id=120 Here's my patch that applies against 2.4.1 and trunk. It makes CodedInputStream and CodedOutputStream into abstract classes and makes private implementations of those classes. It should be completely API-equivalent. Of course doing this makes it clear that having the serialized size methods as static methods is unfortunate, but that is a much bigger change. Attachments: my.patch 101 KB -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to protobuf@googlegroups.com. To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.
[protobuf] Re: Issue 120 in protobuf: Request to allow extending CodedInputStream and CodeOutputStream
Comment #4 on issue 120 by owen.oma...@gmail.com: Request to allow extending CodedInputStream and CodeOutputStream http://code.google.com/p/protobuf/issues/detail?id=120 I'd like to make an alternative encoding that has additional properties. With the final set on the Coded{In,Out}putStream that isn't possible without forking the whole protobuf jar. I note that neither the C++ or Python versions have similar restrictions. -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to protobuf@googlegroups.com. To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.