-
-
Couldn't load subscription status.
- Fork 344
Scale bundler install jobs with CPU count up to 8 #822
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
Conversation
The number of jobs should scale with the runner CPU count, up to a point. The bottleneck becomes network / disk I/O at very large numbers, and 8 seems like a reasonable maximum based on my experience at this time.
|
So I think 4 was and might still be the fastest on the GH-hosted runners. |
|
This is likely because most public runners have four cores; in this case, the result here would be the same - it would still use four jobs. Private default runners and paid larger runners do not all have the same number of CPU cores. |
|
That page says Linux & Windows have 2 CPU, macOS 3 or 4. |
bundler.js
Outdated
| } | ||
|
|
||
| // Number of jobs should scale with runner, up to a point | ||
| const jobs = Math.min(os.cpus().length, 8) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be os.availableParallelism() maybe? I found https://stackoverflow.com/a/77532237/388803
Also os.cpus().length doesn't seem to work on Windows, see CI results
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it should from the https://nodejs.org/api/os.html#oscpus docs, I pushed a commit to try os.availableParallelism()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmh, that doesn't seem to work on Windows either :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah the error probably means it needs to be a string and not a number for CLI arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I should know by now that Windows is particular about objects and types. I've been working mostly on Mac and Linux, and I figured the tests runners would catch any issues anyway, but this is good to know.
From https://github.com/ruby/setup-ruby/actions/runs/18838903560/job/53746296174?pr=822#step:3:59 So it looks safe indeed and shouldn't be slower except maybe a little bit on macOS arm64 (3 instead of 4). |
* As https://nodejs.org/api/os.html#oscpus says: os.cpus().length should not be used to calculate the amount of parallelism available to an application. Use os.availableParallelism() for this purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR
My bet is that it will still be marginally faster on average, because it will just use three jobs, but that it won't be a huge difference overall. Using slightly more jobs than the number of cores does not always slow things down, but it can because of context switching and overhead / higher memory usage. In general, if nothing else is running, using the same number as core count is a safe bet until you get into extreme parallelism. In cases where you have other services running such as PostgreSQL or Redis, it might make sense to use even less. |
The number of jobs should scale with the runner CPU count, up to a point. The bottleneck becomes network / disk I/O at very large numbers, and 8 seems like a reasonable maximum based on my experience at this time.
Many of the private repos that I work on use runners with GitHub-hosted runners with only 2 cores, and oversubscribing there can actually lead to slower runs.