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

Add --threads argument to pyright cli #12688

Merged
merged 1 commit into from
Sep 23, 2024
Merged

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Sep 23, 2024

@srittau srittau merged commit f0e16b8 into python:main Sep 23, 2024
66 of 67 checks passed
@AlexWaygood
Copy link
Member

Does this actually speed things up much?

So for our purposes, this argument might slow things down?

@Avasam
Copy link
Collaborator Author

Avasam commented Sep 23, 2024

Does this actually speed things up much?

About a second locally (17.45s vs 18.81s average) running python .\tests\pyright_test.py on a Ryzen 7 5800X. Wanted to see if it could help the CI too, but it looks like it might not be the case.

Even if within the same GitHub action, there's 1-2s variance between jobs, Run pyright with basic settings on all the stubs is consistently over 10s longer on the second action.

I think we can revert, including the non-CI part. I'm not gonna fight for a ~1s improvement (which is whithin some run to run variance) on a 16 logical core CPU when it might make others' run slower ^^".
Dunno if Eric would be interested in seeing these numbers or if that's just expected for GitHub actions.

@AlexWaygood
Copy link
Member

Dunno if Eric would be interested in seeing these numbers or if that's just expected for GitHub actions.

Without having looked into it at all: it might well be the kind of thing where a huge codebase with very large packages in it would get a speedup from --threads, but a repo of our size does not

@JelleZijlstra
Copy link
Member

We already get a ton of parallelism from running so many different GH Actions; I think it makes sense that we don't get much gain from further parallelizing within each action. It might help more when run locally but the speedup @Avasam posted doesn't seem that compelling.

AlexWaygood added a commit to AlexWaygood/typeshed that referenced this pull request Sep 23, 2024
This reverts commit f0e16b8.

See conversation in python#12688: for this repo, it seems like the argument actually makes pyright run slower, rather than faster.
@AlexWaygood
Copy link
Member

I opened #12690 to revert

@Avasam Avasam deleted the pyright--threads branch September 24, 2024 15:41
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.

4 participants