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

Optimize build workflow #2757

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

urob
Copy link
Contributor

@urob urob commented Jan 9, 2025

This PR contains two performance tweaks for the GHA build workflow:

  1. Use treeless clones when pulling in dependencies. This reduces the download size from 1.7 GB to 610 MB.
  2. Replace yaml2json | jq with yq. This saves the installation step shaving off a few more seconds.

The following table summarizes the performance impact for building a reviung41 with the stock config on ZMK v0.1.

cold cache warm cache cache size
upstream 5m 28s 2m 02s 1.7 GB
treeless PR 3m 51s 1m 45s 610 MB

(See https://github.com/urob/zmk-build-benchmarks/actions for the benchmarks.)

Note

The dependency change is most impactful on cold caches. As Github caches expire after 7 days this should be a nice speed up for many build-user-config workflow runs.

For the upstream repo the gains are more limited since the cache should rarely be cold. On the other hand, there isn't really a downside since none of the workflows interacts with trees that aren't reachable from HEAD. For now I also applied the tweak to the ZMK build and test workflows but happy to revert that.

PR check-list

  • Branch has a clean commit history
  • Additional tests are included, if changing behaviors/core code that is testable.
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • Pre-commit used to check formatting of files, commit messages, etc.
  • Includes any necessary documentation changes.

urob added 2 commits January 9, 2025 13:45
Github's ubuntu runners come with `yq` installed, which can replace
piping `yaml2json` to `jq`. It also saves installing `yaml2json`.

It's worth noting that the `yq` version installed is the `go` version
(which has a slightly different syntax than the competing `python`
version).
@urob urob requested a review from a team as a code owner January 9, 2025 20:10
@urob
Copy link
Contributor Author

urob commented Jan 9, 2025

As an aside, for local development using "blobless" clones gives most of the same performance gains without any of the performance penalties from interacting with trees that aren't reachable from HEAD.

To use them, simply replace west update with

west update --fetch-opt=--filter=blob:none

This reduces download size from 1.7 GB to around 800 MB.

@urob
Copy link
Contributor Author

urob commented Jan 9, 2025

One more aside: The ZMK's west manifest pulls in a lot of dependencies that aren't needed for most builds. For example, most nice nano v2-based builds only require 4 modules:

- cmsis
- hal_nordic
- lvgl
- tinycrypt

Pulling in just those modules reduces build times on a cold cache from currently 5m 28s to 2m 11s!! Since most in-tree boards are nordic boards, this might be an argument to moving other boards to modules and specify those extra dependencies in the module manifest.

cold cache warm cache cache size
upstream 5m 28s 2m 02s 1.7 GB
treeless PR 3m 51s 1m 45s 610 MB
minimal + treeless 2m 11s 1m 36s 240 MB

@Nick-Munnich Nick-Munnich added the github_actions Pull requests that update Github_actions code label Jan 11, 2025
caksoylar added a commit to caksoylar/keymap-drawer that referenced this pull request Jan 18, 2025
Using treeless clones from zmkfirmware/zmk#2757
and manifest.project-filter from https://github.com/zmkfirmware/zmk-cli/blob/main/zmk/repo.py#L253
to reduce amount of data fetched.
caksoylar added a commit to caksoylar/keymap-drawer that referenced this pull request Jan 18, 2025
Using treeless clones from zmkfirmware/zmk#2757
and manifest.project-filter from https://github.com/zmkfirmware/zmk-cli/blob/main/zmk/repo.py#L253
to reduce amount of data fetched.

Remove caching since it is no longer very necessary and default to turning it on,
since it adds <10s to runtime.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update Github_actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants