Accelerated Binary Protocol support for TFramedTransport
--------------------------------------------------------
Key: THRIFT-212
URL: https://issues.apache.org/jira/browse/THRIFT-212
Project: Thrift
Issue Type: Improvement
Components: Library (Python)
Reporter: David Reiss
Priority: Minor
This change makes TFramedTransport implement the CReadableTransport interface
so TBinaryProtocolAccelerated can use the fastbinary C module to read from it.
Patches:
http://gitweb.thrift-rpc.org/?p=thrift.git;a=log;h=refs/heads/pri/dreiss/py-cread-framed;hb=HEAD
This change was discussed earlier this year:
http://publists.facebook.com/pipermail/thrift/2008-March/000735.html
bq. Could you change this to avoid the concat? You'd simply have to offset the
initial buffer by 4 bytes and then do:
This is orthogonal to the CReadable change, so it doesn't belong in this issue.
bq. It'd be helpful to have a unit test for that case to prevent accidential
optimization.
Again, orthogonal. I only touched this part of the code in a very superficial
way.
bq. In the case where partialread == "" (which is the norm for the framed
transport) this results in an extra copy.
This is not the case. I took a look at the Python source (the latest svn
trunk), and it optimizes the case of concatenating with an empty string.
bq. Imagine there was a string field fragmented across multiple packets.
I fixed this issue and added a test for it.
bq. I'd really appreciate a unit test on this stuff
The TNonblockingServer test and the split frame test exercise it pretty well.
bq. a transport that makes read() return a random number of bytes each time.
It wouldn't make sense to put such a transport between the
TBinaryProtocolAccelerated and the TFramedTransport. Putting it under the
TFramedTransport would have no effect since TFramedTransport only uses readAll
to get data from the underlying transport.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
