[Python-Dev] Modules/socketmodule.c: avoiding second fcntl() call worth the effort?
Peter Portante
peter.a.portante at gmail.com
Sun Jan 20 22:40:42 CET 2013
FWIW, looking into Python's Lib/socket.py module, it would seem that the
dup() method of a _sockobject() just transfers the underlying fd, not
performing a new socket create. So a caller could know about that behavior
and work to call setblocking(0) only when it has a socket not seen before.
In eventlet's case, it would appear the code could be taught this fact, and
adjusted accordingly. I'll propose a patch for eventlet that will eliminate
most of these calls in their case.
Thanks for your time,
-peter
On Sun, Jan 20, 2013 at 1:31 AM, Peter Portante
<peter.a.portante at gmail.com>wrote:
> I don't have a concrete case where a socket object's setblocking() method
> is called with a value in one module, handed off to another module (which
> does not know what the first did with it) which in turn also calls
> setblocking() with the same value. It certainly seems that that not is a
> common pattern, but perhaps one could argue a valid pattern, since the
> state of blocking/nonblocking is maintained in the kernel behind the
> fcntl() system calls.
>
> Here is what I am seeing concretely.
>
> This is the syscall pattern from eventlet/wsgi.py + eventlet/greenio.py
> (after removing redundants call to set_nonblocking (see
> https://bitbucket.org/portante/eventlet/commits/cc27508f4bbaaea566aecb51cf6c8b4629b083bd)).
> First, these are the call stacks for the three calls to the
> set_nonblocking() method, made in one HTTP request; the
> greenio.py:set_nonblocking() method wraps the socketmodule.c:setblocking()
> method:
>
> pid 1385
> File "/usr/bin/swift-object-server", line 22, in <module>
> run_wsgi(conf_file, 'object-server', default_port=6000, **options)
> File "/usr/lib/python2.6/site-packages/swift/common/wsgi.py", line 194,
> in run_wsgi
> run_server(max_clients)
> File "/usr/lib/python2.6/site-packages/swift/common/wsgi.py", line 158,
> in run_server
> wsgi.server(sock, app, NullLogger(), custom_pool=pool)
> File "/usr/lib/python2.6/site-packages/eventlet/wsgi.py", line 598, in
> server
> *client_socket = sock.accept()*
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 163,
> in accept
> return type(self)(client), addr
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 133,
> in __init__
> *set_nonblocking(fd)*
>
> pid 1385
> File "/usr/lib/python2.6/site-packages/eventlet/greenpool.py", line 80,
> in _spawn_n_impl
> func(*args, **kwargs)
> File "/usr/lib/python2.6/site-packages/eventlet/wsgi.py", line 516, in
> process_request
> proto = self.protocol(socket, address, self)
> File "/usr/lib64/python2.6/SocketServer.py", line 616, in __init__
> self.setup()
> File "/usr/lib/python2.6/site-packages/eventlet/wsgi.py", line 174, in
> setup
> *self.rfile = conn.makefile('rb', self.rbufsize)*
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 219,
> in makefile
> return _fileobject(self.dup(), *args, **kw)
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 214,
> in dup
> newsock = type(self)(sock)
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 133,
> in __init__
> *set_nonblocking(fd)*
>
> pid 1385
> File "/usr/lib/python2.6/site-packages/eventlet/greenpool.py", line 80,
> in _spawn_n_impl
> func(*args, **kwargs)
> File "/usr/lib/python2.6/site-packages/eventlet/wsgi.py", line 516, in
> process_request
> proto = self.protocol(socket, address, self)
> File "/usr/lib64/python2.6/SocketServer.py", line 616, in __init__
> self.setup()
> File "/usr/lib/python2.6/site-packages/eventlet/wsgi.py", line 175, in
> setup
> *self.wfile = conn.makefile('wb', self.wbufsize)*
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 219,
> in makefile
> return _fileobject(self.dup(), *args, **kw)
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 214,
> in dup
> newsock = type(self)(sock)
> File "/usr/lib/python2.6/site-packages/eventlet/greenio.py", line 133,
> in __init__
> *set_nonblocking(fd)*
>
> The first one above is expected, the next two unexpectedly result in
> fcntl() calls on the same fd. The strace looks like:
>
> accept(8, {sa_family=AF_INET, sin_port=htons(54375),
> sin_addr=inet_addr("127.0.0.1")}, [16]) = 14
> fcntl(14, F_GETFL) = 0x2 (flags O_RDWR)
> fcntl(14, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> # *self.rfile = conn.makefile('rb', self.rbufsize)*
> fcntl(14, F_GETFL) = 0x802 (flags O_RDWR|O_NONBLOCK)
> fcntl(14, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> # *self.wfile = conn.makefile('wb', self.wbufsize)*
> fcntl(14, F_GETFL) = 0x802 (flags O_RDWR|O_NONBLOCK)
> fcntl(14, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> recvfrom(14, "GET /sdb1/234456/AUTH_del0/gprfc"..., 8192, 0, NULL, NULL) =
> 353
> getsockname(14, {sa_family=AF_INET, sin_port=htons(6010),
> sin_addr=inet_addr("127.0.0.1")}, [16]) = 0
> ...
>
> It appears that conn.makefile() is attempting to dup() the fd, but rfile
> and wfile end up with objects that share the same fd contained in conn.
>
> For eventlet/wsgi.py based webservers, OpenStack Swift is the one I am
> working with right now, handles millions of requests a day on our customer
> systems. Seems like these suggested code changes are trivial compared to
> the number of system calls that can be saved.
>
> Thanks for indulging on this topic,
>
> -peter
>
>
> On Sun, Jan 20, 2013 at 12:38 AM, Guido van Rossum <guido at python.org>wrote:
>
>> On Sat, Jan 19, 2013 at 8:49 PM, Peter Portante
>> <peter.a.portante at gmail.com> wrote:
>> > I noticed while stracing a process that sock.setblocking() calls always
>> > result in pairs of fcntl() calls on Linux. Checking 2.6.8, 2.7.3, and
>> 3.3.0
>> > Modules/socketmodule.c, the code seems to use the following (unless I
>> have
>> > missed something):
>> >
>> > delay_flag = fcntl(s->sock_fd, F_GETFL, 0);
>> > if (block)
>> > delay_flag &= (~O_NONBLOCK);
>> > else
>> > delay_flag |= O_NONBLOCK;
>> > fcntl(s->sock_fd, F_SETFL, delay_flag);
>> >
>> > Perhaps a check to see the flags changed might be worth making?
>> >
>> > int orig_delay_flag = fcntl(s->sock_fd, F_GETFL, 0);
>> > if (block)
>> > delay_flag = orig_delay_flag & (~O_NONBLOCK);
>> > else
>> > delay_flag = orig_delay_flag | O_NONBLOCK;
>> > if (delay_flag != orig_delay_flag)
>> > fcntl(s->sock_fd, F_SETFL, delay_flag);
>> >
>> > OpenStack Swift using the Eventlet module, which sets the accepted
>> socket
>> > non-blocking, resulting in twice the number of fcntl() calls. Not a
>> killer
>> > on performance, but it seems simple enough to save a system call here.
>>
>> This would seem to be a simple enough fix, but it seems you are only
>> fixing it if a *redundant* call to setblocking() is made (i.e. one
>> that attempts to set the flag to the value it already has). Why would
>> this be a common pattern? Even if it was, is the cost of one extra
>> fcntl() call really worth making the code more complex?
>>
>> --
>> --Guido van Rossum (python.org/~guido)
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20130120/a75cbae0/attachment.html>
More information about the Python-Dev
mailing list