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

Improve concurrent task runner performance #6948

Merged
merged 3 commits into from
Sep 26, 2022
Merged

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Sep 23, 2022

Improves performance of the concurrent task runner by using events instead of spinlocks to wait for results where feasible. I believe this only doesn't apply to use of async tasks from sync flows. If performance is a major concern in that case, users should be using sync or async consistently. This follows a pattern established in PrefectFuture.wait.

Closes #6468
Supersedes #6239

Example

In #6468 the concurrent task runner was demonstrated to be 7233% slower when performing synchronous IO. Replicated here, the concurrent task runner is 16% faster when performing synchronous IO. I did not attempt to reproduce the original slow download as my internet is not as fast as theirs and their example would take a theoretical 3 hours to download on my machine.

I failed to demonstrate an improvement in total flow runtime with a benchmark using CPU intensive tasks derived from #6239.

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • This pull request includes a label categorizing the change e.g. fix, feature, enhancement

@zanieb zanieb added the enhancement An improvement of an existing feature label Sep 23, 2022
@zanieb zanieb requested a review from cicdw as a code owner September 23, 2022 18:17
@netlify
Copy link

netlify bot commented Sep 23, 2022

Deploy Preview for prefect-orion ready!

Name Link
🔨 Latest commit 24af9a2
🔍 Latest deploy log https://app.netlify.com/sites/prefect-orion/deploys/6331d13545d8f2000810dedf
😎 Deploy Preview https://deploy-preview-6948--prefect-orion.netlify.app/api-ref/prefect/task-runners
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@zanieb zanieb requested review from bunchesofdonald and removed request for cicdw September 23, 2022 18:17
Copy link
Contributor

@bunchesofdonald bunchesofdonald left a comment

Choose a reason for hiding this comment

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

I like it!

src/prefect/task_runners.py Outdated Show resolved Hide resolved
Co-authored-by: Chris Pickett <chris.pickett@prefect.io>
@zanieb zanieb merged commit 245b807 into main Sep 26, 2022
@zanieb zanieb deleted the task-runner-performance branch September 26, 2022 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance difference between Sequential and ConcurrentTaskRunner for tasks with synchronous network IO
2 participants