Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-110850: Add PyTime_t C API#115215

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
encukou merged 21 commits intopython:mainfromencukou:fallible-pytime
Feb 12, 2024
Merged

Conversation

encukou
Copy link
Member

@encukouencukou commentedFeb 9, 2024
edited by github-actionsbot
Loading

This builds on Victor's PR:#112135

Additionally:

  • The API of the public functions now allows them to raise exceptions
  • The units ofPyTime_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/

vstinnerand others added10 commitsFebruary 8, 2024 16:43
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`.
Copy link
Member

@vstinnervstinner left a 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 intime.rst.

@vstinner
Copy link
Member

We should either:

  • Stick to 64-bit and nanoseconds.
  • Design the API so that the type can become 128-bit integer and use better resolution than nanoseconds. In this case, I suggest to add 2 functions: PyTime_AsNanoseconds() and PyTime_FromNanoseconds().

@vstinner
Copy link
Member

@encukou: Thanks for working on this PR!

@rhettingerrhettinger removed their request for reviewFebruary 9, 2024 22:53
Copy link
Contributor

@colesburycolesbury left a 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 ofPyTime_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 thatPyTime_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 haveCLOCK_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.

@colesbury
Copy link
Contributor

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.

@vstinner
Copy link
Member

@colesbury:

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 thepublic C API.If Python needs variants which cannot fail, we can still have some in ourinternal C API.

Right, the rationale for reporting errors was discussed in length in the issue:capi-workgroup/decisions#8.

Copy link
Member

@vstinnervstinner left a 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 withoutPyTime_t type: usingint64_t type. It's tempting to make the API as small as possible. IMO the API isbetter with a decided type, even if it's well documented that it's exactlyint64_t. In terms of typing, it's better, it makes the code easier to follow, to review, etc.

encukou reacted with thumbs up emoji
@encukou
Copy link
MemberAuthor

Yes: unlikeint64_t,PyTime_t is always in nanoseconds.

vstinner reacted with thumbs up emoji

@encukou
Copy link
MemberAuthor

C API WG is in favour, so I'll merge.

vstinner reacted with hooray emojivstinner reacted with heart emoji

@encukouencukou merged commit879f454 intopython:mainFeb 12, 2024
@encukouencukou deleted the fallible-pytime branchFebruary 12, 2024 17:13
@encukou
Copy link
MemberAuthor

Thank you for the code & reviews!

@vstinner
Copy link
Member

@encukou and me added a bare minimum public C API to read time with aPyTime_t type (64-bit signed number: nanoseconds): change commit879f454.

typedefint64_tPyTime_t;#definePyTime_MIN INT64_MIN#definePyTime_MAX INT64_MAXPyAPI_FUNC(double)PyTime_AsSecondsDouble(PyTime_tt);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.

fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this pull requestFeb 14, 2024
*pythongh-110850: Add PyTime_t C APIAdd 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>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@vstinnervstinnervstinner approved these changes

@colesburycolesburycolesbury left review comments

@pgansslepganssleAwaiting requested review from pgansslepganssle is a code owner

@abalkinabalkinAwaiting requested review from abalkinabalkin is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@encukou@vstinner@colesbury@Eclips4

[8]ページ先頭

©2009-2025 Movatter.jp