make start_soon always start soon - support eager tasks #851
Conversation
|
does this need documenting as a new feature? |
|
I weep at the complexity. Is all this absolutely necessary? |
|
Hopefully we get a This took a few rounds to squash down as much complexity as possible. It used to be much worse - so I'm biased to be somewhat happy with it |
|
This probably needs a test with a fresh instance of create_eager_task_factory so it goes through the eagerness measurement process |
| def create_eager_task_factory( | ||
| custom_task_constructor: Callable[..., Any], | ||
| ) -> Callable[..., Any]: | ||
| def factory( | ||
| loop: Any, | ||
| coro: Any, | ||
| *, | ||
| name: str | None = None, | ||
| context: contextvars.Context | None = None, | ||
| ) -> Any: | ||
| return custom_task_constructor( | ||
| coro, loop=loop, name=name, context=context, eager_start=True | ||
| ) | ||
|
|
||
| return factory |
There was a problem hiding this comment.
Just what is the point of this?
There was a problem hiding this comment.
This is to make a task factory that's eager, but not the same as the built in eager task factory, so it can test the slow path that measures task factories for eagerness
| pytest.param( | ||
| ( | ||
| "asyncio", | ||
| { | ||
| "debug": True, | ||
| "loop_factory": task_factory_loop_factory_factory( | ||
| asyncio.create_eager_task_factory(asyncio.Task) | ||
| ), | ||
| }, | ||
| ), | ||
| id="asyncio+stdlib-custom-eager", | ||
| ), |
There was a problem hiding this comment.
Isn't this covered by the other asyncio_params already?
There was a problem hiding this comment.
No this is different because it uses a totally custom eager task factory that takes the slow path in is_eager
|
I pushed a simplification that gets rid of the two counts of pending_eager_tasks, and collects any errors from the eager task spawner |
| if getattr(tf, "__code__", object()) is _TF_CODE: | ||
| _TF_IS_EAGER_MAP[tf] = True | ||
| return True |
There was a problem hiding this comment.
Why is it necessary to special case asyncio.eager_task_factory?
There was a problem hiding this comment.
To avoid having to create a task and measure it in the default case
| pass | ||
|
|
||
|
|
||
| if sys.version_info >= (3, 12): |
There was a problem hiding this comment.
Wouldn't it be possible to create an eager task factory on earlier Python versions?
There was a problem hiding this comment.
It's very difficult to get current_task to work without the 3.12 swap_task code
| nonlocal ran | ||
| ran = True | ||
|
|
||
| tf(_ShimEventLoop(), corofn()).cancel() |
There was a problem hiding this comment.
What do we need the shim event loop for really?
There was a problem hiding this comment.
To avoid creating a task that gets destroyed when pending
There was a problem hiding this comment.
I think I could refactor this to use a task in the TaskGroup
|
Closing in favor of #853. |
Changes
Fixes #.
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/) added which would fail without your patchdocs/, in case of behavior changes or newfeatures)
docs/versionhistory.rst).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**as the version.If, say, your patch fixes issue #123, the entry should look like this:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.