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

Use 2 cores per build task in github CI build #3203

Merged
merged 2 commits into from
Jul 29, 2020

Conversation

kvc0
Copy link

@kvc0 kvc0 commented Jul 25, 2020

As of today, https://docs.github.com/en/actions/reference/virtual-environments-for-github-hosted-runners
states that hosted runners have a 2-core CPU.

This uses make -j $physical_cores to try and be better about utilizing the time spent on those machines.
When github upgrades runners to have more cores we'll benefit from that too. Today it cuts about 1 minute off each m4 board build set. It might be faster still to run 2 single-core builds in parallel.

As of today, https://docs.github.com/en/actions/reference/virtual-environments-for-github-hosted-runners
states that hosted runners have a 2-core CPU.

This uses make -j $physical_cores to try and be better about utilizing the time spent on those machines.
When github upgrades runners to have more cores we'll benefit from that too.
dhalbert
dhalbert previously approved these changes Jul 25, 2020
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thank you for spotting this! I run -j4 builds all the time at home, so it shouldn't cause any issues, and this is a welcome speedup.

I also saw at the link you gave that ubuntu-20.04 is available as a preview. I'm not sure it makes much difference, but we could switch at some point. ubuntu-latest is still 18.04, for now

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

I'm late to the party, but I believe it's the SECOND invocation of make down at line 56-59 where the bulk of the work happens:

        make_result = subprocess.run(
            "make -C ../ports/{port} TRANSLATION={language} BOARD={board} BUILD={build}".format(
                port = board_info["port"], language=language, board=board, build=build_dir),
            shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)

It would make sense to put -j there as well.

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

See my comments above about adding -j2 in a second location.
(For some reason I thought the PR had already been merged when I commented earlier, so I did not Request Changes)

@dhalbert dhalbert dismissed their stale review July 25, 2020 16:32

@jepler found an issue

@dhalbert
Copy link
Collaborator

dhalbert commented Jul 25, 2020

I tried a timing test running -j2 with two boards in sequence vs -j1 with two boards in parallel, but there's something that breaks when the boards are run in parallel: I was getting some errors related to the selected translation.

@kvc0
Copy link
Author

kvc0 commented Jul 25, 2020

I'm late to the party, but I believe it's the SECOND invocation of make down at line 56-59 where the bulk of the work happen:

🤦 thanks

I tried a timing test running -j2 with two boards in sequence vs -j1 with two boards in parallel, but there's something that breaks when the boards are run in parallel: I was getting some errors related to the selected translation.

Yeah, I expect two -j 1's will require more work. Probably just using -j 2 will get 70% of the value. Thanks for checking into it anyway!

@kvc0 kvc0 requested a review from jepler July 25, 2020 18:11
@dhalbert
Copy link
Collaborator

@WarriorOfWire Do you understand why the wall-clock time is not different even after your latest commit, ebc1373?

https://github.com/adafruit/circuitpython/actions:

image

@dhalbert
Copy link
Collaborator

dhalbert commented Jul 25, 2020

I am wondering if the build times are mostly related to the time to do -flto. Each translation is run on top of an existing build, and is not a clean build, so many compilations can be skipped. I thought perhaps LTO was single-threaded, but there's discussion of it being splittable between cores. I tried -flto vs -flto=4 and -flto=auto, and they were all the same for a single build.

@kvc0
Copy link
Author

kvc0 commented Jul 25, 2020

I have seen build times be pretty variable; no I do not know in detail where all of the time went or why this latest run was not materially different than the previous one. This was more of a documentation-based optimization that I saw than a Big Win rigorously validated via scientific method (which I'm not planning to do for this change).

@jepler
Copy link
Member

jepler commented Jul 29, 2020

There are two major components of the build that are time consuming and can't use make's parallelism:

  • lto linking
  • qstr preprocessing

Despite not being a measurable "win" in time spent waiting for CI, the changes ARE correct and an improvement. If we ever are able to improve the above two items, it may put us into the "win" column.

@jepler jepler merged commit fbc7897 into adafruit:main Jul 29, 2020
@tannewt
Copy link
Member

tannewt commented Jul 29, 2020

FWIW, in my ninja branch I was working on parallelizing the qstr preprocessing.

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