[ 
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

Reply via email to