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

tools: increase macOS cores to 3 on GitHub CI #45340

Merged
merged 1 commit into from
Nov 6, 2022

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 6, 2022

In efbec85, we reduced the cores to 2 based on GitHub documentation. The documentation now says that there are 3 cores.

Refs: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Nov 6, 2022
- name: Test
run: make run-ci -j2 V=1 TEST_CI_ARGS="-p actions --measure-flakiness 9"
run: make run-ci -j3 V=1 TEST_CI_ARGS="-p actions --measure-flakiness 9"
Copy link
Member

Choose a reason for hiding this comment

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

@Trott Can we get the core count using bash, and remove the need to update this workflow in the future? Something like getconf _NPROCESSORS_ONLN

Copy link
Member Author

@Trott Trott Nov 6, 2022

Choose a reason for hiding this comment

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

If we're sure it returns the right number on virtual machines (instead of returning the number available to the physical host that the VM is running on), yeah, that would be great. I'm going to let this run as-is so we can confirm that -j3 is faster than -j2. I'll leave a comment about that on main.

@Trott
Copy link
Member Author

Trott commented Nov 6, 2022

The last successful GitHub Action (at the time of this writing) for building/testing on macOS took 1h 50m 36s for the build step, 33m 0s for the test step, and 2h 24m 49s overall. https://github.com/nodejs/node/actions/runs/3404078997/jobs/5661053610

Hopefully this will improve at least a tiny bit on those times... https://github.com/nodejs/node/actions/runs/3405116815/jobs/5662849595

@Trott
Copy link
Member Author

Trott commented Nov 6, 2022

The last successful GitHub Action (at the time of this writing) for building/testing on macOS took 1h 50m 36s for the build step, 33m 0s for the test step, and 2h 24m 49s overall. https://github.com/nodejs/node/actions/runs/3404078997/jobs/5661053610

Hopefully this will improve at least a tiny bit on those times... https://github.com/nodejs/node/actions/runs/3405116815/jobs/5662849595

This cuts the build time by about 28%, around a half hour. Let's land it!

Build time went from 1h 50m 36s to 1h 19m 11s. 🚀

Test time went from 33m to 30m. Tests are parallelized by tools/test.py and not the make step, so that checks out.

Total time went form 2h 24m 49s to 1h 50m 33s. 🚀

@Trott Trott added build Issues and PRs related to build files or the CI. fast-track PRs that do not need to wait for 48 hours to land. and removed meta Issues and PRs related to the general management of the project. labels Nov 6, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2022

Fast-track has been requested by @Trott. Please 👍 to approve.

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 6, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 6, 2022
@nodejs-github-bot nodejs-github-bot merged commit f9fab41 into nodejs:main Nov 6, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in f9fab41

@Trott Trott deleted the macos-cores branch November 7, 2022 00:09
Trott added a commit to Trott/io.js that referenced this pull request Nov 7, 2022
lucshi pushed a commit to lucshi/node that referenced this pull request Nov 9, 2022
In efbec85, we reduced the cores to 2
based on GitHub documentation. The documentation now says that there are
3 cores.

Refs: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
PR-URL: nodejs#45340
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
In efbec85, we reduced the cores to 2
based on GitHub documentation. The documentation now says that there are
3 cores.

Refs: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
PR-URL: #45340
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Nov 10, 2022
nodejs-github-bot pushed a commit that referenced this pull request Nov 10, 2022
Refs: #45340 (comment)
PR-URL: #45350
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
ruyadorno pushed a commit that referenced this pull request Nov 21, 2022
Refs: #45340 (comment)
PR-URL: #45350
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
In efbec85, we reduced the cores to 2
based on GitHub documentation. The documentation now says that there are
3 cores.

Refs: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
PR-URL: #45340
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
Refs: #45340 (comment)
PR-URL: #45350
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
In efbec85, we reduced the cores to 2
based on GitHub documentation. The documentation now says that there are
3 cores.

Refs: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
PR-URL: #45340
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
Refs: #45340 (comment)
PR-URL: #45350
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
In efbec85, we reduced the cores to 2
based on GitHub documentation. The documentation now says that there are
3 cores.

Refs: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
PR-URL: #45340
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
Refs: #45340 (comment)
PR-URL: #45350
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
Refs: #45340 (comment)
PR-URL: #45350
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants