-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
gh-127859: Fixed documentation for call_later and call_at regarding early wakeup. #137859
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The news looks good to me, but maybe we can merge the two notes into one or better yet remove the note tags altogether. But I’m not sure about this either ways it looks all right
I thought about it. When I've put it at the end of the section it wasn't visible enough. The duplication here helps readability. The note is a little too visible on one hand, but it is an "unrelated note" on the other hand, which is why I sectioned it like so. |
@@ -304,6 +304,12 @@ clocks to track time. | |||
custom :class:`contextvars.Context` for the *callback* to run in. | |||
The current context is used when no *context* is provided. | |||
|
|||
.. note:: | |||
|
|||
For performance, callbacks scheduled with :meth:`loop.call_later` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this isn't the case for asyncio, it is uvloop which incorrectly calls the callback early right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is according to asveltov.
By adding that guarantee, we will also require it from different implementations, whether different Python or different drop-in replacement packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will also require it from different implementations, whether different Python or different drop-in replacement packages.
Yes, for an API like call_later
it is fine if the callback gets delayed one extra cycle because of floating point/ timer resolution error but it should definitely not call it earlier.
This is ensured in asyncio by end_time = self.time() + self._clock_resolution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear uvloop is at fault here and that should be fixed but we should document the correct behavior here not what happens with uvloop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So yeah, the documentation says exactly that - they may run up to one clock-resolution early, which is both the asyncio and expected behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for an API like
call_later
it is fine if the callback gets delayed one extra cycle because of floating point/ timer resolution error but it should definitely not call it earlier.This is ensured in asyncio by
end_time = self.time() + self._clock_resolution
I think you got confused a little - asyncio does call it earlier - up to one clock resolution. The end_time = self.time() + self._clock_resolution
is what asyncio handles. Callbacks scheduled after the current self.time()
will still run as long as they are less than end_time
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, got it, I had misinterpreted the issue.
Anyways, the docs seems fine then, I would appreciate @asvetlov to have a look as well.
Misc/NEWS.d/next/Documentation/2025-08-16-15-36-30.gh-issue-127859.CdXB5S.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
📚 Documentation preview 📚: https://cpython-previews--137859.org.readthedocs.build/