From 1e8d5b7ebf82d7ce733d2a7f2b9c43ba52ce33ff Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 4 Dec 2018 12:59:48 -0800 Subject: [PATCH 1/7] Factor out struct _pending_call. --- Include/internal/pycore_ceval.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index b9f2d7d1758537..5c174912d1c896 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -11,6 +11,11 @@ extern "C" { #include "pycore_atomic.h" #include "pythread.h" +struct _pending_call { + int (*func)(void *); + void *arg; +}; + struct _pending_calls { unsigned long main_thread; PyThread_type_lock lock; @@ -21,10 +26,7 @@ struct _pending_calls { Guarded by the GIL. */ int async_exc; #define NPENDINGCALLS 32 - struct { - int (*func)(void *); - void *arg; - } calls[NPENDINGCALLS]; + struct _pending_call calls[NPENDINGCALLS]; int first; int last; }; From 70b0fdc6a68b4d1892f01d20a3c39a14c2672b13 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 4 Dec 2018 13:57:24 -0800 Subject: [PATCH 2/7] Factor out _add_pending_call() and _pop_pending_call(). --- Python/ceval.c | 59 +++++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 3e82ceb952510e..b7b0e55263a95d 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -322,6 +322,34 @@ _PyEval_SignalReceived(void) SIGNAL_PENDING_SIGNALS(); } +int +_add_pending_call(int (*func)(void *), void *arg) +{ + int i = _PyRuntime.ceval.pending.last; + int j = (i + 1) % NPENDINGCALLS; + if (j == _PyRuntime.ceval.pending.first) { + return -1; /* Queue full */ + } + _PyRuntime.ceval.pending.calls[i].func = func; + _PyRuntime.ceval.pending.calls[i].arg = arg; + _PyRuntime.ceval.pending.last = j; + return 0; +} + +void +_pop_pending_call(int (**func)(void *), void **arg) +{ + /* pop one item off the queue while holding the lock */ + int i = _PyRuntime.ceval.pending.first; + if (i == _PyRuntime.ceval.pending.last) { + return; /* Queue empty */ + } + + *func = _PyRuntime.ceval.pending.calls[i].func; + *arg = _PyRuntime.ceval.pending.calls[i].arg; + _PyRuntime.ceval.pending.first = (i + 1) % NPENDINGCALLS; +} + /* This implementation is thread-safe. It allows scheduling to be made from any thread, and even from an executing callback. @@ -330,7 +358,6 @@ _PyEval_SignalReceived(void) int Py_AddPendingCall(int (*func)(void *), void *arg) { - int i, j, result=0; PyThread_type_lock lock = _PyRuntime.ceval.pending.lock; /* try a few times for the lock. Since this mechanism is used @@ -345,6 +372,7 @@ Py_AddPendingCall(int (*func)(void *), void *arg) * this function is called before any bytecode evaluation takes place. */ if (lock != NULL) { + int i; for (i = 0; i<100; i++) { if (PyThread_acquire_lock(lock, NOWAIT_LOCK)) break; @@ -353,20 +381,13 @@ Py_AddPendingCall(int (*func)(void *), void *arg) return -1; } - i = _PyRuntime.ceval.pending.last; - j = (i + 1) % NPENDINGCALLS; - if (j == _PyRuntime.ceval.pending.first) { - result = -1; /* Queue full */ - } else { - _PyRuntime.ceval.pending.calls[i].func = func; - _PyRuntime.ceval.pending.calls[i].arg = arg; - _PyRuntime.ceval.pending.last = j; - } + int res = _add_pending_call(func, arg); + /* signal main loop */ SIGNAL_PENDING_CALLS(); if (lock != NULL) PyThread_release_lock(lock); - return result; + return res; } static int @@ -420,24 +441,18 @@ make_pending_calls(void) /* perform a bounded number of calls, in case of recursion */ for (int i=0; i Date: Tue, 4 Dec 2018 14:20:00 -0800 Subject: [PATCH 3/7] Switch to a linked list. --- Include/internal/pycore_ceval.h | 12 +++++++---- Python/ceval.c | 37 ++++++++++++++++++++++----------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 5c174912d1c896..008668bd8007ae 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -11,11 +11,16 @@ extern "C" { #include "pycore_atomic.h" #include "pythread.h" +struct _pending_call; + struct _pending_call { int (*func)(void *); void *arg; + struct _pending_call *next; }; +#define NPENDINGCALLS 32 + struct _pending_calls { unsigned long main_thread; PyThread_type_lock lock; @@ -25,10 +30,9 @@ struct _pending_calls { thread state. Guarded by the GIL. */ int async_exc; -#define NPENDINGCALLS 32 - struct _pending_call calls[NPENDINGCALLS]; - int first; - int last; + int ncalls; + struct _pending_call *head; + struct _pending_call *last; }; #include "pycore_gil.h" diff --git a/Python/ceval.c b/Python/ceval.c index b7b0e55263a95d..43d25b51cd8803 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -325,29 +325,42 @@ _PyEval_SignalReceived(void) int _add_pending_call(int (*func)(void *), void *arg) { - int i = _PyRuntime.ceval.pending.last; - int j = (i + 1) % NPENDINGCALLS; - if (j == _PyRuntime.ceval.pending.first) { + if (_PyRuntime.ceval.pending.ncalls == NPENDINGCALLS) { return -1; /* Queue full */ } - _PyRuntime.ceval.pending.calls[i].func = func; - _PyRuntime.ceval.pending.calls[i].arg = arg; - _PyRuntime.ceval.pending.last = j; + + struct _pending_call *call = PyMem_RawMalloc(sizeof(struct _pending_call)); + if (call == NULL) { + return -1; + } + call->func = func; + call->arg = arg; + call->next = NULL; + + if (_PyRuntime.ceval.pending.head == NULL) { + _PyRuntime.ceval.pending.head = call; + } + else { + _PyRuntime.ceval.pending.last->next = call; + } + _PyRuntime.ceval.pending.last = call; + _PyRuntime.ceval.pending.ncalls++; return 0; } void _pop_pending_call(int (**func)(void *), void **arg) { - /* pop one item off the queue while holding the lock */ - int i = _PyRuntime.ceval.pending.first; - if (i == _PyRuntime.ceval.pending.last) { + struct _pending_call *call = _PyRuntime.ceval.pending.head; + if (call == NULL) { return; /* Queue empty */ } + _PyRuntime.ceval.pending.head = call->next; + _PyRuntime.ceval.pending.ncalls--; - *func = _PyRuntime.ceval.pending.calls[i].func; - *arg = _PyRuntime.ceval.pending.calls[i].arg; - _PyRuntime.ceval.pending.first = (i + 1) % NPENDINGCALLS; + *func = call->func; + *arg = call->arg; + PyMem_RawFree(call); } /* This implementation is thread-safe. It allows From a38908f9ba5082a66b78950fe05cfb328a3724b4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 4 Dec 2018 14:21:00 -0800 Subject: [PATCH 4/7] Add a TODO. --- Python/ceval.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Python/ceval.c b/Python/ceval.c index 43d25b51cd8803..9df6c4cc563261 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -325,6 +325,7 @@ _PyEval_SignalReceived(void) int _add_pending_call(int (*func)(void *), void *arg) { + // TODO: Drop the limit? if (_PyRuntime.ceval.pending.ncalls == NPENDINGCALLS) { return -1; /* Queue full */ } From 3305a6767e3047b342f7fce5746089ed38a6a26a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 11 Dec 2018 15:12:13 -0700 Subject: [PATCH 5/7] Add a NEWS entry. --- .../Core and Builtins/2018-12-11-15-12-03.bpo-35466.5xu737.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-12-11-15-12-03.bpo-35466.5xu737.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-12-11-15-12-03.bpo-35466.5xu737.rst b/Misc/NEWS.d/next/Core and Builtins/2018-12-11-15-12-03.bpo-35466.5xu737.rst new file mode 100644 index 00000000000000..43ae1bf3ce1344 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-12-11-15-12-03.bpo-35466.5xu737.rst @@ -0,0 +1 @@ +Simply the ceval pending calls list by using a linked list. From 827273f0f6d994d5cd9c0989d51ad8869c3b1f94 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 11 Dec 2018 15:30:01 -0700 Subject: [PATCH 6/7] Make helper functions static. --- Python/ceval.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 9df6c4cc563261..8654915752b0e9 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -322,7 +322,7 @@ _PyEval_SignalReceived(void) SIGNAL_PENDING_SIGNALS(); } -int +static int _add_pending_call(int (*func)(void *), void *arg) { // TODO: Drop the limit? @@ -349,7 +349,7 @@ _add_pending_call(int (*func)(void *), void *arg) return 0; } -void +static void _pop_pending_call(int (**func)(void *), void **arg) { struct _pending_call *call = _PyRuntime.ceval.pending.head; From 76cd3a7e9a843a50b119856456bc995bc8df1f0f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 25 Jan 2019 12:34:57 -0700 Subject: [PATCH 7/7] Add a note about why we kept NPENDINGCALLS. --- Include/internal/pycore_ceval.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 008668bd8007ae..f087879d0c103c 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -19,6 +19,9 @@ struct _pending_call { struct _pending_call *next; }; +// We technically do not need this limit around any longer since we +// moved from a circular queue to a linked list. However, having a +// size limit is still a good idea so we keep the one we already had. #define NPENDINGCALLS 32 struct _pending_calls {