Skip to content

Commit 3e25a93

Browse files
zackwzackw
committed
Take advantage of PyErr_CheckSignals now being callable without the GIL.
The source tree contains dozens of loops of this form: int res; do { Py_BEGIN_ALLOW_THREADS res = some_system_call(arguments...); Py_END_ALLOW_THREADS } while (res < 0 && errno == EINTR && !PyErr_CheckSignals()); Now that it is possible to call PyErr_CheckSignals without holding the GIL, the locking operations can be moved out of the loop: Py_BEGIN_ALLOW_THREADS do { res = some_system_call(arguments...); } while (res < 0 && errno == EINTR && !PyErr_CheckSignals()); Py_END_ALLOW_THREADS This demonstrates the motivation for making it possible to call PyErr_CheckSignals without holding the GIL. It shouldn’t make any measurable difference performance-wise for _these_ loops, which almost never actually cycle; but for loops that do cycle many times it’s very much desirable to not take and release the GIL every time through. In some cases I also moved uses of _Py_(BEGIN|END)_SUPPRESS_IPH, which is often paired with Py_(BEGIN|END)_ALLOW_THREADS, to keep the pairing intact. It was already considered safe to call PyErr_CheckSignals from both inside and outside an IPH suppression region. More could be done in this vein: I didn’t change any loops where the inside of the loop was more complicated than a single system call, _except_ that I did refactor py_getentropy and py_getrandom (in bootstrap_hash.c) to make it possible to move the unlock and lock outside the loop, demonstrating a more complicated case.
1 parent c0f5de9 commit 3e25a93

File tree

11 files changed

+167
-191
lines changed

11 files changed

+167
-191
lines changed

‎Modules/_io/fileio.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,16 +403,16 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
403403

