GeezerVison is showing github.com — gh-110850: Add PyTime_TimeUnchecked() function by vstinner · Pull Request #115973 · python/cpython
Conversation
vstinner
commented
Feb 26, 2024
•
edited by github-actions
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Add functions:
- PyTime_MonotonicUnchecked()
- PyTime_PerfCounterUnchecked()
- PyTime_TimeUnchecked()
Python now fails with a fatal error at startup if reading the system clock, the monotonic clock or the perf counter clock fails.
Add py_perf_counter() function to factorize code.
📚 Documentation preview 📚: https://cpython-previews--115973.org.readthedocs.build/
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add functions:
* PyTime_MonotonicUnchecked()
* PyTime_PerfCounterUnchecked()
* PyTime_TimeUnchecked()
Python now fails with a fatal error at startup if reading the system
clock, the monotonic clock or the perf counter clock fails.
Add py_perf_counter() function to factorize code.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
OK, I'll repeat my questions here, too.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Do we guarantee that you can call these functions before Python is initialized & after it's finalized?
No function can be called before init/after init, except of these few functions listed there: https://docs.python.org/dev/c-api/init.html
What concrete use cases / user needs does this solve that are not solved by <time.h> functions?
Convenient int64_t format using Unix epoch 1970 in all platforms. Examples:
- Windows epoch is in 1600
- Windows and macOS() return a fraction for perf_counter()
- Unix return a floating point number as timespec
- There are 3 to 5 implementations per function, it's hard to rewrite them. Python has already a written implementation with tests and is well tested for 10 years. pytime.c is like 1000 lines of C code, "using time.h" is not so straightforward.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
No function can be called before init/after init, except of these few functions
Similar for holding the GIL, and in my mind, doesn't need GIL and doesn't need the runtime initialized are the same set of functions. I'm not happy about another bit to track in the upcoming list of all Python API.
Convenient int64_t format using Unix epoch 1970 in all platforms.
I know what it does; I'm asking why. Do we know about a use case that needs this, and can't GIL for exception-based error handling?
I also know Cython wraps and re-exports the private API. I'm asking why it's needed. (If we just need to keep Cython going, we can add back the underscored versions, or add these as PyUnstable.)
There are 3 to 5 implementations per function
And when we add a sixth one, we might need to increase the set of errors the Unchecked API silently ignores. I'd like to avoid that.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
vstinner
commented
Feb 29, 2024
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
I know what it does; I'm asking why. Do we know about a use case that needs this, and can't GIL for exception-based error handling?
The issue was created for Cython. I cannot check right now how people use Cython.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Similar for holding the GIL, and in my mind, doesn't need GIL and doesn't need the runtime initialized are the same set of functions. I'm not happy about another bit to track in the upcoming list of all Python API.
The Python C API already has many functions which don't need to hold the GIL. Check fileutils.h functions.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@scoder: You may reply to the Cython question.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
I also know Cython wraps and re-exports the private API. I'm asking why it's needed.
It's convenient. It gives the same results as Python's time module, in a platform independent way. Exposing the C functions without requiring the GIL makes them available for e.g. timing sections in thread-parallel code.
If you hold the GIL, you can probably (often) afford to keep a reference to time.time et at. around and call that, which is fairly fast except for the object creation. You'd have less of an urge to require a C-API for that. Removing the GIL requirement opens up a much larger plane of applicability.
|
vstinner reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The Python C API already has many functions which don't need to hold the GIL. Check fileutils.h functions.
OK! There are three functions in two fileutils.h files:
None of these are documented to not require the GIL. If they work without the GIL in practice, I consider that an accident of the implementation.
There is also pycore_fileutils.h, but private API is not relevant here.
Thanks for the use cases.
I think they're conflating use cases for perf timers with ones for wall-clock time (where the epoch matters, but GIL overhead seems less of an issue).
As far as I can see, Cython only exposes wall-clock time.
I get that Python exposes the “best” implementation, across platforms, but it still seems to me that Python-style error handling is an appropriate price for that, and <time.h> is a reasonable fallback.
But now I can see myself being convinced.
I still think these changes would be good:
- The docs should say that a
0 return value always indicates an error. (Where 0 is a correct return value, we should e.g. fudge it and return 1, losing a nanosecond of accuracy.)
- The docs should say that the value is clamped on overflow
- Add a comment to the startup check to explicitly mark it as an implementation detail. If and when we port to a platform that doesn't provide the necessary clocks, we should be free to change CPython so that only some parts of it don't work, and remove the check. If third-party code wants the check, it should do it itself.
- Allow the functions to be called without an initialized runtime. If the implementation needs the runtime, it can check and fail (return 0). (I you don't agree I can argue more about the benefits of merging the ”needs GIL” and “needs runtime” prerequisites. But maybe you'd be willing to humour me here.)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There are three functions in two fileutils.h files
Oh I forgot that most fileutils.h functions are private. It's a good thing 😁
<time.h> is a reasonable fallback
You need more includes to access all functions.
#include "Python.h"
#include <time.h> // gmtime_r()
#ifdef HAVE_SYS_TIME_H
# include <sys/time.h> // gettimeofday()
#endif
#ifdef MS_WINDOWS
# include <winsock2.h> // struct timeval
#endif
#if defined(__APPLE__)
# include <mach/mach_time.h> // mach_absolute_time(), mach_timebase_info()
#endif
The basic gettimeofday() is not provided by time.h.
time() is fine if you don't need better resolution than 1 second. Here we are talking about nanoseconds. See PEP 564 and PEP 418 for the rationale.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|