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.

Sign up for GitHub

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

Jump to bottom

Enable building tracing in chip-tool by default #13702

Merged
merged 2 commits into from
Jan 19, 2022
Merged

Enable building tracing in chip-tool by default #13702

merged 2 commits into from
Jan 19, 2022

Conversation

tcarmelveilleux
Copy link
Contributor

Problem

  • Transport and other tracing is useful for both test automations based
    around chip-tool and for debugging.
  • Enabling tracing in chip-tool is already conditional at runtime based on command
    line flags, but requires building with the tracing enabled
  • Tracing is very low-touch and is zero cost when disabled at runtime, so having it
    enabled by default in chip-tool build makes it easier to access/use.

Issue #11552

Change overview

  • This PR enables the build to always include tracing feature
    enabled in chip-tool since it's generally useful.

Testing

Testing done:

  • Unit tests and integration pass with no change
  • Tested that with --trace_file XXX and --trace_log 1, the tracing works as intended

@restyled-io restyled-io bot mentioned this pull request Jan 19, 2022
Closed
@github-actions
Copy link

github-actions bot commented Jan 19, 2022 •
edited
Loading

PR #13702: Size comparison from a04bf54 to 80b3181

Increases above 0.2%:

platform target config section a04bf54 80b3181 change % change
linux chip-tool-ipv6only arm64 .data 1112 1120 8 0.7
.init_array 168 184 16 9.5
Increases (1 build for linux)
platform target config section a04bf54 80b3181 change % change
linux chip-tool-ipv6only arm64 (read only) 8123116 8140764 17648 0.2
(read/write) 374225 374577 352 0.1
.bss 55217 55345 128 0.2
.data 1112 1120 8 0.7
.data.rel.ro 248696 248864 168 0.1
.got 65504 65528 24 0.0
.init_array 168 184 16 9.5
.rodata 422244 422724 480 0.1
.text 6903204 6919540 16336 0.2
Full report (14 builds for efr32, esp32, linux, p6, qpg, telink)
platform target config section a04bf54 80b3181 change % change
efr32 lighting-app BRD4161A (read only) 834948 834948 0 0.0
(read/write) 127636 127636 0 0.0
.bss 125744 125744 0 0.0
.data 1892 1892 0 0.0
.text 834940 834940 0 0.0
BRD4161A+rpc (read only) 822352 822352 0 0.0
(read/write) 144296 144296 0 0.0
.bss 142304 142304 0 0.0
.data 1992 1992 0 0.0
.text 822344 822344 0 0.0
window-app BRD4161A (read only) 805536 805536 0 0.0
(read/write) 126328 126328 0 0.0
.bss 124480 124480 0 0.0
.data 1844 1844 0 0.0
.text 805528 805528 0 0.0
esp32 all-clusters-app c3devkit (read only) 912352 912352 0 0.0
(read/write) 1316698 1316698 0 0.0
.dram0.bss 70528 70528 0 0.0
.dram0.data 14292 14292 0 0.0
.flash.rodata 178384 178384 0 0.0
.flash.text 912352 912352 0 0.0
.iram0.text 62056 62056 0 0.0
m5stack (read only) 961231 961231 0 0.0
(read/write) 448856 448856 0 0.0
.dram0.bss 74984 74984 0 0.0
.dram0.data 34072 34072 0 0.0
.flash.rodata 207672 207672 0 0.0
.flash.text 955847 955847 0 0.0
.iram0.text 123399 123399 0 0.0
linux chip-tool-ipv6only arm64 (read only) 8123116 8140764 17648 0.2
(read/write) 374225 374577 352 0.1
.bss 55217 55345 128 0.2
.data 1112 1120 8 0.7
.data.rel.ro 248696 248864 168 0.1
.dynamic 560 560 0 0.0
.got 65504 65528 24 0.0
.init 24 24 0 0.0
.init_array 168 184 16 9.5
.rodata 422244 422724 480 0.1
.text 6903204 6919540 16336 0.2
thermostat-no-ble arm64 (read only) 2044220 2044220 0 0.0
(read/write) 145985 145985 0 0.0
.bss 65089 65089 0 0.0
.data 896 896 0 0.0
.data.rel.ro 73016 73016 0 0.0
.dynamic 560 560 0 0.0
.got 4048 4048 0 0.0
.init 24 24 0 0.0
.init_array 304 304 0 0.0
.rodata 129932 129932 0 0.0
.text 1699680 1699680 0 0.0
p6 all-clusters-app default (read/write) 2405304 2405304 0 0.0
.bss 117556 117556 0 0.0
.data 2608 2608 0 0.0
.text 1363568 1363568 0 0.0
light-app default (read/write) 2330160 2330160 0 0.0
.bss 106128 106128 0 0.0
.data 2400 2400 0 0.0
.text 1288424 1288424 0 0.0
lock-app default (read/write) 2299184 2299184 0 0.0
.bss 104976 104976 0 0.0
.data 2352 2352 0 0.0
.text 1257448 1257448 0 0.0
qpg lighting-app qpg6105+debug (read only) 565152 565152 0 0.0
(read/write) 146940 146940 0 0.0
.bss 89960 89960 0 0.0
.data 1056 1056 0 0.0
.text 559832 559832 0 0.0
lock-app qpg6105+debug (read only) 515564 515564 0 0.0
(read/write) 146936 146936 0 0.0
.bss 88584 88584 0 0.0
.data 980 980 0 0.0
.text 510244 510244 0 0.0
persistent-storage-app qpg6105+debug (read only) 106848 106848 0 0.0
(read/write) 146940 146940 0 0.0
.bss 38512 38512 0 0.0
.data 288 288 0 0.0
.text 101528 101528 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 840886 840886 0 0.0
bss 87316 87316 0 0.0
noinit 37160 37160 0 0.0
text 587842 587842 0 0.0