404404
errno = 0;
405405
if (opener == Py_None) {
406+
Py_BEGIN_ALLOW_THREADS
406407
do {
407-
Py_BEGIN_ALLOW_THREADS
408408
#ifdef MS_WINDOWS
409409
self->fd = _wopen(widename, flags, 0666);
410410
#else
411411
self->fd = open(name, flags, 0666);
412412
#endif
413-
Py_END_ALLOW_THREADS
414413
} while (self->fd < 0 && errno == EINTR &&
415414
!(async_err = PyErr_CheckSignals()));
415+
Py_END_ALLOW_THREADS
416416

417417
if (async_err)
418418
goto error;

‎Modules/_io/winconsoleio.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,9 +647,7 @@ read_console_w(HANDLE handle, DWORD maxlen, DWORD *readlen) {
647647
if (WaitForSingleObjectEx(hInterruptEvent, 100, FALSE)
648648
== WAIT_OBJECT_0) {
649649
ResetEvent(hInterruptEvent);
650-
Py_BLOCK_THREADS
651650
sig = PyErr_CheckSignals();
652-
Py_UNBLOCK_THREADS
653651
if (sig < 0)
654652
break;
655653
}

‎Modules/_multiprocessing/posixshmem.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ _posixshmem_shm_open_impl(PyObject *module, PyObject *path, int flags,
5757
PyErr_SetString(PyExc_ValueError, "embedded null character");
5858
return -1;
5959
}
60+
Py_BEGIN_ALLOW_THREADS
6061
do {
61-
Py_BEGIN_ALLOW_THREADS
6262
fd = shm_open(name, flags, mode);
63-
Py_END_ALLOW_THREADS
6463
} while (fd < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
64+
Py_END_ALLOW_THREADS
6565

6666
if (fd < 0) {
6767
if (!async_err)
@@ -102,11 +102,11 @@ _posixshmem_shm_unlink_impl(PyObject *module, PyObject *path)
102102
PyErr_SetString(PyExc_ValueError, "embedded null character");
103103
return NULL;
104104
}
105+
Py_BEGIN_ALLOW_THREADS
105106
do {
106-
Py_BEGIN_ALLOW_THREADS
107107
rv = shm_unlink(name);
108-
Py_END_ALLOW_THREADS
109108
} while (rv < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
109+
Py_END_ALLOW_THREADS
110110

111111
if (rv < 0) {
112112
if (!async_err)

‎Modules/_multiprocessing/semaphore.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,19 +356,19 @@ _multiprocessing_SemLock_acquire_impl(SemLockObject *self, int blocking,
356356

357357
if (res < 0 && errno == EAGAIN && blocking) {
358358
/* Couldn't acquire immediately, need to block */
359+
Py_BEGIN_ALLOW_THREADS
359360
do {
360-
Py_BEGIN_ALLOW_THREADS
361361
if (!use_deadline) {
362362
res = sem_wait(self->handle);
363363
}
364364
else {
365365
res = sem_timedwait(self->handle, &deadline);
366366
}
367-
Py_END_ALLOW_THREADS
368367
err = errno;
369368
if (res == MP_EXCEPTION_HAS_BEEN_SET)
370369
break;
371370
} while (res < 0 && errno == EINTR && !PyErr_CheckSignals());
371+
Py_END_ALLOW_THREADS
372372
}
373373

374374
if (res < 0) {

‎Modules/fcntlmodule.c

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
7373
}
7474
}
7575

76+
Py_BEGIN_ALLOW_THREADS
7677
do {
77-
Py_BEGIN_ALLOW_THREADS
7878
ret = fcntl(fd, code, (int)int_arg);
79-
Py_END_ALLOW_THREADS
8079
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
80+
Py_END_ALLOW_THREADS
8181
if (ret < 0) {
8282
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
8383
}
@@ -103,11 +103,11 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
103103
memcpy(buf + len, guard, GUARDSZ);
104104
PyBuffer_Release(&view);
105105

106+
Py_BEGIN_ALLOW_THREADS
106107
do {
107-
Py_BEGIN_ALLOW_THREADS
108108
ret = fcntl(fd, code, buf);
109-
Py_END_ALLOW_THREADS
110109
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
110+
Py_END_ALLOW_THREADS
111111
if (ret < 0) {
112112
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
113113
}
@@ -195,11 +195,11 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
195195
}
196196
}
197197

198+
Py_BEGIN_ALLOW_THREADS
198199
do {
199-
Py_BEGIN_ALLOW_THREADS
200200
ret = ioctl(fd, code, int_arg);
201-
Py_END_ALLOW_THREADS
202201
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
202+
Py_END_ALLOW_THREADS
203203
if (ret < 0) {
204204
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
205205
}
@@ -219,11 +219,11 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
219219
memcpy(buf + len, guard, GUARDSZ);
220220
ptr = buf;
221221
}
222+
Py_BEGIN_ALLOW_THREADS
222223
do {
223-
Py_BEGIN_ALLOW_THREADS
224224
ret = ioctl(fd, code, ptr);
225-
Py_END_ALLOW_THREADS
226225
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
226+
Py_END_ALLOW_THREADS
227227
if (ret < 0) {
228228
if (!async_err) {
229229
PyErr_SetFromErrno(PyExc_OSError);
@@ -261,11 +261,11 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
261261
memcpy(buf + len, guard, GUARDSZ);
262262
PyBuffer_Release(&view);
263263

264+
Py_BEGIN_ALLOW_THREADS
264265
do {
265-
Py_BEGIN_ALLOW_THREADS
266266
ret = ioctl(fd, code, buf);
267-
Py_END_ALLOW_THREADS
268267
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
268+
Py_END_ALLOW_THREADS
269269
if (ret < 0) {
270270
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
271271
}
@@ -308,11 +308,11 @@ fcntl_flock_impl(PyObject *module, int fd, int code)
308308
}
309309

310310
#ifdef HAVE_FLOCK
311+
Py_BEGIN_ALLOW_THREADS
311312
do {
312-
Py_BEGIN_ALLOW_THREADS
313313
ret = flock(fd, code);
314-
Py_END_ALLOW_THREADS
315314
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
315+
Py_END_ALLOW_THREADS
316316
#else
317317

318318
#ifndef LOCK_SH
@@ -335,11 +335,11 @@ fcntl_flock_impl(PyObject *module, int fd, int code)
335335
return NULL;
336336
}
337337
l.l_whence = l.l_start = l.l_len = 0;
338+
Py_BEGIN_ALLOW_THREADS
338339
do {
339-
Py_BEGIN_ALLOW_THREADS
340340
ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l);
341-
Py_END_ALLOW_THREADS
342341
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
342+
Py_END_ALLOW_THREADS
343343
}
344344
#endif /* HAVE_FLOCK */
345345
if (ret < 0) {
@@ -439,11 +439,11 @@ fcntl_lockf_impl(PyObject *module, int fd, int code, PyObject *lenobj,
439439
return NULL;
440440
}
441441
l.l_whence = whence;
442+
Py_BEGIN_ALLOW_THREADS
442443
do {
443-
Py_BEGIN_ALLOW_THREADS
444444
ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l);
445-
Py_END_ALLOW_THREADS
446445
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
446+
Py_END_ALLOW_THREADS
447447
}
448448
if (ret < 0) {
449449
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;

0 commit comments

Comments
 (0)