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

Small artifacts for bloat reports #9331

Merged
merged 8 commits into from
Sep 13, 2021
Merged

Conversation

kpschoedel
Copy link
Contributor

@kpschoedel kpschoedel commented Aug 30, 2021

Small-artifact bloat reports

Problem

The current bloat report requires preserving large binaries, and has
trouble matching tree parents for comparison.

Change overview

This change makes example builds generate small artifacts containing
json file(s) containing build item identification and section sizes.

Artifacts names have the form “Size,PR,WORKFLOW,CURRENT_SHA,PARENT_SHA”;
these contain one or more json files, each containing a report on one
build target.

The reporting scripts then examines the list of available artifacts to
list of artifacts to identify pairs with the same $WORKFLOW where one's
CURRENT_SHA is the other's PARENT_SHA, and only then downloads and
processes those artifacts to generate comparisons for matching builds.

  • Changes to examples-… workflows:

    • Adds some GH_EVENT_… variables to workflow env, to identify
      the current run.
    • Uses the gh_sizes.py script to generate size report json files.
    • Uploads size report artifacts.
  • Changes to scripts/tools/memory:

    • Adds minimal platform config files for recently-added platforms.
    • Adds markdown output options (required for github comments) along
      with some associated cleanup.
    • Adds a script scripts/tools/memory/gh_sizes.py for use by workflows;
      this is similar to report_summary.py with a suitable consistent set
      of arguments.
    • Adds a script scripts/tools/memory/gh_report.py to analyze size
      report artifacts.
  • Modifies bloat_check.py to ignore the size report artifacts.

The github comments produced by gh_report.py are slightly different
fromt the existing reports. Since this change enables reports for many
more builds, a full report will be over a hundred lines, and is placed
inside a details tag. Only size changes above a configurable threshold
are called out ‘above the fold’.

Note that this PR does not include a change to actually send size
report comments on github; this is left to a followup after that code
has been verified on real-world size artifacts.

Testing

Fork-CI runs and offline verification on resulting artifacts.

@kpschoedel kpschoedel force-pushed the nubloat branch 5 times, most recently from e16da0a to 0a266db Compare September 7, 2021 18:52
@kpschoedel kpschoedel changed the title Prototype small bloat report artifacts Small artifacts for bloat reports Sep 7, 2021
@kpschoedel kpschoedel force-pushed the nubloat branch 2 times, most recently from 379650a to a569bdd Compare September 7, 2021 21:55
@kpschoedel kpschoedel marked this pull request as ready for review September 7, 2021 21:57
@kpschoedel
Copy link
Contributor Author

Note: the bloat check workflow is expected to run this with something like :

scripts/tools/memory/gh_report.py \
  --verbose \
  --report-increases 1 \
  --report-pr \
  --github-comment \
  --github-repository project-chip/connectedhomeip \
  --github-api-token "${{ secrets.GITHUB_TOKEN }}"

(in this example highlighting size increases over 1%)

@mspang
Copy link
Contributor

mspang commented Sep 9, 2021

Do you have an example of the reports will look like?

@kpschoedel
Copy link
Contributor Author

Do you have an example of the reports will look like?

A small fake-data example is below. For actual runs, the table would contain all the available builds. I'll try to add a fuller example later.


PR #1234: Size comparison from d571fb9 to e1ea910

Increases above 1.0% from d571fb9 to e1ea910:

platform target config section e1ea910 d571fb9 change % change
mbed lighting-app CY8CPROTO_062_4343W+release .data 4960 5016 56 1.1
.text 1087392 1109600 22208 2.0
.zero.table 0 8 8 inf
1 build
platform target config section e1ea910 d571fb9 change % change
mbed lighting-app CY8CPROTO_062_4343W+release ABS 4096 4096 0 0.0
.bss 170044 171396 1352 0.8
.copy.table 24 24 0 0.0
.cy_m0p_image 5996 5996 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 4960 5016 56 1.1
.heap 862464 861056 -1408 -0.2
.noinit 120 120 0 0.0
.ramVectors 736 736 0 0.0
.text 1087392 1109600 22208 2.0
.zero.table 0 8 8 inf

@mspang
Copy link
Contributor

mspang commented Sep 9, 2021

Do you have an example of the reports will look like?

A small fake-data example is below. For actual runs, the table would contain all the available builds. I'll try to add a fuller example later.

PR #1234: Size comparison from d571fb9 to e1ea910

