From 9a69ac4701ac3ba36be730856a97469d6c0c6bd7 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 29 Apr 2024 17:00:25 +0200 Subject: [PATCH 1/3] gh-110850: Add PyTime_TimeRaw() function Add "Raw" variant of PyTime functions: * PyTime_MonotonicRaw() * PyTime_PerfCounterRaw() * PyTime_TimeRaw() Changes: * Add documentation and tests. Tests release the GIL while calling raw clock functions. * py_get_system_clock() and py_get_monotonic_clock() now check that the GIL is hold by the caller if raise_exc is non-zero. * Reimplement "Unchecked" functions with raw clock functions. --- Doc/c-api/time.rst | 25 ++++++ Doc/whatsnew/3.13.rst | 12 ++- Include/cpython/pytime.h | 4 + Lib/test/test_capi/test_time.py | 19 +++-- ...-04-29-17-19-07.gh-issue-110850.vcpLn1.rst | 7 ++ Modules/_testcapi/time.c | 54 +++++++++++++ Python/pytime.c | 79 +++++++++++++------ 7 files changed, 165 insertions(+), 35 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2024-04-29-17-19-07.gh-issue-110850.vcpLn1.rst diff --git a/Doc/c-api/time.rst b/Doc/c-api/time.rst index 7791cdb1781055..17ddbc5f9cd3e7 100644 --- a/Doc/c-api/time.rst +++ b/Doc/c-api/time.rst @@ -72,6 +72,31 @@ with the :term:`GIL` held. See :func:`time.time` for details important on this clock. +Raw Clock Functions +------------------- + +Similar to clock functions, but don't set an exception on error and don't +require the caller to hold the GIL. + +The functions return ``0`` on success, or set ``*result`` to ``0`` return +``-1`` on failure. + +.. c:function:: int PyTime_MonotonicRaw(PyTime_t *result) + + Similar to :c:func:`PyTime_Monotonic`, + but don't set an exception on error and don't require holding the GIL. + +.. c:function:: int PyTime_PerfCounterRaw(PyTime_t *result) + + Similar to :c:func:`PyTime_PerfCounter`, + but don't set an exception on error and don't require holding the GIL. + +.. c:function:: int PyTime_TimeRaw(PyTime_t *result) + + Similar to :c:func:`PyTime_Time`, + but don't set an exception on error and don't require holding the GIL. + + Conversion functions -------------------- diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 98349a5984bb7e..d23a0f07204761 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -1882,9 +1882,15 @@ New Features * :c:type:`PyTime_t` type. * :c:var:`PyTime_MIN` and :c:var:`PyTime_MAX` constants. - * :c:func:`PyTime_AsSecondsDouble` - :c:func:`PyTime_Monotonic`, :c:func:`PyTime_PerfCounter`, and - :c:func:`PyTime_Time` functions. + * Add functions: + + * :c:func:`PyTime_AsSecondsDouble`. + * :c:func:`PyTime_Monotonic`. + * :c:func:`PyTime_MonotonicRaw`. + * :c:func:`PyTime_PerfCounter`. + * :c:func:`PyTime_PerfCounterRaw`. + * :c:func:`PyTime_Time`. + * :c:func:`PyTime_TimeRaw`. (Contributed by Victor Stinner and Petr Viktorin in :gh:`110850`.) diff --git a/Include/cpython/pytime.h b/Include/cpython/pytime.h index d8244700d614ce..5c68110aeedb86 100644 --- a/Include/cpython/pytime.h +++ b/Include/cpython/pytime.h @@ -16,6 +16,10 @@ PyAPI_FUNC(int) PyTime_Monotonic(PyTime_t *result); PyAPI_FUNC(int) PyTime_PerfCounter(PyTime_t *result); PyAPI_FUNC(int) PyTime_Time(PyTime_t *result); +PyAPI_FUNC(int) PyTime_MonotonicRaw(PyTime_t *result); +PyAPI_FUNC(int) PyTime_PerfCounterRaw(PyTime_t *result); +PyAPI_FUNC(int) PyTime_TimeRaw(PyTime_t *result); + #ifdef __cplusplus } #endif diff --git a/Lib/test/test_capi/test_time.py b/Lib/test/test_capi/test_time.py index 10b7fbf2c372a3..17ebd7c1962d48 100644 --- a/Lib/test/test_capi/test_time.py +++ b/Lib/test/test_capi/test_time.py @@ -18,11 +18,6 @@ def test_min_max(self): self.assertEqual(PyTime_MIN, -2**63) self.assertEqual(PyTime_MAX, 2**63 - 1) - def check_clock(self, c_func, py_func): - t1 = c_func() - t2 = py_func() - self.assertAlmostEqual(t1, t2, delta=CLOCK_RES) - def test_assecondsdouble(self): # Test PyTime_AsSecondsDouble() def ns_to_sec(ns): @@ -58,14 +53,22 @@ def ns_to_sec(ns): self.assertEqual(_testcapi.PyTime_AsSecondsDouble(ns), ns_to_sec(ns)) + def check_clock(self, c_func, py_func): + t1 = c_func() + t2 = py_func() + self.assertAlmostEqual(t1, t2, delta=CLOCK_RES) + def test_monotonic(self): - # Test PyTime_Monotonic() + # Test PyTime_Monotonic() and PyTime_MonotonicRaw() self.check_clock(_testcapi.PyTime_Monotonic, time.monotonic) + self.check_clock(_testcapi.PyTime_MonotonicRaw, time.monotonic) def test_perf_counter(self): - # Test PyTime_PerfCounter() + # Test PyTime_PerfCounter() and PyTime_PerfCounterRaw() self.check_clock(_testcapi.PyTime_PerfCounter, time.perf_counter) + self.check_clock(_testcapi.PyTime_PerfCounterRaw, time.perf_counter) def test_time(self): - # Test PyTime_time() + # Test PyTime_Time() and PyTime_TimeRaw() self.check_clock(_testcapi.PyTime_Time, time.time) + self.check_clock(_testcapi.PyTime_TimeRaw, time.time) diff --git a/Misc/NEWS.d/next/C API/2024-04-29-17-19-07.gh-issue-110850.vcpLn1.rst b/Misc/NEWS.d/next/C API/2024-04-29-17-19-07.gh-issue-110850.vcpLn1.rst new file mode 100644 index 00000000000000..786da0147eae46 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-04-29-17-19-07.gh-issue-110850.vcpLn1.rst @@ -0,0 +1,7 @@ +Add "Raw" variant of PyTime functions + +* :c:func:`PyTime_MonotonicRaw` +* :c:func:`PyTime_PerfCounterRaw` +* :c:func:`PyTime_TimeRaw` + +Patch by Victor Stinner. diff --git a/Modules/_testcapi/time.c b/Modules/_testcapi/time.c index 68f082bf3f3d88..3f07c1c4836071 100644 --- a/Modules/_testcapi/time.c +++ b/Modules/_testcapi/time.c @@ -58,6 +58,23 @@ test_pytime_monotonic(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) } +static PyObject* +test_pytime_monotonic_raw(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) +{ + PyTime_t t; + int res; + Py_BEGIN_ALLOW_THREADS + res = PyTime_MonotonicRaw(&t); + Py_END_ALLOW_THREADS + if (res < 0) { + PyErr_SetString(PyExc_RuntimeError, "PyTime_MonotonicRaw() failed"); + return NULL; + } + assert(res == 0); + return pytime_as_float(t); +} + + static PyObject* test_pytime_perf_counter(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) { @@ -71,6 +88,23 @@ test_pytime_perf_counter(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) } +static PyObject* +test_pytime_perf_counter_raw(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) +{ + PyTime_t t; + int res; + Py_BEGIN_ALLOW_THREADS + res = PyTime_PerfCounterRaw(&t); + Py_END_ALLOW_THREADS + if (res < 0) { + PyErr_SetString(PyExc_RuntimeError, "PyTime_PerfCounterRaw() failed"); + return NULL; + } + assert(res == 0); + return pytime_as_float(t); +} + + static PyObject* test_pytime_time(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) { @@ -84,11 +118,31 @@ test_pytime_time(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) } +static PyObject* +test_pytime_time_raw(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) +{ + PyTime_t t; + int res; + Py_BEGIN_ALLOW_THREADS + res = PyTime_TimeRaw(&t); + Py_END_ALLOW_THREADS + if (res < 0) { + PyErr_SetString(PyExc_RuntimeError, "PyTime_TimeRaw() failed"); + return NULL; + } + assert(res == 0); + return pytime_as_float(t); +} + + static PyMethodDef test_methods[] = { {"PyTime_AsSecondsDouble", test_pytime_assecondsdouble, METH_VARARGS}, {"PyTime_Monotonic", test_pytime_monotonic, METH_NOARGS}, + {"PyTime_MonotonicRaw", test_pytime_monotonic_raw, METH_NOARGS}, {"PyTime_PerfCounter", test_pytime_perf_counter, METH_NOARGS}, + {"PyTime_PerfCounterRaw", test_pytime_perf_counter_raw, METH_NOARGS}, {"PyTime_Time", test_pytime_time, METH_NOARGS}, + {"PyTime_TimeRaw", test_pytime_time_raw, METH_NOARGS}, {NULL}, }; diff --git a/Python/pytime.c b/Python/pytime.c index d5b38047b6db31..d8c46af99b979a 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -898,6 +898,10 @@ static int py_get_system_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) { assert(info == NULL || raise_exc); + if (raise_exc) { + // raise_exc requires to hold the GIL + assert(PyGILState_Check()); + } #ifdef MS_WINDOWS FILETIME system_time; @@ -1004,29 +1008,37 @@ py_get_system_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) } -PyTime_t -_PyTime_TimeUnchecked(void) +int +PyTime_Time(PyTime_t *result) { - PyTime_t t; - if (py_get_system_clock(&t, NULL, 0) < 0) { - // If clock_gettime(CLOCK_REALTIME) or gettimeofday() fails: - // silently ignore the failure and return 0. - t = 0; + if (py_get_system_clock(result, NULL, 1) < 0) { + *result = 0; + return -1; } - return t; + return 0; } int -PyTime_Time(PyTime_t *result) +PyTime_TimeRaw(PyTime_t *result) { - if (py_get_system_clock(result, NULL, 1) < 0) { + if (py_get_system_clock(result, NULL, 0) < 0) { *result = 0; return -1; } return 0; } + +PyTime_t +_PyTime_TimeUnchecked(void) +{ + PyTime_t t; + (void)PyTime_TimeRaw(&t); + return t; +} + + int _PyTime_TimeWithInfo(PyTime_t *t, _Py_clock_info_t *info) { @@ -1140,6 +1152,10 @@ static int py_get_monotonic_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) { assert(info == NULL || raise_exc); + if (raise_exc) { + // raise_exc requires to hold the GIL + assert(PyGILState_Check()); + } #if defined(MS_WINDOWS) if (py_get_win_perf_counter(tp, info, raise_exc) < 0) { @@ -1225,22 +1241,21 @@ py_get_monotonic_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) } -PyTime_t -_PyTime_MonotonicUnchecked(void) +int +PyTime_Monotonic(PyTime_t *result) { - PyTime_t t; - if (py_get_monotonic_clock(&t, NULL, 0) < 0) { - // Ignore silently the error and return 0. - t = 0; + if (py_get_monotonic_clock(result, NULL, 1) < 0) { + *result = 0; + return -1; } - return t; + return 0; } int -PyTime_Monotonic(PyTime_t *result) +PyTime_MonotonicRaw(PyTime_t *result) { - if (py_get_monotonic_clock(result, NULL, 1) < 0) { + if (py_get_monotonic_clock(result, NULL, 0) < 0) { *result = 0; return -1; } @@ -1248,6 +1263,15 @@ PyTime_Monotonic(PyTime_t *result) } +PyTime_t +_PyTime_MonotonicUnchecked(void) +{ + PyTime_t t; + (void)PyTime_MonotonicRaw(&t); + return t; +} + + int _PyTime_MonotonicWithInfo(PyTime_t *tp, _Py_clock_info_t *info) { @@ -1262,17 +1286,24 @@ _PyTime_PerfCounterWithInfo(PyTime_t *t, _Py_clock_info_t *info) } -PyTime_t -_PyTime_PerfCounterUnchecked(void) +int +PyTime_PerfCounter(PyTime_t *result) { - return _PyTime_MonotonicUnchecked(); + return PyTime_Monotonic(result); } int -PyTime_PerfCounter(PyTime_t *result) +PyTime_PerfCounterRaw(PyTime_t *result) { - return PyTime_Monotonic(result); + return PyTime_MonotonicRaw(result); +} + + +PyTime_t +_PyTime_PerfCounterUnchecked(void) +{ + return _PyTime_MonotonicUnchecked(); } From 615f8a0d31d11db2101da197b32df91ccc2364f9 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 1 May 2024 17:35:50 +0200 Subject: [PATCH 2/3] Suggestions for `time_raw` (#2) --- Doc/c-api/time.rst | 8 ++++++-- Modules/_testcapi/time.c | 6 ++++++ Python/pytime.c | 10 ++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Doc/c-api/time.rst b/Doc/c-api/time.rst index 17ddbc5f9cd3e7..5cfdef71b3e191 100644 --- a/Doc/c-api/time.rst +++ b/Doc/c-api/time.rst @@ -78,8 +78,12 @@ Raw Clock Functions Similar to clock functions, but don't set an exception on error and don't require the caller to hold the GIL. -The functions return ``0`` on success, or set ``*result`` to ``0`` return -``-1`` on failure. +On success, the functions return ``0``. + +On failure, they set ``*result`` to ``0`` and return ``-1``, *without* setting +an exception. To get the cause of the error, acquire the GIL and call the +regular (non-``Raw``) function. Note that the regular function may succeed after +the ``Raw`` one failed. .. c:function:: int PyTime_MonotonicRaw(PyTime_t *result) diff --git a/Modules/_testcapi/time.c b/Modules/_testcapi/time.c index 3f07c1c4836071..464cf5c3125012 100644 --- a/Modules/_testcapi/time.c +++ b/Modules/_testcapi/time.c @@ -51,6 +51,7 @@ test_pytime_monotonic(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) PyTime_t t; int res = PyTime_Monotonic(&t); if (res < 0) { + assert(t == 0); return NULL; } assert(res == 0); @@ -67,6 +68,7 @@ test_pytime_monotonic_raw(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) res = PyTime_MonotonicRaw(&t); Py_END_ALLOW_THREADS if (res < 0) { + assert(t == 0); PyErr_SetString(PyExc_RuntimeError, "PyTime_MonotonicRaw() failed"); return NULL; } @@ -81,6 +83,7 @@ test_pytime_perf_counter(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) PyTime_t t; int res = PyTime_PerfCounter(&t); if (res < 0) { + assert(t == 0); return NULL; } assert(res == 0); @@ -97,6 +100,7 @@ test_pytime_perf_counter_raw(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args res = PyTime_PerfCounterRaw(&t); Py_END_ALLOW_THREADS if (res < 0) { + assert(t == 0); PyErr_SetString(PyExc_RuntimeError, "PyTime_PerfCounterRaw() failed"); return NULL; } @@ -111,6 +115,7 @@ test_pytime_time(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) PyTime_t t; int res = PyTime_Time(&t); if (res < 0) { + assert(t == 0); return NULL; } assert(res == 0); @@ -127,6 +132,7 @@ test_pytime_time_raw(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args)) res = PyTime_TimeRaw(&t); Py_END_ALLOW_THREADS if (res < 0) { + assert(t == 0); PyErr_SetString(PyExc_RuntimeError, "PyTime_TimeRaw() failed"); return NULL; } diff --git a/Python/pytime.c b/Python/pytime.c index d8c46af99b979a..d2bda090ad99d3 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1034,7 +1034,10 @@ PyTime_t _PyTime_TimeUnchecked(void) { PyTime_t t; - (void)PyTime_TimeRaw(&t); + int result = PyTime_TimeRaw(&t); + // We assume that getting the time succeeds on current platforms. + // If this ever is not the case, use PyTime_TimeRaw instead. + assert(result == 0); return t; } @@ -1267,7 +1270,10 @@ PyTime_t _PyTime_MonotonicUnchecked(void) { PyTime_t t; - (void)PyTime_MonotonicRaw(&t); + int result = PyTime_MonotonicRaw(&t); + // We assume that getting the time succeeds on current platforms. + // If this ever is not the case, use PyTime_MonotonicRaw instead. + assert(result == 0); return t; } From 283bc6398983ccb0c5bb749b55aac3f2cc3e68ae Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 1 May 2024 17:40:47 +0200 Subject: [PATCH 3/3] Use Py_FatalError() in debug mode --- Python/pytime.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Python/pytime.c b/Python/pytime.c index d2bda090ad99d3..12b36bbc881f9a 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1034,10 +1034,14 @@ PyTime_t _PyTime_TimeUnchecked(void) { PyTime_t t; +#ifdef Py_DEBUG int result = PyTime_TimeRaw(&t); - // We assume that getting the time succeeds on current platforms. - // If this ever is not the case, use PyTime_TimeRaw instead. - assert(result == 0); + if (result != 0) { + Py_FatalError("unable to read the system clock"); + } +#else + (void)PyTime_TimeRaw(&t); +#endif return t; } @@ -1270,10 +1274,14 @@ PyTime_t _PyTime_MonotonicUnchecked(void) { PyTime_t t; +#ifdef Py_DEBUG int result = PyTime_MonotonicRaw(&t); - // We assume that getting the time succeeds on current platforms. - // If this ever is not the case, use PyTime_MonotonicRaw instead. - assert(result == 0); + if (result != 0) { + Py_FatalError("unable to read the monotonic clock"); + } +#else + (void)PyTime_MonotonicRaw(&t); +#endif return t; }