You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "David Reiss (JIRA)" <ji...@apache.org> on 2008/11/21 01:58:44 UTC

[jira] Assigned: (THRIFT-212) Accelerated Binary Protocol support for TFramedTransport

     [ https://issues.apache.org/jira/browse/THRIFT-212?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Reiss reassigned THRIFT-212:
----------------------------------

    Assignee: David Reiss

> 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
>            Assignee: 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.