-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
build: auto-set GitHub runner make job count #9690
base: master
Are you sure you want to change the base?
Conversation
Seems this does not compile with Debian and Ubuntu CI jobs |
This PR follows from the discussion in #9510. Based on testing with GitHub macOS runners, I determined the suitable memory requirement to be 2.25GiB per runner, rather than the 2.75GiB hypothesized in the thread: With three jobs (current config) on a 3 CPU + 7GiB machine, a full build takes roughly Should the need arise, this PR allows custom memory requirements to be set per OS. Build output confirms this PR results in job counts identical to the current fixed configuration (by design, the dynamically determined count appears in the build output because it's sourced from an environment variable). |
9a20099
to
8dc82cf
Compare
@selsta Build should now work. A typo snuck in when preparing a rebased version of this branch and I couldn't fix it due to GitHub instability (also the reason my prior comment follows the PR by an hour). |
8dc82cf
to
07774e6
Compare
Looks good so far although my basic arithmetic skills are horrendous (thank you for the 9/4 comment which i seen later, i have just about grasped that 2.25 = 9 quarters) "But where's the math.floor?" i asked myself. TIL Bash does floor rounding by division automatically 1 The only suggestion i have would be returning the ram in bytes at the source rather than dealing with 3 different units. powershell can return value in bytes directly "TotalPhysicalMemory"2 and vmstat has untested: |
If it were trivial to achieve, I would have initially used a common unit (probably KiB, because that's what /proc/meminfo reports, as bytes are unreasonably small). I'd rather not have anything to do with powershell, as that introduces a second major set of OS tools. I could change vmstat to use KiB, but I'm not sure exactly what that would accomplish: It's fairly obvious the command reports MiB, and the unit conversions between KiB, MiB, and GiB, aren't particularly complex (aside from the appearance of k, M, etc, in the commands, the denominators also pretty clearly indicate the scales, and relations between them). |
MacOS reports in bytes and i was unable to find a flag/option for it to get another unit directly, vmstat can but the powershell command is required for windows. It would have made tests more trivial but i checked all calculations with their respective units and it performs correctly. would you consider publishing this action as a public good in its own repository? https://docs.github.com/en/actions/sharing-automations/creating-actions/publishing-actions-in-github-marketplace |
aefe0d2
to
1ce5d3a
Compare
Dynamically determine an appropriate number of make jobs for the GitHub runner container, based upon the number of available CPU cores and RAM, using the rule that each job requires a dedicated core and 2.25GiB of RAM.
1ce5d3a
to
f18b966
Compare
As I had to rebase this branch to avoid conflicts, I made a couple of improvements:
|
No (I don't think it makes any sense, as every project has its own needs, and I have no interest in trying to cater to them). |
Dynamically determine an appropriate number of make jobs for the GitHub runner container, based upon the number of available CPU cores and RAM, using the rule that each job requires a dedicated core and 2.25GiB of RAM.