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

--threads=max should be default, except in CI #2795

Closed
muglug opened this issue Feb 12, 2020 · 5 comments
Closed

--threads=max should be default, except in CI #2795

muglug opened this issue Feb 12, 2020 · 5 comments

Comments

@muglug
Copy link
Collaborator

muglug commented Feb 12, 2020

Why?

Because faster Psalm is good.

@vimeo vimeo deleted a comment from psalm-github-bot bot Feb 12, 2020
@iluuu1994
Copy link
Contributor

Why not in CI? As long as the pcntl extension is enabled (which admittedly is probably not very often).

@muglug
Copy link
Collaborator Author

muglug commented Feb 12, 2020

Why not in CI?

Because I've encountered (anecdotally) instances of where threading doesn't perform optimally on resource-constrained boxes in CI – maybe the machine has the capability, but it's performing a ton of other tasks at the same time.

@iluuu1994
Copy link
Contributor

Ok, I was just interested.

@jarstelfox
Copy link
Contributor

jarstelfox commented Feb 13, 2020

@muglug, This is just an edge-case I noticed, but worth mentioning.

Back around psalm 3.7.2, we ran psalm with 4 threads. If a thread, hit an internal expectation from psalm, all the classes it would have analyzed were never seen by psalm. worse yet: Psalm report it passed when one of the threads had crashed.

We found it possible to make psalm errors (which were not reported) in the files that were not looked at by the analyzer.

Since you would be making multi-threaded the default, I thought it was worth mentioning.

I'm not sure if you have fixed this or I should move this to a new issue for you.

@muglug
Copy link
Collaborator Author

muglug commented Feb 18, 2020

Psalm report it passed when one of the threads had crashed.

This is now fixed, and moreover --threads=max is now the default

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

No branches or pull requests

3 participants