Increases above 1.0% from d571fb9 to e1ea910:

platform target config section e1ea910 d571fb9 change % change
mbed lighting-app CY8CPROTO_062_4343W+release .data 4960 5016 56 1.1
.text 1087392 1109600 22208 2.0
.zero.table 0 8 8 inf
1 build

Nice.. I've really missed the baseline in the current reports.

@kpschoedel
Copy link
Contributor Author

Here's an example of what a full report would look like, given the workflows in this PR. (This is a comparison of two handy builds on my fork, and doesn't match an actual commit or PR.)


Size comparison from a184f57 to df7771f

Increases above 1.0% from a184f57 to df7771f:

platform target config section df7771f a184f57 change % change
k32w shell k32w061+debug .data 660 672 12 1.8
linux ota-provider-app debug RAM 60792 61520 728 1.2
shell debug RAM 55832 56576 744 1.3
qpg lighting-app qpg6100+debug .data 980 992 12 1.2
lock-app qpg6100+debug .data 916 928 12 1.3
persistent-storage-app qpg6100+debug .data 268 288 20 7.5
.text 107092 108788 1696 1.6
40 builds
platform target config section df7771f a184f57 change % change
efr32 lighting-app BRD4161A .bss 260360 260352 -8 -0.0
.data 1780 1792 12 0.7
.text 752612 753364 752 0.1
lock-app BRD4161A .bss 260424 260408 -16 -0.0
.data 1720 1732 12 0.7
.text 731232 731968 736 0.1
window-app BRD4161A .bss 260416 260408 -8 -0.0
.data 1724 1736 12 0.7
.text 732632 733384 752 0.1
lighting-app BRD4161A+rpc .bss 260296 260288 -8 -0.0
.data 1844 1856 12 0.7
.text 730136 730872 736 0.1
esp32 all-clusters-app c3devkit .dram0.bss 65336 65384 48 0.1
.dram0.data 16280 16288 8 0.0
.flash.rodata 196856 196960 104 0.1
.flash.text 857648 857772 124 0.0
.iram0.text 57320 57320 0 0.0
bridge-app default .dram0.bss 65648 65664 16 0.0
.dram0.data 19980 19988 8 0.0
.flash.rodata 156316 156392 76 0.0
.flash.text 749327 749507 180 0.0
.iram0.text 115755 115755 0 0.0
ipv6only-app default .dram0.bss 19760 19760 0 0.0
.dram0.data 16232 16232 0 0.0
.flash.rodata 99040 99040 0 0.0
.flash.text 471015 471015 0 0.0
.iram0.text 84275 84275 0 0.0
lock-app default .dram0.bss 47384 47392 8 0.0
.dram0.data 19796 19804 8 0.0
.flash.rodata 157284 157368 84 0.1
.flash.text 751871 752099 228 0.0
.iram0.text 115755 115755 0 0.0
persistent-storage default .dram0.bss 2152 2152 0 0.0
.dram0.data 10128 10128 0 0.0
.flash.rodata 30896 30896 0 0.0
.flash.text 97451 97451 0 0.0
.iram0.text 41151 41151 0 0.0
pigweed-app default .dram0.bss 3456 3456 0 0.0
.dram0.data 10208 10208 0 0.0
.flash.rodata 35440 35440 0 0.0
.flash.text 98183 98183 0 0.0
.iram0.text 41727 41727 0 0.0
shell default .dram0.bss 36640 36640 0 0.0
.dram0.data 19692 19700 8 0.0
.flash.rodata 142868 142936 68 0.0
.flash.text 724799 724975 176 0.0
.iram0.text 119015 119015 0 0.0
all-clusters-app m5stack .dram0.bss 64480 64536 56 0.1
.dram0.data 32132 32140 8 0.0
.flash.rodata 198420 198536 116 0.1
.flash.text 863859 864027 168 0.0
.iram0.text 121839 121839 0 0.0
temperature-measurement-app optimize .dram0.bss 45880 45888 8 0.0
.dram0.data 15204 15212 8 0.1
.flash.rodata 110084 110160 76 0.1
.flash.text 716447 716603 156 0.0
.iram0.text 107475 107475 0 0.0
k32w lock-app k32w061+debug .bss 66460 66476 16 0.0
.data 1820 1832 12 0.7
.text 496100 496372 272 0.1
shell k32w061+debug .bss 56296 56296 0 0.0
.data 660 672 12 1.8
.text 365460 365700 240 0.1
lighting-app k32w061+se05x+release .bss 76400 76424 24 0.0
.data 1884 1896 12 0.6
.text 594100 594384 284 0.0
linux all-clusters-app debug unknown 4064 4064 0 0.0
FLASH 1309123 1297027 -12096 -0.9
RAM 115592 116400 808 0.7
chip-tool debug unknown 4128 4128 0 0.0
FLASH 2560593 2545297 -15296 -0.6
RAM 89088 89832 744 0.8
ota-provider-app debug unknown 3928 3928 0 0.0
FLASH 1011854 999638 -12216 -1.2
RAM 60792 61520 728 1.2
shell debug unknown 3896 3896 0 0.0
FLASH 696093 697133 1040 0.1
RAM 55832 56576 744 1.3
tv-app debug unknown 4184 4184 0 0.0
FLASH 1430476 1417516 -12960 -0.9
RAM 267808 268584 776 0.3
bridge-app debug+rpc unknown 3912 3912 0 0.0
FLASH 1058053 1045973 -12080 -1.1
RAM 79992 80752 760 1.0
lighting-app debug+rpc unknown 4096 4096 0 0.0
FLASH 1244367 1232239 -12128 -1.0
RAM 94808 95552 744 0.8
mbed lighting-app CY8CPROTO_062_4343W+release .bss 171396 171420 24 0.0
.data 5016 5024 8 0.2
.heap 861056 861024 -32 -0.0
.text 1109500 1110012 512 0.0
lock-app CY8CPROTO_062_4343W+release .bss 170044 170060 16 0.0
.data 4960 4976 16 0.3
.heap 862464 862432 -32 -0.0
.text 1087356 1087804 448 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 bss 112892 112908 16 0.0
rodata 536312592 536312224 -368 -0.0
text 557172 557536 364 0.1
lock-app nrf52840dk_nrf52840 bss 111508 111520 12 0.0
rodata 536330512 536330144 -368 -0.0
text 539286 539654 368 0.1
pigweed-app nrf52840dk_nrf52840 bss 52232 52232 0 0.0
rodata 536533632 536533632 0 0.0
text 336330 336330 0 0.0
pump-app nrf52840dk_nrf52840 bss 111932 111940 8 0.0
rodata 536327216 536326832 -384 -0.0
text 542596 542964 368 0.1
pump-controller-app nrf52840dk_nrf52840 bss 111432 111440 8 0.0
rodata 536331968 536331600 -368 -0.0
text 537840 538204 364 0.1
shell nrf52840dk_nrf52840 bss 110236 110232 -4 -0.0
rodata 536347344 536346944 -400 -0.0
text 522472 522874 402 0.1
lighting-app nrf52840dk_nrf52840+rpc bss 109108 109128 20 0.0
rodata 536336832 536336464 -368 -0.0
text 533060 533424 364 0.1
nrf5340dk_nrf5340_cpuapp bss 115860 115880 20 0.0
rodata 536374592 536374224 -368 -0.0
text 494496 494860 364 0.1
lock-app nrf5340dk_nrf5340_cpuapp bss 114480 114488 8 0.0
rodata 536392528 536392160 -368 -0.0
text 476594 476962 368 0.1
shell nrf5340dk_nrf5340_cpuapp bss 112816 112812 -4 -0.0
rodata 536420176 536419776 -400 -0.0
text 448982 449384 402 0.1
p6 lock-app default .bss 67976 67992 16 0.0
.data 2384 2400 16 0.7
.heap 967080 967048 -32 -0.0
.text 1101416 1102200 784 0.1
qpg lighting-app qpg6100+debug .bss 88040 88032 -8 -0.0
.data 980 992 12 1.2
.text 482772 483056 284 0.1
lock-app qpg6100+debug .bss 88104 88096 -8 -0.0
.data 916 928 12 1.3
.text 459060 459340 280 0.1
persistent-storage-app qpg6100+debug .bss 88760 88736 -24 -0.0
.data 268 288 20 7.5
.text 107092 108788 1696 1.6
telink lighting-app tlsr9518adk80d bss 71404 71416 12 0.0
noinit 33216 33216 0 0.0
text 444114 444454 340 0.1

