-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-110850: Add PyTime_t C API #115215
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`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_t
is 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_t
are 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/