gh-110850: Add PyTime_t C API#112135
Conversation
colesbury
left a comment
There was a problem hiding this comment.
A few drive-by comments
Sorry, something went wrong.
|
@colesbury: I copied the long comment from the private header file as the documentation. I already had doubts about mentioning details of the internal C API which are not exposed in the public C API yet. I will reorganize the "public" documentation to only leave documentation relevant to the few functions made public. |
Sorry, something went wrong.
Not being able to report errors is not safe. The risk of error while reading time is low, whereas the cost of having to report errors to the caller while reading time is high. It makes code way more complicated. There are some specific C extensions where we cannot even report errors, since the API doesn't let to report errors to the caller. One example if the _PyTime_t deadline = _PyTime_Add(_PyTime_GetMonotonicClock(), timeout);
_PyTime_AsTimespec_clamp(deadline, &abs_timeout);It uses In existing code which doesn't use the Since I designed and implemented PEP 418 in 2012, I paid a lot of attention to bug reports about "reading time". I only recall on bug report in Red Hat bug tracker of an application running in a sandbox and the sandbox blocked syscalls to read time. Well, it was more a sandbox configuration issue, than a bug in Python. When I implemented PEP 418, I was scared of regressions, so I added a read the monotonic clock at Python startup: so Python was able to check for "read time" error at one place, and only one place. That's how I discovered the sandbox issue. Later, I removed these checks at startup. I decided that it's not worth it. |
Sorry, something went wrong.
|
@colesbury @scoder @serhiy-storchaka: Please review the updated PR.
|
Sorry, something went wrong.
colesbury
left a comment
There was a problem hiding this comment.
A few suggestions about the documentation below.
Sorry, something went wrong.
@colesbury: Thanks, I applied your suggestions. Does the updated doc look better? |
Sorry, something went wrong.
|
I have too many things on my plate. I prefer to close the PR for now. |
Sorry, something went wrong.
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.
|
I reopened my PR since #110850 issue is now marked as a release blocker issue. I rebased my PR on main. |
Sorry, something went wrong.
|
I created a C API Working Group issue: capi-workgroup/decisions#8 |
Sorry, something went wrong.
Add PyTime_t API:
📚 Documentation preview 📚: https://cpython-previews--112135.org.readthedocs.build/