[ https://issues.apache.org/jira/browse/THRIFT-1088?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13006390#comment-13006390 ]
Will Pierce commented on THRIFT-1088: ------------------------------------- % ./runtest2.sh Building libraries make[1]: Leaving directory `/home/willp/src/thrift-svn/trunk' h2. Test of unpatched source Starting test, python=sys.version_info(major=2, minor=7, micro=0, releaselevel='final', serial=0), iterations=100000, tests: ('test_newgen', 'test_oldgen', 'test_newgen_accel', 'test_oldgen_accel') | test_newgen | 116.6607690 usec/iteration | 11.666077 total time | test_oldgen | 114.1114902 usec/iteration | 11.411149 total time | test_newgen_accel | 22.4592090 usec/iteration | 2.245921 total time | test_oldgen_accel | 21.6746688 usec/iteration | 2.167467 total time h3. Running test with python2.4 (no accelerated protocol, not built for this python) Starting test, python=(2, 4, 6, 'final', 0), iterations=100000, tests: ('test_newgen', 'test_oldgen', 'test_newgen_accel', 'test_oldgen_accel') | test_newgen | 122.2611308 usec/iteration | 12.226113 total time | test_oldgen | 119.1130900 usec/iteration | 11.911309 total time | test_newgen_accel | 127.0162797 usec/iteration | 12.701628 total time | test_oldgen_accel | 123.3603191 usec/iteration | 12.336032 total time h2. Test of patched code patching file lib/py/src/protocol/TBinaryProtocol.py patching file lib/py/src/protocol/TCompactProtocol.py patching file lib/py/src/protocol/TProtocol.py patching file lib/py/src/server/TServer.py patching file lib/py/src/server/TNonblockingServer.py patching file lib/py/src/Thrift.py patching file lib/py/src/transport/TTransport.py Building libraries make[1]: Leaving directory `/home/willp/src/thrift-svn/trunk' Starting test, python=sys.version_info(major=2, minor=7, micro=0, releaselevel='final', serial=0), iterations=100000, tests: ('test_newgen', 'test_oldgen', 'test_newgen_accel', 'test_oldgen_accel') | test_newgen | 120.1339102 usec/iteration | 12.013391 total time | test_oldgen | 118.3011293 usec/iteration | 11.830113 total time | test_newgen_accel | 21.6843987 usec/iteration | 2.168440 total time | test_oldgen_accel | 21.5187597 usec/iteration | 2.151876 total time h3. Running test with python2.4 (no accelerated protocol, not built for this python) Starting test, python=(2, 4, 6, 'final', 0), iterations=100000, tests: ('test_newgen', 'test_oldgen', 'test_newgen_accel', 'test_oldgen_accel') | test_newgen | 131.1226416 usec/iteration | 13.112264 total time | test_oldgen | 129.4400001 usec/iteration | 12.944000 total time | test_newgen_accel | 136.2670994 usec/iteration | 13.626710 total time | test_oldgen_accel | 135.5415392 usec/iteration | 13.554154 total time Backing out patch (-R for reverse) patching file lib/py/src/protocol/TBinaryProtocol.py patching file lib/py/src/protocol/TCompactProtocol.py patching file lib/py/src/protocol/TProtocol.py patching file lib/py/src/server/TServer.py patching file lib/py/src/server/TNonblockingServer.py patching file lib/py/src/Thrift.py patching file lib/py/src/transport/TTransport.py Done ... ... ... My observations, reformated into a table: || Python version || Generated-class types || Accelerated Proto? || Unpatched elapsed time/iteration || Patched time/iteration || Performance difference | 2.7 | New-style | No | 116.6607690 usec | 120.1339102 usec | +2.977% | 2.7 | Old-style | No | 114.1114902 usec | 118.3011293 usec | +3.672% | 2.7 | New-style | Yes| 22.4592090 usec | 21.6843987 usec | -3.450% | 2.7 | Old-style | Yes| 21.6746688 usec | 21.5187597 usec | -0.719% | 2.4 | New-style | No | 122.2611308 usec | 120.1339102 usec | -1.740% | 2.4 | Old-style | No | 119.1130900 usec | 129.4400001 usec | +8.670% The accelerated protocol (fastbinary.so) in the python2.7 testing shows little change in speed. The speedup of 3.5% is real and even grows to ~4.5% on longer runs. I don't know why, but I would guess python 2.7 has really improved new-style class invocation from C extension code. The python 2.7 pure-python (non-accel) numbers show a 2.9 - 3.6% slowdown with the patch. The performance difference is smallest when using new-style generated classes, for some reason. Going back to older python 2.4.6, which I don't have the fastbinary.so built for, so no accelerated protocol numbers, there is a small ~2% speedup for new-style generated code, and a ~8% slowdown for old-style generated code. I ran more tests of py2.4 with more iterations (50,000) and got a 120.3 usec vs. 128.15 usec difference, or a +6.52% slowdown on the old-style generated code. Since I don't have fastbinary.so built against python2.4, I don't know if there's much of a speed difference for that case, but I think it's probably about the same as the 2.7 testing- marginal differences. The test code itself isn't the most representative of python use cases, since it only serializes, so if anyone want to enhance the perftest.py code to be more representative, that would help. (I tried to add TCompactProtocol to the test case, but actually uncovered a bug in the encoder, so I'll submit a fix for that in a different ticket.) I hope this is useful information. > Thrift library python classes should be new-style classes > --------------------------------------------------------- > > Key: THRIFT-1088 > URL: https://issues.apache.org/jira/browse/THRIFT-1088 > Project: Thrift > Issue Type: Improvement > Components: Python - Library > Affects Versions: 0.6 > Reporter: Will Pierce > Priority: Minor > Attachments: THRIFT-1088_pthon_new_style_classes.patch, perftest.py, > runtest.sh, runtest2.sh > > > The included python thrift library classes (TType, TProtocolBase, etc...) are > all defined using old-style python class definitions. This makes it somewhat > more difficult to dynamically deal with instances of those classes at > runtime, since the type() builtin doesn't return the class name. I don't > think there's really any good reason for avoiding using new style classes. > The typical fear/uncertainty/doubt with new-style classes (since 2001 when > they were made available in python 2.2) is that they are thought to be slower > than old-style classes. I personally haven't ever found the performance > difference between old and new style classes to > Unless there's a compelling reason that Thrift developers want to use > old-style classes, I think it's a more programmer-friendly choice to switch > these to new-style classes. Not to mention the benefits that new style > classes provide, see: > http://realmike.org/blog/2010/07/18/introduction-to-new-style-classes-in-python/ > > The following thrift classes don't inherit from the 'object' class, so are > old-style classes: > {noformat} > % find . -name \*py | xargs grep class | grep -v [.]svn | grep '[a-zA-Z0-9]:$' > ./src/protocol/TCompactProtocol.py:class CompactType: > ./src/protocol/TCompactProtocol.py:class TCompactProtocolFactory: > ./src/protocol/TProtocol.py:class TProtocolBase: > ./src/protocol/TProtocol.py:class TProtocolFactory: > ./src/protocol/TBinaryProtocol.py:class TBinaryProtocolFactory: > ./src/protocol/TBinaryProtocol.py:class TBinaryProtocolAcceleratedFactory: > ./src/Thrift.py:class TType: > ./src/Thrift.py:class TMessageType: > ./src/Thrift.py:class TProcessor: > ./src/server/TServer.py:class TServer: > ./src/server/TNonblockingServer.py:class Connection: > ./src/server/TNonblockingServer.py:class TNonblockingServer: > ./src/transport/TTransport.py:class TTransportBase: > ./src/transport/TTransport.py:class CReadableTransport: > ./src/transport/TTransport.py:class TServerTransportBase: > ./src/transport/TTransport.py:class TTransportFactoryBase: > ./src/transport/TTransport.py:class TBufferedTransportFactory: > ./src/transport/TTransport.py:class TFramedTransportFactory: > {noformat} > I'll make changes to my local copy of trunk, run the tests and then attach a > patch to this ticket for evaluation. > Unless anyone has objections or concerns about switching to new style classes? -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira