jonstewart
commented
2 years ago So <clocktime> here is overall elapsed "wall clock" time? Yikes!
I had a look at the thread pool. I'm a bit skeeved out by it, to be honest:
void sleep_or_yield()
{
if (sleep_duration)
std::this_thread::sleep_for(std::chrono::microseconds(sleep_duration));
else
std::this_thread::yield();
}
/**
* @brief A worker function to be assigned to each thread in the pool. Continuously pops tasks out of the queue and executes them, as long as the atomic variable running is set to true.
*/
void worker()
{
while (running)
{
std::function<void()> task;
if (!paused && pop_task(task))
{
task();
tasks_total--;
}
else
{
sleep_or_yield();
}
}
}So, I'm more familiar with a normal condition_variable-based threadpool, where the CV is used to notify threads when there's work to do. Instead, this implementation has the worker thread either yield or sleep, and then poll the task queue again. std::this_thread::yield()'s behavior is supposedly to yield to other threads in the process with the same priority. If that happens, then we can assume that the thread is suspended for a set period of time (perhaps more time than we want, i.e., work may show up but time-slicing keeps the thread from waking up, but not so long). However, bulk_extractor doesn't have so many more threads than the hardware supports, so it might not yield at all! In that case, this is a spin lock, and will generate a bunch of contention on the mutex--maybe making it harder to enqueue new tasks.
But it gets worse! If sleep_duration is nonzero, then the worker thread sleeps, not yields. What's the default value of sleep_duration? A whole second! See line 278 of thread_pool.hpp. Woof! And a search in the code base for sleep_duration doesn't turn up any usage of it outside of the thread pool, so this is probably what's happening.
However! Without profiling/timing these different occurrences, it's hard to know what's going on. We can reason about possible outcomes, but I don't feel like I have intuition about what's really happening. Is data being read fast enough?
One thing that you could do, and it would help regardless of which thread_pool you used, would be to change the task functor to work on a batch of sbufs, rather than have a task per sbuf. In a situation where you're CPU-bound and the queue is backed up (likely), this lets the threads work full throttle on sbufs and lessens contention on the mutex.
Since it's a one-line change, I'd also try setting sleep_duration = 0; somewhere and seeing whether that makes a difference.
--
Oh wait Did I misread the time duration on the sleep as 1000 milliseconds when it's actually 1000 microseconds? Hmm.
simsong
After testing on a 16-core machine, I have validated that BE1.6 appears to have better threading utilization (with roughly 100% cpu saturation) compared with 2.0beta2, which uses https://github.com/bshoshany/thread-pool.
Here are 1.6 runs on domexusers:
Here are 2.0beta2 runs:
What this isn't showing is the CPU performance over time. I visually saw the results on the system load, but was not able to capture it. See next issue.