Skip to content
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

Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public APIs #110850

Closed
ryancdotorg opened this issue Oct 14, 2023 · 28 comments
Labels
3.13 bugs and security fixes release-blocker topic-C-API type-feature A feature request or enhancement

Comments

@ryancdotorg ryancdotorg added the type-feature A feature request or enhancement label Oct 14, 2023
@hugovk hugovk changed the title Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public apis Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public APIs Oct 14, 2023
@hugovk
Copy link
Member

hugovk commented Oct 14, 2023

Please could you say something about your use case?

Why do you need these functions?

Have you tried alternative APIs to achieve the same result?

cc @vstinner

@ryancdotorg
Copy link
Author

ryancdotorg commented Oct 14, 2023

I have a C/C++ module that was using them which I wrote a couple years ago. It broke.

I actually dug into what I was doing, and it turns out it was basically just (error checking omitted):

_PyTime_t tp;
struct timeval tv;
get_timestamp(&tv, ...);
_PyTime_FromTimeval(&tp, &tv);
double ts = _PyTime_AsSecondsDouble(tp); 

and I was able to more sensibly replace it with

double tv_frac = tv.tv_usec / 1e6;
struct timeval tv;
get_timestamp(&tv, ...);
double tv_frac = tv.tv_usec / 1e6;
double ts = tv.tv_sec + tv_frac;

It seems I just didn't want to bother figuring out the safest way to do the conversion.

This can be closed.

@hugovk hugovk closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2023
@vstinner
Copy link
Member

vstinner commented Oct 14, 2023

I'm open to make some _Py_Time functions public, but apparently here, there wasn't a strong need for that. Maybe it can wait. Thanks for the report anyway.

@scoder
Copy link
Contributor

scoder commented Oct 30, 2023

We actually expose that API in Cython's cpython/time.pxd. Please move it back to where it was.

@scoder scoder reopened this Oct 30, 2023
@vstinner
Copy link
Member

We actually expose that API in Cython's cpython/time.pxd. Please move it back to where it was.

This file uses the following APIs:

  • _PyTime_t
  • _PyTime_GetSystemClock()
  • _PyTime_AsSecondsDouble()

@scoder
Copy link
Contributor

scoder commented Oct 30, 2023

This file uses the following APIs

Correct. It basically mimics the Python time module and reimplements a part of it in C.

I'd still recommend exposing the complete C-API. We are talking about a standard library module here, not a part of the core C-API, so it represents a natural API border already. We are not talking about internals here. Let's just keep that up and allow regular external use as before.

@vstinner
Copy link
Member

I designed _PyTime_t API for Python internal usage, but apparently some people discovered it and decided to use it since it's convenient.

_PyTime_t API was designed to be able to write easily "deadline = time + timeout", but later I started to add "clamp" variants in code which cannot report errors to the caller. Clamp the result in the range [_PyTime_MIN; _PyTime_MAX] is better than having an undefined behavior.

One issue is that _PyTime_t range is smaller than time_t range:

// The _PyTime_t API supports a resolution of 1 nanosecond. The _PyTime_t type
// is signed to support negative timestamps. The supported range is around
// [-292.3 years; +292.3 years]. Using the Unix epoch (January 1st, 1970), the
// supported date range is around [1677-09-21; 2262-04-11].

64-bit time_t (standard even on 32-bit platforms) is way larger than that. In practice, it should not be an issue. But the datetime module cannot use _PyTime_t everywhere for example, since datetime max year is 9999, not year 2262.

I tried to make the _PyTime_t unit an implementation details and enforce _PyTime_FromNanoseconds() or _PyTime_FromSeconds() usage.

About raised exception, ValueError is raised for Not-A-Number floating pointer number, and OverflowError on conversions (_PyTime_t, time_t, timeval, timespec, etc.).

@encukou
Copy link
Member

encukou commented Oct 31, 2023

Victor, do you have any intention of putting the removed API back?

@vstinner
Copy link
Member

vstinner commented Nov 1, 2023

Victor, do you have any intention of putting the removed API back?

My plan in this issue is to add public functions replacing removed _PyTime functions. I'm collecting data to see how the private API was used and which API should be added.

vstinner added a commit to vstinner/cpython that referenced this issue Nov 16, 2023
Add PyTime_t API:

* PyTime_t type.
* PyTime_MIN and PyTime_MAX constants.
* PyTime_AsSecondsDouble(), PyTime_GetMonotonicClock(),
  PyTime_GetPerfCounter() and PyTime_GetSystemClock() functions.
vstinner added a commit to vstinner/cpython that referenced this issue Nov 16, 2023
Add PyTime_t API:

* PyTime_t type.
* PyTime_MIN and PyTime_MAX constants.
* PyTime_AsSecondsDouble(), PyTime_GetMonotonicClock(),
  PyTime_GetPerfCounter() and PyTime_GetSystemClock() functions.
@vstinner
Copy link
Member

I wrote PR #112135 to add the public functions: PyTime_AsSecondsDouble(), PyTime_GetMonotonicClock(), PyTime_GetPerfCounter() and PyTime_GetSystemClock().

vstinner added a commit to vstinner/cpython that referenced this issue Nov 16, 2023
Add PyTime_t API:

* PyTime_t type.
* PyTime_MIN and PyTime_MAX constants.
* PyTime_AsSecondsDouble(), PyTime_GetMonotonicClock(),
  PyTime_GetPerfCounter() and PyTime_GetSystemClock() functions.
vstinner added a commit to vstinner/cpython that referenced this issue Dec 14, 2023
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.
vstinner added a commit to vstinner/cpython that referenced this issue Dec 14, 2023
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.
vstinner added a commit to vstinner/cpython that referenced this issue Dec 14, 2023
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.
@scoder scoder added release-blocker 3.13 bugs and security fixes labels Jan 9, 2024
@scoder
Copy link
Contributor

scoder commented Jan 9, 2024

Marking this as a release blocker for 3.13 since the API removal is a regression in 3.13.

vstinner added a commit to vstinner/cpython that referenced this issue Jan 18, 2024
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.
@vstinner
Copy link
Member

Marking this as a release blocker for 3.13 since the API removal is a regression in 3.13.

I reopened my PR #112135 and created a C API Working Group issue: capi-workgroup/decisions#8

diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Run command:

sed -i -e 's!\<_PyTime_t\>!PyTime_t!g' $(find -name "*.c" -o -name "*.h")
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
<pycore_time.h> include is no longer needed to get the PyTime_t type
in internal header files. This type is now provided by <Python.h>
include. Add <pycore_time.h> includes to C files instead.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Rename functions:

* _PyTime_GetSystemClock() => _PyTime_TimeUnchecked()
* _PyTime_GetPerfCounter() => _PyTime_PerfCounterUnchecked()
* _PyTime_GetMonotonicClock() => _PyTime_MonotonicUnchecked()
* _PyTime_GetSystemClockWithInfo() => _PyTime_TimeWithInfo()
* _PyTime_GetMonotonicClockWithInfo() => _PyTime_MonotonicWithInfo()
* _PyTime_GetMonotonicClockWithInfo() => _PyTime_MonotonicWithInfo()

Changes:

* Remove "typedef PyTime_t PyTime_t;" which was
  "typedef PyTime_t _PyTime_t;" before a previous rename.
* Update comments of "Unchecked" functions.
* Remove invalid PyTime_Time() comment.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Replace private _PyTime functions with public PyTime functions.

random_seed_time_pid() now reports errors to its caller.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…ython#115751)

Remove references to the old names _PyTime_MIN
and _PyTime_MAX, now that PyTime_MIN and
PyTime_MAX are public.

Replace also _PyTime_MIN with PyTime_MIN.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…#115753)

PyTime_t no longer uses an arbitrary unit, it's always a number of
nanoseconds (64-bit signed integer).

* Rename _PyTime_FromNanosecondsObject() to _PyTime_FromLong().
* Rename _PyTime_AsNanosecondsObject() to _PyTime_AsLong().
* Remove pytime_from_nanoseconds().
* Remove pytime_as_nanoseconds().
* Remove _PyTime_FromNanoseconds().
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Return 0 on success. Set an exception and return -1 on error.

Fix os.timerfd_settime(): properly report exceptions on
_PyTime_FromSecondsDouble() failure.

No longer export _PyTime_FromSecondsDouble().
vstinner added a commit to vstinner/cpython that referenced this issue Apr 29, 2024
Add "Raw" variant of PyTime functions:

* PyTime_MonotonicRaw()
* PyTime_PerfCounterRaw()
* PyTime_TimeRaw()

Changes:

* Add documentation and tests. Tests release the GIL while calling
  raw clock functions.
* py_get_system_clock() and py_get_monotonic_clock() now check that
  the GIL is hold by the caller if raise_exc is non-zero.
