-
Notifications
You must be signed in to change notification settings - Fork 341
auto spawn new worker thread #744
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
7714038
to
b46c151
Compare
adding the capability for the users to set their own monitoring policy, so they can tunning the monitor to meet specific performance requirement. |
@win-t I would not want to expose an additional public module. |
yeah, @skade I think so, introducing public API was not my original proposal, let's do it step by step, I still have doubt about the action: scaling up the runtime or just abort the whole program that doubt drive me to think that the user should decide it by themself by exposing via new API, but I don't think that is also a good idea, |
I decide to just scale up the runtime, if the users want to abort, they can create their own monitor thread, because that doesn't need access to internal runtime now we need to come up with the right value of probing interval that everyone happy with |
fn schedule(&self, rt: &Runtime, task: Runnable) { | ||
self.processor.lock().schedule(rt, task); | ||
fn schedule(&self, rt: &Runtime, task: Task) { | ||
if !self.drained.load(Ordering::SeqCst /* ??? */) { |
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.
this is hot path, and I don't fully understand what SeqCst mean, maybe other values give better performance
Just hold this PR for now until #748 is resolved |
Hi guys, this is my proposal for solving #740
what changes I made:
RwLock
because we need to able to modify it laterRuntime::new
I combined it with the creation of the thread intoRuntime::start_new_thread
Runtime::start_new_thread
btw, I need insight on atomic ordering, I don't understand it well, I don't think SeqCst is needed
I don't know how to test the performance overhead of this solution, so I need advice here