Questions about Protocol Buffer Implementation

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Questions about Protocol Buffer Implementation

cdollins
Google Protocol Buffer Dependency
How should I handle the dependency addition for protocol buffer library. Netty adds the optional element to their maven dependency which as I've read here is a stop-gap measure. From this same document I take it that the right way to do this should be to create a separate sub-module. Any thoughts?
Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

oleksiys
Administrator
Hi Chad,


> Google Protocol Buffer Dependency
> How should I handle the dependency addition for protocol buffer library.
> Netty adds the optional element to their maven dependency which as I've read
> http://maven.apache.org/guides/introduction/introduction-to-optional-and-excludes-dependencies.html
> here  is a stop-gap measure. From this same document I take it that the
> right way to do this should be to create a separate sub-module. Any
> thoughts?
Agree, it should be a submodule under 2dot0/extras/google-protobufs

WBR,
Alexey.
Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

cdollins
This is a first cute of the work for protocol buffers in grizzly grizzly-google-protobufs. Interestingly enough this can be cloned directly inside of extras and the extras pom only need reference the new module google-protobufs. Isn't git great!

Currently in the style of the StringFilter an int is used indicate protobuf message size in bytes. That is the assumption that all protocol buffers coming over the connection will be preceded by 4 bytes indicating the remaining bytes to read that represent the message itself.

Implementation Questions:

1. Do we want a variable length header i.e. varint encoded length header as is done in latest netty? This is similar to the protobuf it self's encoding

2. Do we want to separate out the message length from protocol buffer message codec? This might relieve the burden of external applications to interpret the assumed byte size header and allow greater flexibility for header implementation. i.e. see the above possible encoding for a message size.

3. What if multiple types protobuf messages come down the pipe? As far as I can tell netty does not address this issue. One possible solution is store a message code in a separate header preceding the message. What I would like to see which would solve a lot of problems is a kin to com.sun.jersey.config.property.packages initParam that detects classes to use as resources for a web application. That is automatically build up message type based on a set of classes. Just sayin.
Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

oleksiys
Administrator
Hi Chad,

the code looks fine! Later we can optimize it a bit, but it's very good
start!
Please see the comments inline...

> This is a first cute of the work for protocol buffers in grizzly
> https://github.com/cdollins/grizzly-google-protobufs
> grizzly-google-protobufs . Interestingly enough this can be cloned directly
> inside of extras and the extras pom only need reference the new module
> google-protobufs. Isn't git great!
yep :)

> Currently in the style of the StringFilter an int is used indicate protobuf
> message size in bytes. That is the assumption that all protocol buffers
> coming over the connection will be preceded by 4 bytes indicating the
> remaining bytes to read that represent the message itself.
Is it possible to determine the end of protobuf message w/o having a
length-prefix?

> Implementation Questions:
>
> 1. Do we want a variable length header i.e. varint encoded length header as
> is done in latest netty? This is similar to the
> http://code.google.com/apis/protocolbuffers/docs/encoding.html#varints
> protobuf it self's encoding
Even though I'm not a big fan of varint type (not sure I understand its
benefit), but IMO it could be a good idea to support it to be interopable.
What do you think?

> 2. Do we want to separate out the message length from protocol buffer
> message codec? This might relieve the burden of external applications to
> interpret the assumed byte size header and allow greater flexibility for
> header implementation. i.e. see the above possible encoding for a message
> size.
Agree.

> 3. What if multiple types protobuf messages come down the pipe? As far as I
> can tell netty does not address this issue. One possible solution is store a
> message code in a separate header preceding the message. What I would like
> to see which would solve a lot of problems is a kin to
> com.sun.jersey.config.property.packages initParam that detects classes to
> use as resources for a web application. That is automatically build up
> message type based on a set of classes. Just sayin.
Are we able to detect the protobuf message type?
 From the ProtobufDecoder code:

protobufMessage  =  message.newBuilderForType().mergeFrom(body).build();

not sure I see what we can change in order to support different protobuf
message types.
Any ideas?

Thanks a lot!

WBR,
Alexey.





> --
> View this message in context: http://grizzly.1045725.n5.nabble.com/Questions-about-Protocol-Buffer-Implementation-tp4305806p4314834.html
> Sent from the Grizzly - Development mailing list archive at Nabble.com.

Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

cdollins
Alright, so the second draft is up @ grizzly-google-protobufs. The improvements that were made were to use the BufferInputStream and Protobuf mergeFrom InputStream variant as suggested by Oleksiy, also a unit test was added. Please take the time to review the code and make suggestions.
Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

oleksiys
Administrator
Hi,

> Alright, so the second draft is up @
> https://github.com/cdollins/grizzly-google-protobufs
> grizzly-google-protobufs . The improvements that were made were to use the
> BufferInputStream and Protobuf mergeFrom InputStream variant as suggested by
> Oleksiy, also a unit test was added. Please take the time to review the code
> and make suggestions.
The Decoder looks very good!
IMO it would be great to improve Encoder the similar way, so it won't
perform the buffer copying.

