gh-110850: Add PyTime_t C API#115215
Conversation
Add PyTime_t API: * PyTime_t type. * PyTime_MIN and PyTime_MAX constants. * PyTime_AsSecondsDouble(), PyTime_Monotonic(), PyTime_PerfCounter() and PyTime_GetSystemClock() functions. Changes: * Add Include/cpython/pytime.h header. * Add Modules/_testcapi/time.c and Lib/test/test_capi/test_time.py tests on these new functions. * Rename _PyTime_GetMonotonicClock() to PyTime_Monotonic(). * Rename _PyTime_GetPerfCounter() to PyTime_PerfCounter(). * Rename _PyTime_GetSystemClock() to PyTime_Time(). * Rename _PyTime_AsSecondsDouble() to PyTime_AsSecondsDouble(). * Rename _PyTime_MIN to PyTime_MIN. * Rename _PyTime_MAX to PyTime_MAX.
The `PyTime_` prefix is already used by datetime API, e.g. `PyDate_FromDate`.
vstinner
left a comment
There was a problem hiding this comment.
Here is a first review.
I suggest to declare that PyTime_t is a number of nanoseconds and not add PyTime_AsNanoseconds().
There are some Sphinx warnings, like: c:identifier reference target not found: result in time.rst.
|
We should either:
|
|
@encukou: Thanks for working on this PR! |
colesbury
left a comment
There was a problem hiding this comment.
The fallible APIs and generalization of PyTime_t makes these APIs harder to use without, it seems to me, sufficient benefit:
- API users are likely to start relying on details, like the fact that
PyTime_tis 64-bit time in nanoseconds. If we ever in the future wanted an API with a different precision or data type, we'd be better off introducing a new API than breaking users. - The underlying functions shouldn't fail on modern operating systems. Is there any OS we support that doesn't have
CLOCK_MONOTONIC?
I commented below, but I don't think we can set+clear exceptions in the internal versions. They're used in contexts that don't have active PyThreadStates.
|
Oh, I see that fallible vs infallible API has already been extensively discussed in the C-API WG. The API doc should probably explicitly state that the functions require holding the GIL. |
That's the public C API. If Python needs variants which cannot fail, we can still have some in our internal C API. Right, the rationale for reporting errors was discussed in length in the issue: capi-workgroup/decisions#8. |
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
For a few seconds (or many nanoseconds ;-)), I tried to imagine how the API and the implementation would look without PyTime_t type: using int64_t type. It's tempting to make the API as small as possible. IMO the API is better with a decided type, even if it's well documented that it's exactly int64_t. In terms of typing, it's better, it makes the code easier to follow, to review, etc.
|
Yes: unlike |
|
C API WG is in favour, so I'll merge. |
|
Thank you for the code & reviews! |
|
@encukou and me added a bare minimum public C API to read time with a typedef int64_t PyTime_t;
#define PyTime_MIN INT64_MIN
#define PyTime_MAX INT64_MAX
PyAPI_FUNC(double) PyTime_AsSecondsDouble(PyTime_t t);
PyAPI_FUNC(int) PyTime_Monotonic(PyTime_t *result);
PyAPI_FUNC(int) PyTime_PerfCounter(PyTime_t *result);
PyAPI_FUNC(int) PyTime_Time(PyTime_t *result);If someone wants more functions to be exposed, please open a separated issue. |
* pythongh-110850: Add PyTime_t C API Add PyTime_t API: * PyTime_t type. * PyTime_MIN and PyTime_MAX constants. * PyTime_AsSecondsDouble(), PyTime_Monotonic(), PyTime_PerfCounter() and PyTime_GetSystemClock() functions. Co-authored-by: Victor Stinner <vstinner@python.org>
This builds on Victor's PR: #112135
Additionally:
PyTime_tare now an implementation detail;PyTime_AsNanoseconds()is provided to convert to nanoseconds. (Currently, it's a no-op and always succeeds; the API allows reporting errors like overflow in the future.)📚 Documentation preview 📚: https://cpython-previews--115215.org.readthedocs.build/