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

Implement work-stealing scheduler #862

Merged
merged 3 commits into from
Jan 11, 2023
Merged

Conversation

amezin
Copy link
Collaborator

@amezin amezin commented Jan 5, 2023

Closes #858

@nicoddemus
Copy link
Member

Thanks @amezin!

I will review it in the next few days.

You have made several contributions already, would you like to help maintain the project and become a contributor?

@amezin
Copy link
Collaborator Author

amezin commented Jan 6, 2023

What does "becoming a contributor" mean? GitHub already shows the "Contributor" badge...

@nicoddemus
Copy link
Member

Basically write access, being able to review/merge PRs, and making releases.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

This looks really good!

Left a few minor comments which would be nice to get in as well.

Did you test this new mode in a large test suite?

src/xdist/dsession.py Show resolved Hide resolved
changelog/858.feature Show resolved Hide resolved
src/xdist/remote.py Outdated Show resolved Hide resolved
src/xdist/remote.py Show resolved Hide resolved
src/xdist/scheduler/worksteal.py Show resolved Hide resolved
src/xdist/scheduler/worksteal.py Show resolved Hide resolved
src/xdist/scheduler/worksteal.py Show resolved Hide resolved
@nicoddemus
Copy link
Member

Feel free to use type-annotations as well, as we run mypy on the repository. 👍

amezin added 3 commits January 8, 2023 05:39
This will be necessary later for 'steal' command.

Luckily, all tests still pass after this change.
@amezin
Copy link
Collaborator Author

amezin commented Jan 8, 2023

Did you test this new mode in a large test suite?

https://github.com/ddterm/gnome-shell-extension-ddterm/actions/runs/3843994508/jobs/6546739410

Speed/time seems to be indistinguishable from load - either when the suite is split to multiple jobs, or when I run all tests at once (~4k tests total). On GitHub CI, run to run variance is higher than load vs worksteal difference. When I run the same suite locally, I get maybe 5% speedup compared to load, but nothing really noticeable.

@amezin
Copy link
Collaborator Author

amezin commented Jan 8, 2023

Basically write access, being able to review/merge PRs, and making releases.

Yes, I'm interested. But can't promise I'll be consistently active.

@nicoddemus
Copy link
Member

Yes, I'm interested. But can't promise I'll be consistently active.

Sure, no problem, we have no such expectations. 👍

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks @amezin!

Will merge this in the next few days to give the chance for others to review too. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement work-stealing scheduler
2 participants