Skip to content

Conversation

siren186
Copy link
Member

@siren186 siren186 commented Aug 2, 2024

  1. Fixed The load balancing issue in Poco::ActiveThreadPool #4544
  2. Fixed Poco::ActiveThreadPool _targetCompleted event never reset #4634
  3. Support dynamic manages and recycles individual Poco::Thread objects.
  4. Support priority queue tasks

@siren186 siren186 changed the title make Poco::ActiveThreadPool easy to use (#4544) Enhancement Poco::ActiveThreadPool, make it easy to use correctly (#4544) Aug 2, 2024
@siren186
Copy link
Member Author

siren186 commented Aug 8, 2024

@matejk @bas524 Can someone review this PR to see if there are still any issues?

@siren186 siren186 changed the title Enhancement Poco::ActiveThreadPool, make it easy to use correctly (#4544) enh(Poco::ActiveThreadPool) make it easy to use correctly Aug 13, 2024
@siren186 siren186 changed the title enh(Poco::ActiveThreadPool) make it easy to use correctly enh(Poco::ActiveThreadPool): make it easy to use correctly Aug 13, 2024
@aleks-f aleks-f requested a review from matejk August 17, 2024 13:40
@aleks-f aleks-f added this to the Release 1.14.0 milestone Aug 17, 2024
@matejk
Copy link
Contributor

matejk commented Aug 20, 2024

@siren186 , can you write a test that verifies the changes?

@matejk
Copy link
Contributor

matejk commented Aug 21, 2024

@siren186, Thank you for updating the code. Some compile and testrun actions fail.

@siren186 siren186 requested a review from matejk August 23, 2024 02:55
@matejk
Copy link
Contributor

matejk commented Aug 24, 2024

@siren186 , I found out that increasing the macOS target version is better than not using optional. Two changes are committed to a related branch in Poco repository.

Would you merge those two commits?

@siren186
Copy link
Member Author

@matejk Is this check action error caused by my PR ?
https://github.com/pocoproject/poco/actions/runs/10556219311/job/29241333775

@matejk
Copy link
Contributor

matejk commented Aug 26, 2024

I don't think so. I started the job again.

@matejk matejk merged commit 73df368 into pocoproject:main Aug 30, 2024
43 checks passed
@bas524
Copy link
Contributor

bas524 commented Oct 12, 2024

@siren186 , @matejk
Sorry for the long silence.
I've checked this changes.
In my PR #4548 you can see two tests:

  • FifoEventTest::testAsyncNotifyBenchmark
  • ActiveThreadPoolTest::testActiveThreadLoadBalancing

First test verifies that we can run more async events than threads in the pool. My test returns OK in booth realisations, but my realisation is 2 times faster
this PR

./Foundation-testrunner testAsyncNotifyBenchmark

testAsyncNotifyBenchmark: Total async events is 10000000
total wait time is 107072ms (avg notify-time per run is 107.072ms)

OK (1 tests)

my PR

./Foundation-testrunner testAsyncNotifyBenchmark

testAsyncNotifyBenchmark: Total async events is 10000000
total wait time is 58783ms (avg notify-time per run is 58.783ms)

OK (1 tests)

Second test verifies that long tasks and short tasks distributes between threads and we have a garantee that short task don't wait long task if theris a free thread. My realisation returns OK, but this realisation failed

this PR

time ./Foundation-testrunner testActiveThreadLoadBalancing

testActiveThreadLoadBalancing: FAILURE

!!!FAILURES!!!
Runs: 1   Failures: 1   Errors: 0

There was 1 failure:
 1: CppUnit::TestCaller<ActiveThreadPoolTest>.testActiveThreadLoadBalancing
    "lttPerTIDCount != 0"
    in "/Users/a.bychuk/coding/pet/poco/Foundation/testsuite/src/ActiveThreadPoolTest.cpp", line 130

./Foundation-testrunner testActiveThreadLoadBalancing  0.09s user 0.02s system 0% cpu 12.049 total

my PR

time ./Foundation-testrunner testActiveThreadLoadBalancing

testActiveThreadLoadBalancing:

OK (1 tests)

./Foundation-testrunner testActiveThreadLoadBalancing  0.08s user 0.02s system 0% cpu 21.696 total

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Poco::ActiveThreadPool _targetCompleted event never reset The load balancing issue in Poco::ActiveThreadPool
4 participants