From d2bb21fd9ed4e0fafd2ca70730666f574c90b134 Mon Sep 17 00:00:00 2001 From: user202729 <25191436+user202729@users.noreply.github.com> Date: Tue, 25 Feb 2025 11:43:41 +0700 Subject: [PATCH 1/4] Throttle garbage collector frequency in sig_occurred --- src/cysignals/signals.pxd | 2 ++ src/cysignals/signals.pyx | 47 ++++++++++++++++++++++++++++------ src/cysignals/struct_signals.h | 5 ++++ 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/cysignals/signals.pxd b/src/cysignals/signals.pxd index 531a9a84..64b02e6b 100644 --- a/src/cysignals/signals.pxd +++ b/src/cysignals/signals.pxd @@ -16,6 +16,7 @@ #***************************************************************************** from cpython.object cimport PyObject +from posix.time cimport timespec cdef extern from *: int unlikely(int) nogil # Defined by Cython @@ -29,6 +30,7 @@ cdef extern from "struct_signals.h": cy_atomic_int block_sigint const char* s PyObject* exc_value + timespec gc_pause_until cdef extern from "macros.h" nogil: diff --git a/src/cysignals/signals.pyx b/src/cysignals/signals.pyx index c02ca97f..58df3583 100644 --- a/src/cysignals/signals.pyx +++ b/src/cysignals/signals.pyx @@ -29,11 +29,35 @@ from cpython.ref cimport Py_XINCREF, Py_CLEAR from cpython.exc cimport (PyErr_Occurred, PyErr_NormalizeException, PyErr_Fetch, PyErr_Restore) from cpython.version cimport PY_MAJOR_VERSION +from posix.time cimport clock_gettime, CLOCK_MONOTONIC cimport cython import sys from gc import collect + +cdef inline bint timespec_is_zero(timespec t): + return t.tv_sec == 0 and t.tv_nsec == 0 + + +cdef inline bint timespec_less(timespec a, timespec b): + return a.tv_sec < b.tv_sec or (a.tv_sec == b.tv_sec and a.tv_nsec < b.tv_nsec) + + +cdef inline timespec timespec_add(timespec a, long long b): + cdef long long d = cython.cdiv(a.tv_nsec + b, 1000000000) + a.tv_sec += d + a.tv_nsec += b - d * 1000000000 + if a.tv_nsec < 0: + a.tv_nsec += 1000000000 + a.tv_sec -= 1 + return a + + +cdef inline long long timespec_diff(timespec a, timespec b): + return (a.tv_sec - b.tv_sec) * 1000000000 + a.tv_nsec - b.tv_nsec + + # On Windows, some signals are not pre-defined. # We define them here with values that will never occur in practice # (to avoid compilation errors and conditional compilation). @@ -355,6 +379,7 @@ def python_check_interrupt(sig, frame): sig_check() + cdef void verify_exc_value() noexcept: """ Check that ``cysigs.exc_value`` is still the exception being raised. @@ -396,14 +421,20 @@ cdef void verify_exc_value() noexcept: Py_CLEAR(cysigs.exc_value) return - # To be safe, we run the garbage collector because it may clear - # references to our exception. - try: - collect() - except Exception: - # This can happen when Python is shutting down and the gc module - # is not functional anymore. - pass + cdef timespec cur_time, finish_time + clock_gettime(CLOCK_MONOTONIC, &cur_time) + if timespec_is_zero(cysigs.gc_pause_until) or timespec_less(cysigs.gc_pause_until, cur_time): + # To be safe, we run the garbage collector because it may clear + # references to our exception. + try: + collect() + except Exception: + # This can happen when Python is shutting down and the gc module + # is not functional anymore. + pass + + clock_gettime(CLOCK_MONOTONIC, &finish_time) + cysigs.gc_pause_until = timespec_add(finish_time, timespec_diff(finish_time, cur_time) * 5) # Make sure we still have cysigs.exc_value at all; if this function was # called again during garbage collection it might have already been set diff --git a/src/cysignals/struct_signals.h b/src/cysignals/struct_signals.h index ed3ed1d7..c97a9ea3 100644 --- a/src/cysignals/struct_signals.h +++ b/src/cysignals/struct_signals.h @@ -26,6 +26,7 @@ #include #include #include +#include /* Choose sigjmp/longjmp variant */ @@ -92,6 +93,10 @@ typedef struct * This is used by the sig_occurred function. */ PyObject* exc_value; + /* Time until calling garbage collector is allowed. Using monotonic clock. + * See https://github.com/sagemath/cysignals/issues/215. */ + struct timespec gc_pause_until; + #if ENABLE_DEBUG_CYSIGNALS int debug_level; #endif From a00f98bb089ee1eba4e50d7717420b3d1980bc0a Mon Sep 17 00:00:00 2001 From: user202729 <25191436+user202729@users.noreply.github.com> Date: Wed, 26 Feb 2025 00:45:17 +0700 Subject: [PATCH 2/4] Add sleep at appropriate places to allow garbage collector to rerun --- src/cysignals/tests.pyx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cysignals/tests.pyx b/src/cysignals/tests.pyx index 5aa02f04..287f274b 100644 --- a/src/cysignals/tests.pyx +++ b/src/cysignals/tests.pyx @@ -933,6 +933,8 @@ def test_sig_occurred_finally(): """ TESTS:: + >>> from time import sleep + >>> sleep(0.5) # see PR 227 >>> from cysignals.tests import * >>> test_sig_occurred_finally() No current exception @@ -1022,6 +1024,8 @@ def test_sig_occurred_dealloc_in_gc(): >>> l = [DeallocDebug(), e] >>> l.append(l) >>> gc.disable() + >>> from time import sleep + >>> sleep(0.5) # see PR 227 >>> try: ... del l, e ... print_sig_occurred() From 7289aafac19e37b416ce130da29234637c63bd51 Mon Sep 17 00:00:00 2001 From: user202729 <25191436+user202729@users.noreply.github.com> Date: Wed, 26 Feb 2025 01:00:20 +0700 Subject: [PATCH 3/4] Use time.perf_counter for portability --- src/cysignals/signals.pxd | 3 +-- src/cysignals/signals.pyx | 33 +++++---------------------------- src/cysignals/struct_signals.h | 4 ++-- 3 files changed, 8 insertions(+), 32 deletions(-) diff --git a/src/cysignals/signals.pxd b/src/cysignals/signals.pxd index 64b02e6b..10bcd346 100644 --- a/src/cysignals/signals.pxd +++ b/src/cysignals/signals.pxd @@ -16,7 +16,6 @@ #***************************************************************************** from cpython.object cimport PyObject -from posix.time cimport timespec cdef extern from *: int unlikely(int) nogil # Defined by Cython @@ -30,7 +29,7 @@ cdef extern from "struct_signals.h": cy_atomic_int block_sigint const char* s PyObject* exc_value - timespec gc_pause_until + double gc_pause_until cdef extern from "macros.h" nogil: diff --git a/src/cysignals/signals.pyx b/src/cysignals/signals.pyx index 58df3583..2e2a3779 100644 --- a/src/cysignals/signals.pyx +++ b/src/cysignals/signals.pyx @@ -29,35 +29,13 @@ from cpython.ref cimport Py_XINCREF, Py_CLEAR from cpython.exc cimport (PyErr_Occurred, PyErr_NormalizeException, PyErr_Fetch, PyErr_Restore) from cpython.version cimport PY_MAJOR_VERSION -from posix.time cimport clock_gettime, CLOCK_MONOTONIC +import time cimport cython import sys from gc import collect -cdef inline bint timespec_is_zero(timespec t): - return t.tv_sec == 0 and t.tv_nsec == 0 - - -cdef inline bint timespec_less(timespec a, timespec b): - return a.tv_sec < b.tv_sec or (a.tv_sec == b.tv_sec and a.tv_nsec < b.tv_nsec) - - -cdef inline timespec timespec_add(timespec a, long long b): - cdef long long d = cython.cdiv(a.tv_nsec + b, 1000000000) - a.tv_sec += d - a.tv_nsec += b - d * 1000000000 - if a.tv_nsec < 0: - a.tv_nsec += 1000000000 - a.tv_sec -= 1 - return a - - -cdef inline long long timespec_diff(timespec a, timespec b): - return (a.tv_sec - b.tv_sec) * 1000000000 + a.tv_nsec - b.tv_nsec - - # On Windows, some signals are not pre-defined. # We define them here with values that will never occur in practice # (to avoid compilation errors and conditional compilation). @@ -421,9 +399,8 @@ cdef void verify_exc_value() noexcept: Py_CLEAR(cysigs.exc_value) return - cdef timespec cur_time, finish_time - clock_gettime(CLOCK_MONOTONIC, &cur_time) - if timespec_is_zero(cysigs.gc_pause_until) or timespec_less(cysigs.gc_pause_until, cur_time): + cdef double cur_time = time.perf_counter(), finish_time + if cysigs.gc_pause_until == 0 or cysigs.gc_pause_until < cur_time: # To be safe, we run the garbage collector because it may clear # references to our exception. try: @@ -433,8 +410,8 @@ cdef void verify_exc_value() noexcept: # is not functional anymore. pass - clock_gettime(CLOCK_MONOTONIC, &finish_time) - cysigs.gc_pause_until = timespec_add(finish_time, timespec_diff(finish_time, cur_time) * 5) + finish_time = time.perf_counter() + cysigs.gc_pause_until = finish_time + (finish_time - cur_time) * 5 # Make sure we still have cysigs.exc_value at all; if this function was # called again during garbage collection it might have already been set diff --git a/src/cysignals/struct_signals.h b/src/cysignals/struct_signals.h index c97a9ea3..9682dae7 100644 --- a/src/cysignals/struct_signals.h +++ b/src/cysignals/struct_signals.h @@ -93,9 +93,9 @@ typedef struct * This is used by the sig_occurred function. */ PyObject* exc_value; - /* Time until calling garbage collector is allowed. Using monotonic clock. + /* Time until calling garbage collector is allowed. Using Python time.perf_counter. * See https://github.com/sagemath/cysignals/issues/215. */ - struct timespec gc_pause_until; + double gc_pause_until; #if ENABLE_DEBUG_CYSIGNALS int debug_level; From e82fd9eab720287072f826eaa5647e587b09ea31 Mon Sep 17 00:00:00 2001 From: user202729 <25191436+user202729@users.noreply.github.com> Date: Wed, 26 Feb 2025 01:05:04 +0700 Subject: [PATCH 4/4] Fix lint --- src/cysignals/signals.pyx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cysignals/signals.pyx b/src/cysignals/signals.pyx index 2e2a3779..50c7f53c 100644 --- a/src/cysignals/signals.pyx +++ b/src/cysignals/signals.pyx @@ -35,7 +35,6 @@ cimport cython import sys from gc import collect - # On Windows, some signals are not pre-defined. # We define them here with values that will never occur in practice # (to avoid compilation errors and conditional compilation). @@ -357,7 +356,6 @@ def python_check_interrupt(sig, frame): sig_check() - cdef void verify_exc_value() noexcept: """ Check that ``cysigs.exc_value`` is still the exception being raised.