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

Combine the no-detail-logging and no-progress-logging builds with the gcc_debug build #8463

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

This better amortizes setup costs: the builds themselves are pretty
cheap compared to the various setup we have to do to get to them.

Problem

We use a lot of CI resources for these logging-compilation tests. Something like 7-8 minutes of CI time of which only about 2-2.5 minutes is the actual build.

Change overview

Just do these builds as part of the gcc_debug job. Don't run tests for them, because we're just trying to ensure things compile.

Testing

  • Verified that CI runs more quickly for this one task (~13 mins) than for the three tasks before (~21 mins).
  • Verified that if I introduce a variable that's only used in a progress log statement, the no-progress-logging build fails with an unused variable error.

@mspang
Copy link
Contributor

mspang commented Jul 16, 2021

Should we just combine all of them?

@bzbarsky-apple
Copy link
Contributor Author

It's a bit more work for the others because we have to do the CodeQL bits, but we could do this, yes....

@bzbarsky-apple bzbarsky-apple force-pushed the combine-no-logging-builds branch from e1ee438 to 09dd5d7 Compare July 20, 2021 03:01
@bzbarsky-apple
Copy link
Contributor Author

@woody-apple @Damian-Nordic @msandstedt @saurabhst @LuDuda Take a look please? Should help a bit with our CI latency.

… gcc_debug build.

This better amortizes setup costs: the builds themselves are pretty
cheap compared to the various setup we have to do to get to them.
@bzbarsky-apple bzbarsky-apple force-pushed the combine-no-logging-builds branch from 09dd5d7 to 5458ac6 Compare July 20, 2021 03:39
@github-actions
Copy link

Size increase report for "esp32-example-build" from d24b3c0

File Section File VM
chip-lock-app.elf .flash.text -64 -64
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
[Unmapped],0,64
.flash.text,-64,-64


@Damian-Nordic Damian-Nordic merged commit 3e57b5f into project-chip:master Jul 20, 2021
@bzbarsky-apple bzbarsky-apple deleted the combine-no-logging-builds branch July 20, 2021 13:35
drempelg pushed a commit to drempelg/connectedhomeip that referenced this pull request Jul 20, 2021
… gcc_debug build. (project-chip#8463)

This better amortizes setup costs: the builds themselves are pretty
cheap compared to the various setup we have to do to get to them.
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
… gcc_debug build. (project-chip#8463)

This better amortizes setup costs: the builds themselves are pretty
cheap compared to the various setup we have to do to get to them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants