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

Getting rid of the autojump task #1587

Closed
njsmith opened this issue Jun 9, 2020 · 0 comments · Fixed by #1588
Closed

Getting rid of the autojump task #1587

njsmith opened this issue Jun 9, 2020 · 0 comments · Fixed by #1588

Comments

@njsmith
Copy link
Member

njsmith commented Jun 9, 2020

Right now the autojump clock works internally by spawning a system task that loops on wait_all_tasks_blocked.

Doing this using only existing public APIs is a neat trick, but it has some downsides:

So we should probably make the autojump functionality something that doesn't rely on a task and is a bit more closely integrated with the run loop.

How should this work?

In general when implementing a feature like the MockClock, our first impulse is to try to do it using public APIs, because we want to leave the door open for our users to implement new similar features we haven't thought of and without asking us for permission.

In the case of MockClock, it's not really clear how much demand there is to write custom autojump functionality, or what it would need... so it's not clear what a "generic" API for this would look like.

I thought about maybe replacing the core support for wait_all_tasks_blocked with a mechanism for registering a callback to be invoked after all tasks were blocked, and then using that to implement both wait_all_tasks_blocked and MockClock. The appealing thing about this is that it would mean the runloop only has to deal with one concept of "things are idle", and not deal with the different variants of that. But it's not quite as simple as just a add_idle_callback(cushion, callback) type API, because wait_all_tasks_blocked needs to be able to cancel a wakeup, MockClock needs to adjust the autojump threshold...

I think the simplest code change would be to straight-up add an autojump_threshold attribute to Runner, where if it gets exceeded we call current_clock()._autojump(). This is so simple that it feels like the right implementation to me. The question is how to expose it.

One option would be to have a trio.lowlevel.set_clock_autojump_threshold, or something. Or put the autojump_threshold on the clock object itself, and document that as a public part of the trio.abc.Clock interface. Given that this is such a specialized feature and we have no idea how anyone else would use this though, trying to make a public API feels premature.

The other option would be to keep it a private API, that only MockClock can use. If we do this then we should also move MockClock into _core for consistency. I'm not super happy with the idea of offering this magic clock that no-one else can replicate, but I guess I'm even less happy with the alternatives, and it's not hard to add a public API later if anyone needs it. So I guess let's do that.

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

Successfully merging a pull request may close this issue.

2 participants