Make ProgressThread::remove_function less blocking
#265
Conversation
pentschev
changed the title
ProgressThread::remove_function non-blockingProgressThread::remove_function less blocking
|
@nirandaperera just to follow-up on our brief offline discussion, this is a way we could make the existing |
cpp/src/progress_thread.cpp
Outdated
| std::lock_guard<std::mutex> lock(mutex_); | ||
| for (auto& [id, function] : functions_) { | ||
| function(); | ||
| if (function.is_done) { | ||
| completed_functions.push_back(id); | ||
| completion_tracker_.mark_completed(id); | ||
| } | ||
| } |
There was a problem hiding this comment.
@pentschev do you think we could traverse the functions_ map without holding the lock? I think that is the bottleneck in the current impl. And suspect that's why @madsbk 's PR of adding a small sleep helped.
There was a problem hiding this comment.
Yes, that is in essence what you've implemented as well, done in 5956475 . (Again, not cleaned up for any resources that are now redundant)
There was a problem hiding this comment.
Let me go through this
|
@pentschev I think we are on the same page here. I also sent a PR #266 which I think is a little more simpler. LMK what you think |
Track completed functions to make
ProgressThread::remove_functionless blocking.