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

Maybe use a more efficient wakeup mechanism #1554

Open
oremanj opened this issue May 26, 2020 · 3 comments
Open

Maybe use a more efficient wakeup mechanism #1554

oremanj opened this issue May 26, 2020 · 3 comments

Comments

@oremanj
Copy link
Member

oremanj commented May 26, 2020

#1551 had some discussion of I/O wakeup mechanisms. For signal delivery, AFAICT we don't have any option than the current WakeupSocketpair without interpreter changes, and signals aren't delivered often enough for it to matter anyway. For cross-thread wakeups and the new guest mode wakeups, though, each of our I/O backends has a more efficient way to do a wakeup, and maybe we should use it.

  • epoll: epoll is Linux-specific, so we can use the Linux-specific eventfd interface. The man page specifies:

    Applications can use an eventfd file descriptor instead of a pipe (see pipe(2)) in all cases where a pipe is used simply to signal events. The kernel overhead of an eventfd file descriptor is much lower than that of a pipe, and only one file descriptor is required (versus the two required for a pipe).

  • kqueue: there is an EVFILT_USER for things like this; it's only documented on FreeBSD but it appears to work the same on OS X.
  • Windows: post a completion to the IOCP directly, like "Guest mode", for cohabitation with Qt etc. #1551 does
@njsmith
Copy link
Member

njsmith commented May 27, 2020

Right, we have to allocate a socketpair for signals.

For TrioToken.run_sync_soon, socketpair is definitely convenient, because it lets the handler task be a regular task using our regular socket API.

For guest mode wakeups in #1551, I did use PostQueuedCompletionStatus, but that was just because it was the least amount of code :-). (If I'd used a wakeup socket, I'd have to manage its lifetime, drain it, keep resubmitting new AFD_POLL requests, etc.)

I guess if EVFILT_USER works on macOS, then that would be less code as well.

eventfd is more of a hassle, because we'd need to use ctypes to get at it. It's certainly possible, just more of a hassle.

For all of these, wiring them up to TrioToken would also add complexity (await io_manager.wait_task_forced_awake()? what do you do if a forced wakeup happens at a moment when the TrioToken task isn't watching for some reason? etc.)

I agree that cross-thread wakeups and guest mode wakeups are somewhat performance sensitive. But it's not at all obvious to me that these other options actually have a performance advantage over socketpairs :-). So I guess I'd want to see that before making the code any more complicated.

@njsmith
Copy link
Member

njsmith commented May 27, 2020

For all of these, wiring them up to TrioToken would also add complexity (await io_manager.wait_task_forced_awake()? what do you do if a forced wakeup happens at a moment when the TrioToken task isn't watching for some reason? etc.)

Well, hmm, I guess it wouldn't be too hard if we stopped having a TrioToken task, and replaced it with the callback version: every time an IO manager sees a forced wakeup, call run_all_bounded to process the queue.

But we'd still need to:

  • figure out the delicate shutdown dance to guarantee that all jobs are processed or refused.
  • figure out how to inject exceptions from the io manager into the task tree, since right now we use spawn_system_task but I'm not 100% sure if spawn_system_task is always possible at all the times the io manager runs (though maybe we could reuse the machinery needed to inject KeyboardInterrupt in Rethink KeyboardInterrupt protection APIs #733? and that would probably be a good idea anyway, as part of Make the TrioInternalError experience less confusing #1056)
  • have a task to process the wakeup socketpair

@njsmith
Copy link
Member

njsmith commented May 27, 2020

Though I guess using a task to process the wakeup socketpair is technically a bit dubious, exactly because there's no guarantee that it runs for the entire lifetime of our program. (For example, consider a user-created system task that does with CancelScope(shield=True): await trio.sleep(...), for some perverse reason.) During shutdown, right now we cancel all system tasks simultaneously, so a lingering task like this could put us in a situation where the signal wakeup task is gone, trio goes to sleep, and can't be woken...

The autojump clock also uses a system task; that's another one where you kind of need it to keep running until the very end. But over here I think I argued myself into thinking that it needs to stop using a system task anyway, exactly because it's too low-level a concept to rely on tasks.

One option to mitigate some of these issues would be to have two levels of system tasks: system-system tasks, that only Trio makes, and user-system tasks, and make the rule be that during shutdown it goes: main exits → user-system tasks are cancelled → system-system tasks are cancelled.

This line of thought also led to this question: #1521 (comment)

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

No branches or pull requests

2 participants