Skip to content

gh-110850: Add PyTime_TimeUnchecked() function #115973

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

Closed
wants to merge 3 commits into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Feb 26, 2024

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/

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.
@vstinner
Copy link
Member Author

See discussion capi-workgroup/decisions#8 about this API.

@encukou
Copy link
Member

encukou commented Feb 28, 2024

OK, I'll repeat my questions here, too.

  • Do we guarantee that you can call these functions before Python is initialized & after it's finalized?

  • What concrete use cases / user needs does this solve that are not solved by <time.h> functions?

@vstinner
Copy link
Member Author

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.

@encukou
Copy link
Member

encukou commented Feb 29, 2024

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.

@vstinner
Copy link
Member Author

vstinner commented Feb 29, 2024

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.

@vstinner
Copy link
Member Author

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.

@vstinner
Copy link
Member Author

@scoder: You may reply to the Cython question.

@scoder
Copy link
Contributor

scoder commented Feb 29, 2024

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.

@encukou
Copy link
Member

encukou commented Mar 1, 2024

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.)

@vstinner
Copy link
Member Author

vstinner commented Mar 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants