-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Speed up REPL Tests in CI by grouping apps in a single target #41501
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
base: master
Are you sure you want to change the base?
Conversation
|
PR #41501: Size comparison from a95f163 to 72a847b Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41501 +/- ##
==========================================
- Coverage 51.07% 51.06% -0.02%
==========================================
Files 1384 1385 +1
Lines 100886 100883 -3
Branches 13049 13054 +5
==========================================
- Hits 51531 51516 -15
- Misses 49355 49367 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
PR #41501: Size comparison from a95f163 to 9a73a41 Full report (10 builds for cc13x4_26x4, cc32xx, nrfconnect, realtek, stm32)
|
9a73a41 to
72a847b
Compare
|
PR #41501: Size comparison from a95f163 to e16a5fc Full report (33 builds for bl602, bl702, bl702l, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41501: Size comparison from a95f163 to 7452467 Increases above 0.2%:
Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41501: Size comparison from a95f163 to 8d86273 Increases above 0.2%:
Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41501: Size comparison from a95f163 to 3a964c5 Full report (12 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, realtek, stm32)
|
|
PR #41501: Size comparison from a95f163 to b0c0fd3 Full report (12 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, realtek, stm32)
|
|
PR #41501: Size comparison from a95f163 to 257a1d6 Increases above 0.2%:
Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
…es.py in separate calls and deleting out dir
|
PR #41501: Size comparison from 1f47e8d to fe60440 Full report (9 builds for cc13x4_26x4, cc32xx, realtek, stm32)
|
| # } | ||
|
|
||
| if (enable_linux_apps_for_ci_build) { | ||
| group("linux_apps_for_ci") { |
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.
I would not separate out a CI target - we want builds to still look like "ask to build closure/bridge/..." it should just happen faster.
That would allow us to change tests.yaml without needing to also change BUILD.gn (i.e. have less coupling)
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.
Looking at changes needed for build_examples, I think we can completely drop this part.
| # extra_build_deps += [ ":linux_camera_controller_app" ] | ||
| # } | ||
|
|
||
| if (enable_linux_apps_for_ci_build) { |
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.
can we just enable this by default?
|
PR #41501: Size comparison from 1f47e8d to d213e56 Full report (9 builds for cc13x4_26x4, cc32xx, realtek, stm32)
|
|
PR #41501: Size comparison from 1f47e8d to ceae248 Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #41501: Size comparison from 1f47e8d to 070ed97 Full report (9 builds for cc13x4_26x4, cc32xx, realtek, stm32)
|
070ed97 to
f0f7798
Compare
|
PR #41501: Size comparison from 9dda583 to 16645ca Full report (31 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41501: Size comparison from 9dda583 to 688dbf2 Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41501: Size comparison from 9dda583 to 5c5b783 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41501: Size comparison from 9dda583 to c2d8104 Increases above 0.2%:
Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Summary
This PR significantly speeds up the build of example apps in the "REPL Linux Tests" CI job by introducing a unified build that compiles multiple applications within the same output directory in a single target, rather than building each example from scratch. This change allows the build system to reuse shared libraries and output directories, drastically reducing build times.
The build time for each individual app when building from scratch is about 6min.
We are building 28 apps "REPL Tests" workflow, which means a around 2h40min of build time.
From the 28 apps, I was able to aggregate 19 in a single target
linux_apps_for_ciin the rootBUILD.gnfile. This reduces the build time of those apps in theREPL Testsworkflow from ~2h to ~25min.You can build the
linux_apps_for_citarget locally by executing:Background
CI in the Matter SDK is way too slow. In one of my recent Pull Requests, it took 4h to run and affected my ability to move faster and land code upstream. So I'm investigating investigating ways to make CI faster.
I presented some stats in the last
swttstandup and I created slack channel (#swtt-ci) to group people interested in helping with this effort. Please join if you would like to help.Currently, the biggest CI offender is the REPL tests:

Limitations of this PR
We can still do better. I was not able yet to aggregate the following apps in the same target. Some of them require custom
args.gnior dependencies that need to be refactored. I left them alone for now and they are still building individually from scratch and taking a long time:Testing
Relying on CI to validate