pocoproject / poco

The POCO C++ Libraries are powerful cross-platform C++ libraries for building network- and internet-based applications that run on desktop, server, mobile, IoT, and embedded systems.
https://pocoproject.org
Other
8.04k stars 2.11k forks source link

Motivated by "The load balancing issue in Poco::ActiveThreadPool" #4544 #4548

Open bas524 opened 1 month ago

bas524 commented 1 month ago

Optimization allows redistribute tasks to the idle threads When ActiveThread dequeue next Runnable, it checks is optimization is true and if yes it try to find idle thread and resend task By default optimization is false

matejk commented 1 month ago

Thanks for the contribution.

IMO partial refactoring of ActiveThreadPool (and related classes) to have a single task queue in the pool where the threads take the work from would automatically create optimal load-balancing and simplify the implementation at the same time.

bas524 commented 1 month ago

Thanks for the contribution.

IMO partial refactoring of ActiveThreadPool (and related classes) to have a single task queue in the pool where the threads take the work from would automatically create optimal load-balancing and simplify the implementation at the same time.

Hi! I thougt about it. If notificationqueue was lock-free than you proposition shoud be correct. In our case realisation with single queue would contains many locks and bad performance. May be flat combinig implementation is better choise for us...

andrewauclair commented 1 month ago

This change appears to fail with the Thread Sanitizer. The actual log has much more in it https://github.com/pocoproject/poco/actions/runs/8982526627/job/24670333088?pr=4548.

testActiveThreadLoadBalancing: ==================
WARNING: ThreadSanitizer: data race (pid=33153)
  Read of size 8 at 0x7ffe63aa0b60 by thread T16:
    #0 Poco::ActiveThread::run() <null> (libPocoFoundation.so.103+0xf9197)
    #1 <null> <null> (libPocoFoundation.so.103+0x1cc80e)
    #2 Poco::ThreadImpl::runnableEntry(void*) <null> (libPocoFoundation.so.103+0x1cdebd)