#### Problem

The current bloat report requires preserving large binaries, and has
trouble matching tree parents for comparison.

#### Change overview

This change makes example builds generate small artifacts containing
json file(s) containing build item identification and section sizes.

Artifacts names have the form “Size,PR,WORKFLOW,CURRENT_SHA,PARENT_SHA”;
these contain one or more json files, each containing a report on one
build target.

The reporting scripts then examines the list of available artifacts to
list of artifacts to identify pairs with the same $WORKFLOW where one's
$CURRENT_SHA is the other's $PARENT_SHA, and only then downloads and
processes those artifacts to generate comparisons for matching builds.

- Changes to `examples-…` workflows:

  - Adds some `GH_EVENT_…` variables to workflow `env`, to identify
    the current run.
  - Uses the `gh_sizes.py` script to generate size report json files.
  - Uploads size report artifacts.

- Changes to `scripts/tools/memory`:
  - Adds minimal platform config files for recently-added platforms.
  - Adds markdown output options (required for github comments) along
    with some associated cleanup.
  - Adds a script `scripts/tools/memory/gh_sizes.py` for use by workflows;
    this is similar to `report_summary.py` with a suitable consistent set
    of arguments.
  - Adds a script `scripts/tools/memory/gh_report.py` to analyze size
    report artifacts.

