[
https://issues.apache.org/jira/browse/THRIFT-248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12661719#action_12661719
]
Kevin Clark commented on THRIFT-248:
------------------------------------
Some implications of this (which Bryan and I have been discussing, but want to
put in the open): the special case, which checked for an "encode_binary" and
"decode_binary" method on a protocol (to use the accelerated version) has been
removed. Instead, the C serialization calls ruby methods on the protocol to do
the encoding. For the accelerated version to work, we'll need to rework some of
the code to wrap the expected protocol methods directly, rather than sending it
all through one C function. Nitay's initial version of the accelerated binary
protocol did this, and can be used for reference. There's also potential for
optimizations where ruby method calls are avoided altogether when using an
accelerated protocol with the C serialization, but that's for another ticket,
after this is worked out.
Code wise:
In protocol.c:
* skip should probably be static.
* We should intern those method names. It's already being done (as statics)
in struct.c, it might make sense to remove the static and reuse them.
There's some places in the code (both protocol.c and struct.c) I think I would
have gone with a switch (against TTYPEs) instead of the if else structure (I
think it feels less cluttered), but it's not a big deal, and I don't feel
strongly about it.
Other than that, I think this makes a lot of sense. Do you think we should open
a separate ticket (that this would depend on) to tweak the accelerated binary
protocol, or do it here?
> Factor BinaryProtocolAccelerated into separate protocol and struct components
> -----------------------------------------------------------------------------
>
> Key: THRIFT-248
> URL: https://issues.apache.org/jira/browse/THRIFT-248
> Project: Thrift
> Issue Type: Improvement
> Components: Library (Ruby)
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Priority: Minor
> Attachments: thrift-248-v2.patch, thrift-248-v3.patch,
> thrift-248.patch
>
>
> Kevin Clark's excelled BinaryProtocolAccelerated implementation in the Ruby
> library is very fast, in large part due to the fact that it implements not
> just the protocol but also the struct components of serialization directly as
> a C extension. The problem with this arrangement is that other protocols that
> would benefit from accelerated struct code don't get the benefit. In
> particular, I'd like to make my implementation of the Compact Protocol fast
> in Ruby, and the key appears to be the struct serialization code.
> I think that we should make an effort to divorce the struct stuff from the
> protocol stuff in BinaryProtocolAccelerated, so that all protocols can
> benefit. Some quick benchmarking seems to indicate that there is going to be
> some additional method call overhead in this situation, but it's not really
> that substantial.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
