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

Fix SessionHandle args so they are const ref #13053

Merged
merged 1 commit into from
Dec 16, 2021
Merged

Fix SessionHandle args so they are const ref #13053

merged 1 commit into from
Dec 16, 2021

Conversation

mlepage-google
Copy link
Contributor

@mlepage-google mlepage-google commented Dec 15, 2021 •
edited
Loading

Problem

SessionHandle args sometimes passed by value (costly) and even const value
(mistake?) in addition to const reference (preferred).

Change overview

Change SessionHandle args to const reference.

Decreases size on 11 of 12 builds, by about 1K (but notably 6K for p6).

Testing

Build, unit tests, CI.

@mlepage-google
Fix SessionHandle args so they are const ref
ad8a17d 
They were being passed by const ref in some cases, but also by value,
and in a few cases by const value.
@github-actions
Copy link

github-actions bot commented Dec 15, 2021 •
edited
Loading

PR #13053: Size comparison from c939e82 to ad8a17d

Decreases (29 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section c939e82 ad8a17d change % change
efr32 lighting-app BRD4161A (read only) 836360 830232 -6128 -0.7
.text 836352 830224 -6128 -0.7
BRD4161A+rpc (read only) 823964 817836 -6128 -0.7
.text 823956 817828 -6128 -0.7
window-app BRD4161A (read only) 809720 803592 -6128 -0.8
.text 809712 803584 -6128 -0.8
esp32 all-clusters-app c3devkit (read only) 876682 875744 -938 -0.1
.flash.text 876682 875744 -938 -0.1
m5stack (read only) 962507 961603 -904 -0.1
.flash.text 957123 956219 -904 -0.1
k32w lighting-app k32w061+se05x+release (read/write) 687048 686088 -960 -0.1
.text 601060 600100 -960 -0.2
lock-app k32w061+debug (read/write) 634612 633652 -960 -0.2
.text 550520 549560 -960 -0.2
shell k32w061+debug (read/write) 640016 638912 -1104 -0.2
.text 553636 552532 -1104 -0.2
linux chip-tool-ipv6only arm64 (read only) 6884788 6873204 -11584 -0.2
.text 5824820 5813236 -11584 -0.2
thermostat-no-ble arm64 (read only) 1994756 1993620 -1136 -0.1
.text 1655456 1654320 -1136 -0.1
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2334344 2333384 -960 -0.0
.text 1296920 1295960 -960 -0.1
lighting-app CY8CPROTO_062_4343W+release (read/write) 2329840 2328880 -960 -0.0
.text 1292440 1291480 -960 -0.1
lock-app CY8CPROTO_062_4343W+release (read/write) 2302936 2301912 -1024 -0.0
.text 1265536 1264512 -1024 -0.1
shell CY8CPROTO_062_4343W+release (read/write) 2054328 2053688 -640 -0.0
.text 1016928 1016288 -640 -0.1
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 922923 921963 -960 -0.1
text 624212 623264 -948 -0.2
nrf52840dk_nrf52840+rpc (read/write) 885963 885003 -960 -0.1
text 598952 597988 -964 -0.2
nrf5340dk_nrf5340_cpuapp (read/write) 848934 847974 -960 -0.1
text 554652 553704 -948 -0.2
lock-app nrf52840dk_nrf52840 (read/write) 894963 894019 -944 -0.1
text 601980 601032 -948 -0.2
nrf5340dk_nrf5340_cpuapp (read/write) 821234 820274 -960 -0.1
text 532512 531564 -948 -0.2
pump-app nrf52840dk_nrf52840 (read/write) 899915 898971 -944 -0.1
text 605584 604632 -952 -0.2
pump-controller-app nrf52840dk_nrf52840 (read/write) 893123 892179 -944 -0.1
text 600780 599832 -948 -0.2
shell nrf52840dk_nrf52840 (read/write) 782767 782191 -576 -0.1
text 524232 523656 -576 -0.1
nrf5340dk_nrf5340_cpuapp (read/write) 697838 697262 -576 -0.1
text 444868 444296 -572 -0.1
p6 all-clusters-app default (read/write) 2390368 2384224 -6144 -0.3
.text 1348632 1342488 -6144 -0.5
light-app default (read/write) 2330680 2324552 -6128 -0.3
.text 1288944 1282816 -6128 -0.5
lock-app default (read/write) 2302896 2296752 -6144 -0.3
.text 1261160 1255016 -6144 -0.5
qpg lighting-app qpg6105+debug (read only) 533100 532132 -968 -0.2
.text 527780 526812 -968 -0.2
lock-app qpg6105+debug (read only) 504788 503804 -984 -0.2
.text 499468 498484 -984 -0.2
telink lighting-app tlsr9518adk80d (read/write) 831922 830986 -936 -0.1
text 579974 579040 -934 -0.2
Full report (32 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section c939e82 ad8a17d change % change
efr32 lighting-app BRD4161A (read only) 836360 830232 -6128 -0.7
(read/write) 127576 127576 0 0.0
.bss 125696 125696 0 0.0
.data 1876 1876 0 0.0
.text 836352 830224 -6128 -0.7
BRD4161A+rpc (read only) 823964 817836 -6128 -0.7
(read/write) 144248 144248 0 0.0
.bss 142272 142272 0 0.0
.data 1976 1976 0 0.0
.text 823956 817828 -6128 -0.7
window-app BRD4161A (read only) 809720 803592 -6128 -0.8
(read/write) 126512 126512 0 0.0
.bss 124680 124680 0 0.0
.data 1832 1832 0 0.0
.text 809712 803584 -6128 -0.8
esp32 all-clusters-app c3devkit (read only) 876682 875744 -938 -0.1
(read/write) 1313026 1313026 0 0.0
.dram0.bss 70016 70016 0 0.0
.dram0.data 14204 14204 0 0.0
.flash.rodata 175776 175776 0 0.0
.flash.text 876682 875744 -938 -0.1
.iram0.text 62076 62076 0 0.0
m5stack (read only) 962507 961603 -904 -0.1
(read/write) 453784 453784 0 0.0
.dram0.bss 76344 76344 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 211596 211596 0 0.0
.flash.text 957123 956219 -904 -0.1
.iram0.text 123451 123451 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 687048 686088 -960 -0.1
.bss 78280 78280 0 0.0
.data 1908 1908 0 0.0
.text 601060 600100 -960 -0.2
lock-app k32w061+debug (read/write) 634612 633652 -960 -0.2
.bss 76432 76432 0 0.0
.data 1860 1860 0 0.0
.text 550520 549560 -960 -0.2
shell k32w061+debug (read/write) 640016 638912 -1104 -0.2
.bss 78740 78740 0 0.0
.data 1840 1840 0 0.0
.text 553636 552532 -1104 -0.2
linux chip-tool-ipv6only arm64 (read only) 6884788 6873204 -11584 -0.2
(read/write) 322721 322721 0 0.0
.bss 54833 54833 0 0.0
.data 1096 1096 0 0.0
.data.rel.ro 207704 207704 0 0.0
.dynamic 560 560 0 0.0
.got 55384 55384 0 0.0
.init 24 24 0 0.0
.init_array 168 168 0 0.0
.rodata 375460 375460 0 0.0
.text 5824820 5813236 -11584 -0.2
thermostat-no-ble arm64 (read only) 1994756 1993620 -1136 -0.1
(read/write) 144113 144113 0 0.0
.bss 64577 64577 0 0.0
.data 832 832 0 0.0
.data.rel.ro 71976 71976 0 0.0
.dynamic 560 560 0 0.0
.got 3840 3840 0 0.0
.init 24 24 0 0.0
.init_array 288 288 0 0.0
.rodata 127924 127924 0 0.0
.text 1655456 1654320 -1136 -0.1
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2334344 2333384 -960 -0.0
.bss 189268 189268 0 0.0
.data 5256 5256 0 0.0
.heap 841920 841920 0 0.0
.text 1296920 1295960 -960 -0.1
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2329840 2328880 -960 -0.0
.bss 181128 181128 0 0.0
.data 5544 5544 0 0.0
.heap 849776 849776 0 0.0
.text 1292440 1291480 -960 -0.1
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2302936 2301912 -1024 -0.0
.bss 180168 180168 0 0.0
.data 5536 5536 0 0.0
.heap 850744 850744 0 0.0
.text 1265536 1264512 -1024 -0.1
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1140008 1140008 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.heap 1020320 1020320 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2054328 2053688 -640 -0.0
.bss 156980 156980 0 0.0
.data 4864 4864 0 0.0
.heap 874600 874600 0 0.0
.text 1016928 1016288 -640 -0.1
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 922923 921963 -960 -0.1
bss 118788 118788 0 0.0
rodata 104292 104292 0 0.0
text 624212 623264 -948 -0.2
nrf52840dk_nrf52840+rpc (read/write) 885963 885003 -960 -0.1
bss 115140 115140 0 0.0
rodata 95588 95588 0 0.0
text 598952 597988 -964 -0.2
nrf5340dk_nrf5340_cpuapp (read/write) 848934 847974 -960 -0.1
bss 120164 120164 0 0.0
rodata 99552 99552 0 0.0
text 554652 553704 -948 -0.2
lock-app nrf52840dk_nrf52840 (read/write) 894963 894019 -944 -0.1
bss 117968 117968 0 0.0
rodata 99604 99604 0 0.0
text 601980 601032 -948 -0.2
nrf5340dk_nrf5340_cpuapp (read/write) 821234 820274 -960 -0.1
bss 119376 119376 0 0.0
rodata 94892 94892 0 0.0
text 532512 531564 -948 -0.2
pigweed-app nrf52840dk_nrf52840 (read/write) 497447 497447 0 0.0
bss 51820 51820 0 0.0
rodata 45852 45852 0 0.0
text 339488 339488 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 899915 898971 -944 -0.1
bss 117880 117880 0 0.0
rodata 100956 100956 0 0.0
text 605584 604632 -952 -0.2
pump-controller-app nrf52840dk_nrf52840 (read/write) 893123 892179 -944 -0.1
bss 117760 117760 0 0.0
rodata 99092 99092 0 0.0
text 600780 599832 -948 -0.2
shell nrf52840dk_nrf52840 (read/write) 782767 782191 -576 -0.1
bss 109624 109624 0 0.0
rodata 74396 74396 0 0.0
text 524232 523656 -576 -0.1
nrf5340dk_nrf5340_cpuapp (read/write) 697838 697262 -576 -0.1
bss 110604 110604 0 0.0
rodata 69040 69040 0 0.0
text 444868 444296 -572 -0.1
p6 all-clusters-app default (read/write) 2390368 2384224 -6144 -0.3
.bss 117468 117468 0 0.0
.data 2536 2536 0 0.0
.heap 913336 913336 0 0.0
.text 1348632 1342488 -6144 -0.5
light-app default (read/write) 2330680 2324552 -6128 -0.3
.bss 106384 106384 0 0.0
.data 2384 2384 0 0.0
.heap 924576 924576 0 0.0
.text 1288944 1282816 -6128 -0.5
lock-app default (read/write) 2302896 2296752 -6144 -0.3
.bss 105264 105264 0 0.0
.data 2336 2336 0 0.0
.heap 925744 925744 0 0.0
.text 1261160 1255016 -6144 -0.5
qpg lighting-app qpg6105+debug (read only) 533100 532132 -968 -0.2
(read/write) 146936 146936 0 0.0
.bss 87032 87032 0 0.0
.data 1004 1004 0 0.0
.text 527780 526812 -968 -0.2
lock-app qpg6105+debug (read only) 504788 503804 -984 -0.2
(read/write) 146940 146940 0 0.0
.bss 86168 86168 0 0.0
.data 952 952 0 0.0
.text 499468 498484 -984 -0.2
persistent-storage-app qpg6105+debug (read only) 106448 106448 0 0.0
(read/write) 146938 146938 0 0.0
.bss 36146 36146 0 0.0
.data 288 288 0 0.0
.text 101128 101128 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 831922 830986 -936 -0.1
bss 87272 87272 0 0.0
noinit 37160 37160 0 0.0
text 579974 579040 -934 -0.2

@Damian-Nordic Damian-Nordic merged commit 9a595a4 into project-chip:master Dec 16, 2021
@mlepage-google mlepage-google deleted the fix-session-handle-args branch December 16, 2021 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmeg-sfy jmeg-sfy jmeg-sfy approved these changes

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@msandstedt msandstedt msandstedt approved these changes

@kghost kghost kghost approved these changes

@turon turon turon approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

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

@austinh0 austinh0 Awaiting requested review from austinh0

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

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

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

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

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@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

@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

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

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

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

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@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
app controller review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mlepage-google @turon @kghost @msandstedt @Damian-Nordic @jmeg-sfy