-
Notifications
You must be signed in to change notification settings - Fork 340
fix(rt): bring back dynamic machines #748
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
Conversation
Even if we do not make use of the progress blocking, we do need to make use of the dynamic restarting of machines as far as I understand. Keeps the perf, while removing the regression from #747
|
||
// Sleep for a bit longer if the scheduler state hasn't changed in a while. | ||
if idle > 10 { | ||
delay = (delay * 2).min(10_000); |
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.
Is perhaps crossbeam utils BackOff a better fit here than rolling it by hand?
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 figured if it was good enough for @stjepang it'll be good enough for me for now ;)
@dignifiedquire I think it's good to start a thread when you need it and close it when it's no longer needed. |
can we cache the thread? make the thread is just park instead of terminating it, then when a new thread is needed just unpark it and assume this thread is a fresh one and make sure no resource leak to the new thread (reinitialize all TLS), something like this loop {
main_loop();
invalidate_tls();
if push_to_cache(thread::current()) {
thread::park();
} else {
break;
}
} |
I agree we should investigate parking, but for now this is the original logic and fixes the brokeness on master |
Even if we do not make use of the progress blocking, we do need to make use of the dynamic restarting of machines as far as I understand.
Keeps the perf, while removing the regression from #747