Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(590)

Issue 73041: Performance improvements to XMLRPC

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years ago by krisvale
Modified:
15 years ago
Base URL:
http://svn.python.org/view/*checkout*/python/trunk/
Visibility:
Public.

Description

This patch is a combination of various improvements to SocketServer, BaseHTTPServer, SimpleXMLRPCServer and xmlrcplib. They are the result of deploying XMLRPC in a production environment at CCP. There are three parts: 1) Make the XMLRPC client and server handle HTTP/1.1 with keep-alive 2) make the xmlrpc client and server capable of using the gzip content encoding for the xml 3) Implement socket timeout for servers. I have been tying to submit these patches separately but it is an ardous process. Perhaps it is better to get this accepted as a single patch?

Patch Set 1 #

Total comments: 11

Patch Set 2 : Updated the patch. Added error handling for gzip decode error #

Total comments: 1

Patch Set 3 : Fix unittest. Move 'disable_nagle_algorithm' to StreamRequestHandler, where it is more appropriate. #

Total comments: 9

Patch Set 4 : reinstate send_host(), minor changes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+406 lines, -70 lines) Patch
Doc/library/simplexmlrpcserver.rst View 1 1 chunk +9 lines, -0 lines 0 comments Download
Lib/BaseHTTPServer.py View 1 1 chunk +18 lines, -10 lines 0 comments Download
Lib/DocXMLRPCServer.py View 1 1 chunk +0 lines, -4 lines 0 comments Download
Lib/SimpleXMLRPCServer.py View 1 2 3 5 chunks +51 lines, -6 lines 0 comments Download
Lib/SocketServer.py View 1 2 3 3 chunks +8 lines, -3 lines 0 comments Download
Lib/test/test_xmlrpc.py View 1 2 3 7 chunks +128 lines, -14 lines 0 comments Download
Lib/xmlrpclib.py View 1 2 3 11 chunks +192 lines, -33 lines 0 comments Download

Messages

Total messages: 14
krisvale
http://codereview.appspot.com/73041/diff/1/8 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/1/8#newcode1413 Line 1413: connection.putheader("Content-Encoding", "gzipp") typo, should be "gzip" http://codereview.appspot.com/73041/diff/1/8#newcode1429 Line ...
15 years ago (2009-06-11 15:25:51 UTC) #1
Benjamin
Do you have tests for the gzip encoding? http://codereview.appspot.com/73041/diff/1/3 File Lib/BaseHTTPServer.py (right): http://codereview.appspot.com/73041/diff/1/3#newcode317 Line 317: ...
15 years ago (2009-06-11 16:24:58 UTC) #2
krisvale
Also, I will add gzip tests http://codereview.appspot.com/73041/diff/1/3 File Lib/BaseHTTPServer.py (right): http://codereview.appspot.com/73041/diff/1/3#newcode317 Line 317: if not ...
15 years ago (2009-06-11 21:02:02 UTC) #3
josiah.carlson
http://codereview.appspot.com/73041/diff/1/5 File Lib/SimpleXMLRPCServer.py (right): http://codereview.appspot.com/73041/diff/1/5#newcode435 Line 435: encode_threshold = 1400 #a common MTU I notice ...
15 years ago (2009-06-14 17:47:42 UTC) #4
krisvale
Updated the patch. Added error handling for gzip decode error
15 years ago (2009-06-14 21:04:19 UTC) #5
krisvale
Fix unittest. Move 'disable_nagle_algorithm' to StreamRequestHandler, where it is more appropriate.
15 years ago (2009-06-14 21:40:44 UTC) #6
krisvale
This is a message http://codereview.appspot.com/73041/diff/5003/6005 File Lib/SocketServer.py (right): http://codereview.appspot.com/73041/diff/5003/6005#newcode377 Line 377: - disable_nagle_algorithm This was ...
15 years ago (2009-06-14 21:43:45 UTC) #7
Benjamin
It looks good now. :) http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1154 Line 1154: f.close() I agree ...
15 years ago (2009-06-15 03:08:44 UTC) #8
Martin v. Löwis
I'm not sure whether the change can go into 2.7, because it causes incompatibilities. http://codereview.appspot.com/73041/diff/6010/5015 ...
15 years ago (2009-06-20 16:48:45 UTC) #9
krisvale
http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (left): http://codereview.appspot.com/73041/diff/6010/5015#oldcode1264 Line 1264: def send_host(self, connection, host): On 2009/06/20 16:48:46, Martin ...
15 years ago (2009-06-21 17:21:56 UTC) #10
effbot
LGTM. http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1466 Line 1466: return self._connection Why remove the nice error ...
15 years ago (2009-06-21 21:26:05 UTC) #11
krisvale
http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1466 Line 1466: return self._connection On 2009/06/21 21:26:06, effbot wrote: > ...
15 years ago (2009-06-24 21:13:28 UTC) #12
effbot
http://codereview.appspot.com/73041/diff/6010/5015 File Lib/xmlrpclib.py (right): http://codereview.appspot.com/73041/diff/6010/5015#newcode1466 Line 1466: return self._connection > Because this is now part ...
15 years ago (2009-06-24 21:24:02 UTC) #13
krisvale
15 years ago (2009-06-24 21:24:44 UTC) #14
I've made two changes as per your comments:
1) send_host has been reinstated
2) I've re-added the HTTPS attribute check, for completeness.

I also fixed:
1) Transport needs to remember the "host" parameter in order to cache the
"connection"
2) changed the ServerProxy.__call__() to be aan attribute getter.  one can thus
close it with:  p('close')() and access the transport with p('transport').  The
latter can be  particularly useful, for example if one subclasses Transport to
add functionality, such as keepalive pings and other stuff.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b