- Modifies `bloat_check.py` to ignore the size report artifacts.

The github comments produced by `gh_report.py` are slightly different
fromt the existing reports. Since this change enables reports for many
more builds, a full report will be over a hundred lines, and is placed
inside a details tag. Only size changes above a configurable threshold
are called out ‘above the fold’.

Note that this PR does _not_ include a change to actually send size
report comments on github; this is left to a followup after that code
has been verified on real-world size artifacts.

#### Testing

Fork-CI runs and offline verification on resulting artifacts.
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

I did not carefully review and understand all the scripts/tools/memory/memdf changes....

I also did not verify the various section names under scripts/tools/memory/platform/

.github/workflows/examples-efr32.yaml Show resolved Hide resolved
.github/workflows/examples-linux-standalone.yaml Outdated Show resolved Hide resolved
scripts/examples/esp_example.sh Outdated Show resolved Hide resolved
scripts/examples/gn_efr32_example.sh Outdated Show resolved Hide resolved
scripts/tools/memory/gh_report.py Show resolved Hide resolved
- restore original scripts/examples/esp_example.sh
  and scripts/examples/gn_efr32_example.sh
- avoid a problem using pyelftools `describe_p_type`
@github-actions
Copy link

Size increase report for "esp32-example-build" from 2addf00

File Section File VM
chip-ipv6only-app.elf .flash.text 172 172
chip-lock-app.elf .flash.text 60 60
chip-shell.elf .flash.text 43 48
chip-bridge-app.elf .flash.text -72 -72
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-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

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

sections,vmsize,filesize
[Unmapped],0,3924
.flash.text,172,172

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

sections,vmsize,filesize
.flash.text,60,60
.debug_loc,0,4
.xt.lit._ZNK4chip4SpanIhE7SubSpanEjj,0,-8
.xt.prop._ZNK4chip4SpanIhE7SubSpanEjj,0,-12
[Unmapped],0,-60

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

sections,vmsize,filesize
[Unmapped],0,4053
.flash.text,48,43

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

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

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


@woody-apple woody-apple merged commit a24a6c3 into project-chip:master Sep 13, 2021
@kpschoedel kpschoedel deleted the nubloat branch September 29, 2021 20:18
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this pull request Sep 30, 2021
#### Problem

The current bloat report requires preserving large binaries, and has
trouble matching tree parents for comparison.

In a24a6c3 (PR project-chip#9331), many workflows started uploading small json
artifacts contains size information for their builds, but these have not
yet been used to generate reports.

#### Change overview

This change adds a run of `scripts/tools/memory/gh_report.py` to the
periodic bloat check workflow, with tight rate limiting in case there
are unexpected problems.

#### Testing

Manually run offline with various data sets, and minimally on the live
repository.
andy31415 pushed a commit that referenced this pull request Sep 30, 2021
#### Problem

The current bloat report requires preserving large binaries, and has
trouble matching tree parents for comparison.

In a24a6c3 (PR #9331), many workflows started uploading small json
artifacts contains size information for their builds, but these have not
yet been used to generate reports.

#### Change overview

This change adds a run of `scripts/tools/memory/gh_report.py` to the
periodic bloat check workflow, with tight rate limiting in case there
are unexpected problems.

#### Testing

Manually run offline with various data sets, and minimally on the live
repository.
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.

5 participants