NotificationsYou must be signed in to change notification settings
Fork32.1k
Star67.3k

gh-102024: Reduced _idle_semaphore.release calls#102025

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

gpshead merged 5 commits intopython:mainfromjackcvr:optimized-idle-semaphore-in-thread-pool-executor

May 26, 2023

Merged

gh-102024: Reduced _idle_semaphore.release calls#102025

gpshead merged 5 commits intopython:mainfromjackcvr:optimized-idle-semaphore-in-thread-pool-executor

May 26, 2023

Conversation

Copy link

Contributor

jackcvr commentedFeb 18, 2023•
edited
Loading

_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

Issue:Reduce _idle_semaphore calls in ThreadPoolExecutor #102024

bedevere-bot added the awaiting review label

Feb 18, 2023

bedevere-bot mentioned this pull request

Feb 18, 2023

Reduce _idle_semaphore calls in ThreadPoolExecutor#102024

Closed

Copy link

bedevere-bot commentedFeb 18, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

Copy link

ghost commentedFeb 18, 2023•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.

Copy link

bedevere-bot commentedFeb 18, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

arhadthedev added the stdlibPython modules in the Lib dir label

Feb 18, 2023

sobolevn reviewed

Feb 18, 2023

View reviewed changes

Lib/concurrent/futures/thread.py OutdatedShow resolvedHide resolved

Copy link

bedevere-bot commentedFeb 18, 2023

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

Copy link

Member

sobolevn commentedFeb 19, 2023

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?

Copy link

Member

arhadthedev commentedFeb 19, 2023•
edited
Loading

@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.

Copy link

ContributorAuthor

jackcvr commentedFeb 19, 2023•
edited
Loading

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

Copy link

Member

sobolevn commentedFeb 19, 2023•
edited
Loading

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

jackcvrand others added2 commits

February 19, 2023 23:20

gh-102024: Reduced _idle_semaphore.release calls

56949ea

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

📜🤖 Added by blurb_it.

09e1c79

hauntsaninja approved these changes

Feb 20, 2023

View reviewed changes

Copy link

Contributor

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 OutdatedShow resolvedHide resolved

bedevere-bot added awaiting merge and removed awaiting review labels

Feb 20, 2023

jackcvrand others added3 commits

February 20, 2023 12:22

Changed issue reference issue16284 ->GH-60488in Lib/concurrent/futu…

0f50fe5

…res/thread.pyCo-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>

Merge branch 'main' into optimized-idle-semaphore-in-thread-pool-exec…

b071747

…utor

Merge branch 'main' into optimized-idle-semaphore-in-thread-pool-exec…

afbdad6

…utor

hauntsaninja requested a review fromgpshead

May 1, 2023 00:05

hauntsaninja added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

May 24, 2023

Copy link

bedevere-bot commentedMay 24, 2023

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

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

bedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section label

May 24, 2023

gpshead approved these changes

May 26, 2023

View reviewed changes

gpshead added the needs backport to 3.12only security fixes label

May 26, 2023

gpshead merged commit0242e9a intopython:main

May 26, 2023

bedevere-bot removed the awaiting merge label

May 26, 2023

Copy link

Contributor

miss-islington commentedMay 26, 2023

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

pythongh-102024: Reduced _idle_semaphore.release calls (pythonGH-102025)

48d7818

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 commit0242e9a)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>

Copy link

bedevere-bot commentedMay 26, 2023

GH-104959 is a backport of this pull request to the3.12 branch.

bedevere-bot removed the needs backport to 3.12only security fixes label

May 26, 2023

gpshead pushed a commit that referenced this pull request

May 26, 2023

[3.12]gh-102024: Reduced _idle_semaphore.release calls (GH-102025) (#…

8010cef

…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 commit0242e9a)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>