Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh(Poco::ActiveThreadPool): make it easy to use correctly #4624

Merged
merged 26 commits into from
Aug 30, 2024
Merged

enh(Poco::ActiveThreadPool): make it easy to use correctly #4624

merged 26 commits into from
Aug 30, 2024

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
Foundation/src/ActiveThreadPool.cpp Fixed Show fixed Hide fixed
Foundation/src/ActiveThreadPool.cpp Fixed Show fixed Hide fixed
@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?

Foundation/src/ActiveThreadPool.cpp Fixed Show resolved Hide resolved
Foundation/src/ActiveThreadPool.cpp Fixed Show resolved Hide resolved
@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