Signal refactor #2835
Merged
Signal refactor #2835
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Merge the runtime::{io, time} modules into runtime::driver
* Create a concret Driver type which encompases the conditionally
compiled `io` and `time` drivers
* This avoids having other code directly refer to
`runtime::time::Driver` etc. so that other drivers can be added
transparently
* This offers everal benefits: - fewer spurious wakeups: tasks blocked on a `Signal` instance will only get woken up when their respective signal is received rather than on *any* signal - using fewer file descriptors: we now need one file descriptor *per driver/runtime* rather than one per `Signal` instance for receiving wakeups
* This avoids lazy registration and synchronization during every turn
ipetkov
added
A-tokio
|
FWIW I did run the benchmarks before and after the changes on my under-powered machine, but the results would vary wildly in between consecutive runs so I did not bother publishing them here, but happy to if someone would like to see them. |
|
Thanks for getting this done. I did a quick skim, but this is going to take more time to go over closer :) I will look asap. cc/ @seanmonstar this relates to your I/O driver work as well. |
carllerche
approved these changes
Sep 22, 2020
seanmonstar
approved these changes
Sep 22, 2020
|
Thanks for the reviews! Just wanted to mention that @carllerche and I discussed this in the discord and we decided to omit the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactors how Unix signals are dispatched:
SignallistenersSignalinstance will only get woken up when their respective signal is received, rather than always being woken up whenever any signal is receivedSignalinstanceThere is an open question regarding how a runtime should be configured given that we're introducing the concept of dependent drivers, in this case
signaldepending onio(the dependency was alway there implicitly). For example, using theprocessmodule on Unix systems requires thesignalfeature, and theiofeature transitively, but on Windows neither of the modules are required. Should we require that a runtime is explicitly built with all required features, or should turning on a feature transitively enable dependent features?process->signal->iodependency), and subsequent internal refactors could lead to older code relying on more features than they need (e.g.processstops requiringsignal)