Conditional variable instead of sleep_for

[yvoinov]

I'm trying to avoid some overhead when pool is idle or underload.

Pls, advice me - is this way correct? Testing shows less overall pool latency, but I'm in doubt - this should not be serialization point for scaling.

[codecov]

Codecov Report

Merging #23 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master     #23   +/-   ##
======================================
  Coverage    94.2%   94.2%           
======================================
  Files           5       5           
  Lines         138     138           
======================================
  Hits          130     130           
  Misses          8       8

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af95dd8...4be560f. Read the comment docs.

[KayEss]

I think you should do the notification after you have added the job to the queue otherwise the worker might try to pull the job too soon

[yvoinov]

Мммммммм, in theory may be. In practice....hardly. Also, how you imagine such implementation? I see only one way - additional local variable requires, so more assembler, slower execution... No, not so good idea.

[KayEss]

If you're going to worry about the cost of that it might be more interesting to look at the call to notify_one you have for every posted job. Compared to that cost saving the bool result is essentially free.

[yvoinov]

If you're going to worry about the cost of that it might be more interesting to look at the call to notify_one you have for every posted job. Compared to that cost saving the bool result is essentially free.

Take a look on my own fork of this library. Done long time ago. :) This PR is years old :)

Anyway, you can make you own fork, perform benchmark and compare. :)

[yvoinov]

I've just made some performance tests with variant you offered:

inline bool Worker<Task, Queue>::tryPost(Handler&& handler)
{
    {
        std::lock_guard<std::mutex> lock(m_conditional_mutex);
        m_ready.store(true, std::memory_order_relaxed);
    }
    bool ret = m_queue.push(std::forward<Handler>(handler));
    m_conditional_lock.notify_one();
    return ret;
}

There is no visible performance difference with original one. So... see no reason to make such change. Don't forget - compiler can change code order, and it is non-mandatory will execute as written.