classification
Title: some posix module functions unnecessarily release the GIL
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, eric.smith, neologix, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2011-03-03 10:11 by neologix, last changed 2011-04-23 15:34 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
gil_release_py3k.diff neologix, 2011-03-03 19:50 review
Messages (13)
msg129948 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-03-03 10:15
Some posix module functions unnecessarily release the GIL.
For example, posix_dup, posix_dup2 and posix_pipe all release the GIL, but those are non-blocking syscalls (the don't imply any I/O, only modifying the process file descriptors table).
This leads to the famous convoy effect (see http://bugs.python.org/issue7946).

For example:

$ cat /tmp/test_dup2.py 
import os
import threading
import sys
import time


def do_loop():
    while True:
        pass

t = threading.Thread(target=do_loop)
t.setDaemon(True)
t.start()

f = os.open(sys.argv[1], os.O_RDONLY)

for i in range(4, 1000):
    os.dup2(f, i)

Whith  GIL release/acquire:

$ time ./python /tmp/test_dup2.py  /etc/fstab 

real    0m5.238s
user    0m5.223s
sys     0m0.009s

$ time ./python /tmp/test_pipe.py 

real    0m3.083s
user    0m3.074s
sys     0m0.007s

Without GIL release/acquire:

$ time ./python /tmp/test_dup2.py  /etc/fstab 

real    0m0.094s
user    0m0.077s
sys     0m0.010s

$ time ./python /tmp/test_pipe.py 

real    0m0.088s
user    0m0.074s
sys     0m0.008s
msg129949 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-03-03 10:51
On Windows at least, these functions call malloc() and DuplicateHandle().
msg129954 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-03-03 11:25
I didn't even know that Windows had such calls.
But anyway, if we start releasing the GIL around each malloc call, then it's going to get really complicated:

static PyObject *
posix_geteuid(PyObject *self, PyObject *noargs)
{
 	return PyLong_FromLong((long)geteuid());
}

PyLong_FromLong -> _PyLong_New -> PyObject_MALLOC which can call malloc.

As for DuplicateHandle, I assume it's as fast as Unix's dup(2).
msg129957 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-03-03 11:53
What are you trying to achieve? Do you have loops which contain no other syscall than os.dup2()?
msg129961 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-03-03 12:39
Well, those are contrived examples showing the effect of the convoy effect induced by those unneeded GIL release/acquire: releasing and re-acquiring the GIL comes with a cost (e.g. under Linux, futex are really fast in the uncontended case since handled in use space but much slower when there's contention), and subverts the OS scheduling policy (forcing the thread to drop/re-acquire the GIL make the thread block after having consumed a small amount of its time slice and increases the context switching rate). I think that releasing and re-acquiring the GIL should only be done around potentially blocking calls.

> Do you have loops which contain no other syscall than os.dup2()?

No, but it's not a reason for penalizing threads that use dup, dup2 or pipe.
msg129962 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-03 12:42
> Well, those are contrived examples showing the effect of the convoy
> effect induced by those unneeded GIL release/acquire: releasing and
> re-acquiring the GIL comes with a cost (e.g. under Linux, futex are
> really fast in the uncontended case since handled in use space but
> much slower when there's contention), and subverts the OS scheduling
> policy (forcing the thread to drop/re-acquire the GIL make the thread
> block after having consumed a small amount of its time slice and
> increases the context switching rate). I think that releasing and
> re-acquiring the GIL should only be done around potentially blocking
> calls.

Do you want to propose a patch?
msg129965 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-03-03 12:59
> Do you want to propose a patch?

Sure, if removing those calls to Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS seems reasonable (I might haved missed something obvious).
Just to be clear, I'm not at all criticizing the current GIL implementation, there's been a great work done on it.
I'm just saying that releasing and re-acquiring the GIL around fast syscalls is probaly not a good idea.
msg129966 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-03 13:09
> Just to be clear, I'm not at all criticizing the current GIL
> implementation, there's been a great work done on it.
> I'm just saying that releasing and re-acquiring the GIL around fast
> syscalls is probaly not a good idea.

If these syscalls aren't likely to yield control to another thread, then
I agree there's no point in releasing the GIL around them.
(but is it the case that they are always fast? for example, how about
dup() on a network file system? or is it indifferent?)
msg129970 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-03-03 14:32
2011/3/3 Antoine Pitrou <report@bugs.python.org>:
>
> Antoine Pitrou <pitrou@free.fr> added the comment:
>
>> Just to be clear, I'm not at all criticizing the current GIL
>> implementation, there's been a great work done on it.
>> I'm just saying that releasing and re-acquiring the GIL around fast
>> syscalls is probaly not a good idea.
>
> If these syscalls aren't likely to yield control to another thread, then
> I agree there's no point in releasing the GIL around them.
> (but is it the case that they are always fast? for example, how about
> dup() on a network file system? or is it indifferent?)

The initial open can take long, but once it's open, calling dup just
implies copying a reference to the open file (a pointer) to the file
descriptor table. No I/O is done (I tested it one a NFS mount).
Now, I don't know Windows at all, but I'm pretty sure that every
operating system does more or less the same thing, and that those
three calls (there might be others) don't block.

>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue11382>
> _______________________________________
>
msg130000 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-03-03 19:50
Attached is a patch removing useless calls to
Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS for several posix
functions.
It's straigthforward, but since I only have Linux boxes, I couldn't
test it under Windows.
msg134230 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-04-21 17:51
Is there anything I can do to help this move forward ?
msg134304 - (view) Author: Roundup Robot (python-dev) Date: 2011-04-23 15:21
New changeset eb7297fd5840 by Antoine Pitrou in branch 'default':
Issue #11382: Trivial system calls, such as dup() or pipe(), needn't
http://hg.python.org/cpython/rev/eb7297fd5840
msg134305 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-04-23 15:34
Sorry for the delay, I had forgotten about this issue. Thank you for the patch!
History
Date User Action Args
2011-04-23 15:34:08pitrousetstatus: open -> closed
resolution: fixed
messages: + msg134305

stage: resolved
2011-04-23 15:21:43python-devsetnosy: + python-dev
messages: + msg134304
2011-04-21 17:51:50neologixsetmessages: + msg134230
2011-03-04 22:54:03terry.reedysetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
type: performance
versions: + Python 3.3
2011-03-03 19:50:25neologixsetfiles: + gil_release_py3k.diff

messages: + msg130000
keywords: + patch
nosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
2011-03-03 14:32:41neologixsetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
messages: + msg129970
2011-03-03 13:09:11pitrousetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
messages: + msg129966
2011-03-03 12:59:08neologixsetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
messages: + msg129965
2011-03-03 12:42:22pitrousetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
messages: + msg129962
2011-03-03 12:39:11neologixsetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
messages: + msg129961
2011-03-03 11:53:07amaury.forgeotdarcsetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
messages: + msg129957
2011-03-03 11:25:08neologixsetnosy: amaury.forgeotdarc, pitrou, eric.smith, neologix
messages: + msg129954
2011-03-03 10:51:28amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg129949
2011-03-03 10:41:25eric.smithsetnosy: + eric.smith, pitrou
components: + Interpreter Core
2011-03-03 10:15:32neologixsetmessages: + msg129948
title: some posix module functions -> some posix module functions unnecessarily release the GIL
2011-03-03 10:11:55neologixcreate