* Reimplement "Unchecked" functions with raw clock functions.
vstinner added a commit to vstinner/cpython that referenced this issue Apr 29, 2024
Add "Raw" variant of PyTime functions:

* PyTime_MonotonicRaw()
* PyTime_PerfCounterRaw()
* PyTime_TimeRaw()

Changes:

* Add documentation and tests. Tests release the GIL while calling
  raw clock functions.
* py_get_system_clock() and py_get_monotonic_clock() now check that
  the GIL is hold by the caller if raise_exc is non-zero.
* Reimplement "Unchecked" functions with raw clock functions.
vstinner added a commit that referenced this issue May 1, 2024
Add "Raw" variant of PyTime functions:

* PyTime_MonotonicRaw()
* PyTime_PerfCounterRaw()
* PyTime_TimeRaw()

Changes:

* Add documentation and tests. Tests release the GIL while calling
  raw clock functions.
* py_get_system_clock() and py_get_monotonic_clock() now check that
  the GIL is hold by the caller if raise_exc is non-zero.
* Reimplement "Unchecked" functions with raw clock functions.

Co-authored-by: Petr Viktorin <encukou@gmail.com>
@encukou
Copy link
Member

encukou commented May 2, 2024

Raw time API was added in #118394.

@encukou encukou closed this as completed May 2, 2024
vstinner added a commit to vstinner/cpython that referenced this issue May 3, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
vstinner added a commit to vstinner/cpython that referenced this issue May 3, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()
* thread_pthread.c can now uses _PyDeadline_Init() and
  _PyDeadline_Get() since the monotonic clock is now the perf
  counter.

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
vstinner added a commit to vstinner/cpython that referenced this issue May 3, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()
* thread_nt.h can now use _PyDeadline_Init() and
  _PyDeadline_Get() since the monotonic clock is now the perf
  counter.

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
vstinner added a commit to vstinner/cpython that referenced this issue May 3, 2024
Use _PyDeadline_Init() and _PyDeadline_Get() in
EnterNonRecursiveMutex() of thread_nt.h.

_PyDeadline_Get() uses the monotonic clock which is now the same as
the perf counter clock on all platforms. So this change does not
cause any behavior change. It just reuses existing helper functions.
vstinner added a commit that referenced this issue May 4, 2024
Use _PyDeadline_Init() and _PyDeadline_Get() in
EnterNonRecursiveMutex() of thread_nt.h.

_PyDeadline_Get() uses the monotonic clock which is now the same as
the perf counter clock on all platforms. So this change does not
cause any behavior change. It just reuses existing helper functions.
vstinner added a commit to vstinner/cpython that referenced this issue May 4, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
vstinner added a commit to vstinner/cpython that referenced this issue May 4, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
vstinner added a commit to vstinner/cpython that referenced this issue May 4, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
vstinner added a commit that referenced this issue May 5, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
SonicField pushed a commit to SonicField/cpython that referenced this issue May 8, 2024
Add "Raw" variant of PyTime functions:

* PyTime_MonotonicRaw()
* PyTime_PerfCounterRaw()
* PyTime_TimeRaw()

Changes:

* Add documentation and tests. Tests release the GIL while calling
  raw clock functions.
* py_get_system_clock() and py_get_monotonic_clock() now check that
  the GIL is hold by the caller if raise_exc is non-zero.
* Reimplement "Unchecked" functions with raw clock functions.

Co-authored-by: Petr Viktorin <encukou@gmail.com>
SonicField pushed a commit to SonicField/cpython that referenced this issue May 8, 2024
…ython#118556)

Use _PyDeadline_Init() and _PyDeadline_Get() in
EnterNonRecursiveMutex() of thread_nt.h.

_PyDeadline_Get() uses the monotonic clock which is now the same as
the perf counter clock on all platforms. So this change does not
cause any behavior change. It just reuses existing helper functions.
SonicField pushed a commit to SonicField/cpython that referenced this issue May 8, 2024
Use the new public Raw functions:

* _PyTime_PerfCounterUnchecked() with PyTime_PerfCounterRaw()
* _PyTime_TimeUnchecked() with PyTime_TimeRaw()
* _PyTime_MonotonicUnchecked() with PyTime_MonotonicRaw()

Remove internal functions:

* _PyTime_PerfCounterUnchecked()
* _PyTime_TimeUnchecked()
* _PyTime_MonotonicUnchecked()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes release-blocker topic-C-API type-feature A feature request or enhancement
Projects
Development

No branches or pull requests

6 participants