Pls. check the BufferOutputStream (I've recently updated it a bit on
trunk), we can create the instance of BufferOutputStream, reserve 4
bytes for packet length and then let the message serialize its content
to the output stream. Once we know the length - we can put correct 4
bytes length into result buffer. So it will look like:

MemoryManager mm = obtainMemoryManager(...);   // obtain memory manager

BufferOutputStream bos = new BufferOutputStream(mm); // create buffer
outputstream
bos.write(fourByteBuffer);          // reserve 4 bytes which will
contain the length
message.writeTo(bos);      // serialize the message
bos.close();
Buffer buffer = bos.getBuffer().flip();     // get the result buffer
from stream
int len = buffer.remaining() - 4;          // calculate the message len
buffer.putInt(0, len);     // put correct length to the buffer header

it's just my proposal, so feel free to change and improve it.

Thanks a lot!

WBR,
Alexey.

> --
> View this message in context: http://grizzly.1045725.n5.nabble.com/Questions-about-Protocol-Buffer-Implementation-tp4305806p4371922.html
> Sent from the Grizzly - Development mailing list archive at Nabble.com.

Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

oleksiys
Administrator
Hi Chad,

pls. let us know if you have updates on that.

Thanks.

Alexey.

On 05/05/2011 09:05 PM, Oleksiy Stashok wrote:

> Hi,
>
>> Alright, so the second draft is up @
>> https://github.com/cdollins/grizzly-google-protobufs
>> grizzly-google-protobufs . The improvements that were made were to
>> use the
>> BufferInputStream and Protobuf mergeFrom InputStream variant as
>> suggested by
>> Oleksiy, also a unit test was added. Please take the time to review
>> the code
>> and make suggestions.
> The Decoder looks very good!
> IMO it would be great to improve Encoder the similar way, so it won't
> perform the buffer copying.
>
> Pls. check the BufferOutputStream (I've recently updated it a bit on
> trunk), we can create the instance of BufferOutputStream, reserve 4
> bytes for packet length and then let the message serialize its content
> to the output stream. Once we know the length - we can put correct 4
> bytes length into result buffer. So it will look like:
>
> MemoryManager mm = obtainMemoryManager(...);   // obtain memory manager
>
> BufferOutputStream bos = new BufferOutputStream(mm); // create buffer
> outputstream
> bos.write(fourByteBuffer);          // reserve 4 bytes which will
> contain the length
> message.writeTo(bos);      // serialize the message
> bos.close();
> Buffer buffer = bos.getBuffer().flip();     // get the result buffer
> from stream
> int len = buffer.remaining() - 4;          // calculate the message len
> buffer.putInt(0, len);     // put correct length to the buffer header
>
> it's just my proposal, so feel free to change and improve it.
>
> Thanks a lot!
>
> WBR,
> Alexey.
>
>> --
>> View this message in context:
>> http://grizzly.1045725.n5.nabble.com/Questions-about-Protocol-Buffer-Implementation-tp4305806p4371922.html
>> Sent from the Grizzly - Development mailing list archive at Nabble.com.
>

Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

cdollins
Updates to ProtobufEncoder.java have been made to make use of BufferOutputStream. You can find the source on github
Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

Ryan Lubke-2
Hi Chad,

Code looks good to me.  Two comments:

   - ProtobufEncoder:43 consider calling trim() vs flip() to attempt to
reclaim unused buffer space (may
      not be relevant here)
   - Source files (code/resources) need CDDL copyright headers.

Additionally, since this module will depend on a third-party library,
I've started the internal
approval process to have this dependency approved by legal.  Hopefully
it won't take too long.
Will let you know.



On 7/13/11 9:51 AM, cdollins wrote:
> Updates to ProtobufEncoder.java have been made to make use of
> BufferOutputStream. You can find the source on
> https://github.com/cdollins/grizzly-google-protobufs github
>
> --
> View this message in context: http://grizzly.1045725.n5.nabble.com/Questions-about-Protocol-Buffer-Implementation-tp4305806p4583347.html
> Sent from the Grizzly - Development mailing list archive at Nabble.com.

Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

cdollins
Ryan,

Thanks for taking a look at the code and telling me what was need to be added. I've added CDDL to all sources. Maybe you could clarify what the flip() is and why I don't need it.

Thanks.
Reply | Threaded
Open this post in threaded view
|

Re: Questions about Protocol Buffer Implementation

oleksiys
Administrator
Hi Chad,

> Thanks for taking a look at the code and telling me what was need to be
> added. I've added CDDL to all sources. Maybe you could clarify what the
> flip() is and why I don't need it.
Ryan suggested to replace flip with trim.
Trim is pretty similar to flip, it prepares buffer for the following
read operations, but additionally it may return unused Buffer space back
to thread local memory storage.

WBR,
Alexey.

> Thanks.
>
> --
> View this message in context: http://grizzly.1045725.n5.nabble.com/Questions-about-Protocol-Buffer-Implementation-tp4305806p4597945.html
> Sent from the Grizzly - Development mailing list archive at Nabble.com.