Skip to content
New issue

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

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

Already on GitHub? # to your account

gh-102024: Reduced _idle_semaphore.release calls #102025

Merged
merged 5 commits into from
May 26, 2023
Merged

gh-102024: Reduced _idle_semaphore.release calls #102025

merged 5 commits into from
May 26, 2023

Conversation

jackcvr
Copy link
Contributor

@jackcvr jackcvr commented Feb 18, 2023

_idle_semaphore.release() is called if only work_queue is empty

Benchmarks:
submit_only:

./python -m timeit -n 100000 -r 10 -s 'W = 1; from concurrent.futures import ThreadPoolExecutor; tpe = ThreadPoolExecutor(W)' 'tpe.submit(lambda: None)'

before:
W=1: 100000 loops, best of 10: 17.6 usec per loop
W=2: 100000 loops, best of 10: 10.5 usec per loop
W=4: 100000 loops, best of 10: 10.5 usec per loop
after:
W=1: 100000 loops, best of 10: 7.42 usec per loop
W=2: 100000 loops, best of 10: 7.5 usec per loop
W=4: 100000 loops, best of 10: 7.5 usec per loop

submit_then_wait:

./python -m timeit -n 10 -r 10 -s 'W = 1; from concurrent.futures import ThreadPoolExecutor, wait; tpe = ThreadPoolExecutor(W)' 'fs=[tpe.submit(lambda: None) for _ in range(10_000)]; wait(fs)'

before:
W=1: 10 loops, best of 10: 110 msec per loop
W=2: 10 loops, best of 10: 100 msec per loop
W=4: 10 loops, best of 10: 114 msec per loop
after:
W=1: 10 loops, best of 10: 85.2 msec per loop
W=2: 10 loops, best of 10: 83.9 msec per loop
W=4: 10 loops, best of 10: 86.5 msec per loop

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Feb 18, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Feb 18, 2023
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@sobolevn
Copy link
Member

Thanks! One more important thing is missing: benchmarks.
Can you please provide some small code sample that will show us that your solution is actually faster now?

@arhadthedev
Copy link
Member

arhadthedev commented Feb 19, 2023

@sobolevn BTW is there any newcomer tutorial (edit: how-to) dedicated to pyperf one-liners? https://pyperf.readthedocs.io/en/latest/index.html seems to not serve this purpose.

@jackcvr
Copy link
Contributor Author

jackcvr commented Feb 19, 2023

Added some benchmarks. In my tests there is noticeable boost for submits to one thread.

@sobolevn
Copy link
Member

sobolevn commented Feb 19, 2023

@arhadthedev I don't know :(
I never needed one, since I've used pyperf before and quite a lot.

jackcvr and others added 2 commits February 19, 2023 23:20
_idle_semaphore.release() is called if only work_queue is empty
Copy link
Contributor

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good to me. I have this exact same change in some custom thread pool code I use :-)

I'm a little confused by your benchmarking. I'm not sure why you'd expect the only submit case to speed up (and I can't repro a speed up on that case locally), without shutting down the executor. Of course, if you add the shutdown, I can measure a win (though ofc the size of win varies based on task count, task size, thread count).

Lib/concurrent/futures/thread.py Outdated Show resolved Hide resolved
@hauntsaninja hauntsaninja requested a review from gpshead May 1, 2023 00:05
@hauntsaninja hauntsaninja added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 24, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @hauntsaninja for commit afbdad6 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 24, 2023
@gpshead gpshead added the needs backport to 3.12 bug and security fixes label May 26, 2023
@gpshead gpshead merged commit 0242e9a into python:main May 26, 2023
@miss-islington
Copy link
Contributor

Thanks @jackcvr for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 26, 2023
Reduced _idle_semaphore.release calls in concurrent.futures.thread._worker
_idle_semaphore.release() is now only called if only work_queue is empty.

---------

(cherry picked from commit 0242e9a)

Co-authored-by: Andrii Kuzmin <jack.cvr@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
@bedevere-bot
Copy link

GH-104959 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label May 26, 2023
gpshead pushed a commit that referenced this pull request May 26, 2023
…104959)

gh-102024: Reduced _idle_semaphore.release calls (GH-102025)

Reduced _idle_semaphore.release calls in concurrent.futures.thread._worker
_idle_semaphore.release() is now only called if only work_queue is empty.

---------

(cherry picked from commit 0242e9a)

Co-authored-by: Andrii Kuzmin <jack.cvr@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
stdlib Python modules in the Lib dir
Projects
Development

Successfully merging this pull request may close these issues.

7 participants