@bzbarsky-apple
Copy link
Contributor

/rebase

tcarmelveilleux and others added 2 commits January 19, 2022 17:16
@tcarmelveilleux @bzbarsky-apple
Enable building tracing in chip-tool by default
768534e 
- Tracing now works enough that it's generally useful
- Tracing is already gated by command-line args, so not
  running by default.
- This PR enables the build to always include tracing feature
  enabled in chip-tool since it's generally useful.

Testing done:

- Unit tests and integration pass with no change
- chip-tool now always has --trace_log/--trace_file enabled

Issue project-chip#11552
@restyled-commits @bzbarsky-apple
Restyled by prettier-markdown
5986a8c
@woody-apple woody-apple force-pushed the enable-tracing-chip-tool-default branch from 80b3181 to 5986a8c Compare January 19, 2022 17:16
@github-actions
Copy link

PR #13702: Size comparison from 5274676 to 5986a8c

Increases above 0.2%:

platform target config section 5274676 5986a8c change % change
linux chip-tool-ipv6only arm64 (read only) 8122444 8140092 17648 0.2
.bss 55217 55345 128 0.2
.data 1112 1120 8 0.7
.init_array 168 184 16 9.5
.text 6902804 6919140 16336 0.2
Increases (1 build for linux)
platform target config section 5274676 5986a8c change % change
linux chip-tool-ipv6only arm64 (read only) 8122444 8140092 17648 0.2
(read/write) 374145 374497 352 0.1
.bss 55217 55345 128 0.2
.data 1112 1120 8 0.7
.data.rel.ro 248632 248800 168 0.1
.got 65488 65512 24 0.0
.init_array 168 184 16 9.5
.rodata 422212 422692 480 0.1
.text 6902804 6919140 16336 0.2
Full report (14 builds for efr32, k32w, linux, p6, qpg, telink)
platform target config section 5274676 5986a8c change % change
efr32 lighting-app BRD4161A (read only) 834836 834836 0 0.0
(read/write) 127636 127636 0 0.0
.bss 125744 125744 0 0.0
.data 1892 1892 0 0.0
.text 834828 834828 0 0.0
window-app BRD4161A (read only) 805408 805408 0 0.0
(read/write) 126328 126328 0 0.0
.bss 124480 124480 0 0.0
.data 1844 1844 0 0.0
.text 805400 805400 0 0.0
lighting-app BRD4161A+rpc (read only) 822224 822224 0 0.0
(read/write) 144296 144296 0 0.0
.bss 142304 142304 0 0.0
.data 1992 1992 0 0.0
.text 822216 822216 0 0.0
k32w light k32w061+release (read/write) 659060 659060 0 0.0
.bss 77136 77136 0 0.0
.data 1860 1860 0 0.0
.text 574264 574264 0 0.0
lock k32w061+release (read/write) 661792 661792 0 0.0
.bss 77432 77432 0 0.0
.data 1880 1880 0 0.0
.text 576680 576680 0 0.0
linux chip-tool-ipv6only arm64 (read only) 8122444 8140092 17648 0.2
(read/write) 374145 374497 352 0.1
.bss 55217 55345 128 0.2
.data 1112 1120 8 0.7
.data.rel.ro 248632 248800 168 0.1
.dynamic 560 560 0 0.0
.got 65488 65512 24 0.0
.init 24 24 0 0.0
.init_array 168 184 16 9.5
.rodata 422212 422692 480 0.1
.text 6902804 6919140 16336 0.2
thermostat-no-ble arm64 (read only) 2043580 2043580 0 0.0
(read/write) 145905 145905 0 0.0
.bss 65089 65089 0 0.0
.data 896 896 0 0.0
.data.rel.ro 72952 72952 0 0.0
.dynamic 560 560 0 0.0
.got 4032 4032 0 0.0
.init 24 24 0 0.0
.init_array 304 304 0 0.0
.rodata 129916 129916 0 0.0
.text 1699296 1699296 0 0.0
p6 all-clusters-app default (read/write) 2405536 2405536 0 0.0
.bss 117556 117556 0 0.0
.data 2608 2608 0 0.0
.text 1363800 1363800 0 0.0
light-app default (read/write) 2329976 2329976 0 0.0
.bss 106128 106128 0 0.0
.data 2400 2400 0 0.0
.text 1288240 1288240 0 0.0
lock-app default (read/write) 2299000 2299000 0 0.0
.bss 104976 104976 0 0.0
.data 2352 2352 0 0.0
.text 1257264 1257264 0 0.0
qpg lighting-app qpg6105+debug (read only) 565064 565064 0 0.0
(read/write) 146940 146940 0 0.0
.bss 89960 89960 0 0.0
.data 1056 1056 0 0.0
.text 559744 559744 0 0.0
lock-app qpg6105+debug (read only) 515476 515476 0 0.0
(read/write) 146936 146936 0 0.0
.bss 88584 88584 0 0.0
.data 980 980 0 0.0
.text 510156 510156 0 0.0
persistent-storage-app qpg6105+debug (read only) 106848 106848 0 0.0
(read/write) 146940 146940 0 0.0
.bss 38512 38512 0 0.0
.data 288 288 0 0.0
.text 101528 101528 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 840698 840698 0 0.0
bss 87316 87316 0 0.0
noinit 37160 37160 0 0.0
text 587724 587724 0 0.0

@andy31415 andy31415 merged commit e3aa71b into project-chip:master Jan 19, 2022
selissia pushed a commit to selissia/connectedhomeip that referenced this pull request Jan 28, 2022
@tcarmelveilleux @restyled-commits @selissia
Enable building tracing in chip-tool by default (project-chip#13702)
37bdd3a 
* Enable building tracing in chip-tool by default

- Tracing now works enough that it's generally useful
- Tracing is already gated by command-line args, so not
  running by default.
- This PR enables the build to always include tracing feature
  enabled in chip-tool since it's generally useful.

Testing done:

- Unit tests and integration pass with no change
- chip-tool now always has --trace_log/--trace_file enabled

Issue project-chip#11552

* Restyled by prettier-markdown

Co-authored-by: Restyled.io <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@saurabhst saurabhst saurabhst approved these changes

@rgoliver rgoliver rgoliver approved these changes

@msandstedt msandstedt msandstedt approved these changes

@andy31415 andy31415 andy31415 approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@pan-apple pan-apple Awaiting requested review from pan-apple

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees
No one assigned
Labels
examples review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tcarmelveilleux @bzbarsky-apple @andy31415 @msandstedt @rgoliver @saurabhst @restyled-commits