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

[thread/cleanup] Change GetThreadProvision to return OperationalDataset #17054

Merged
merged 2 commits into from
Apr 5, 2022
Merged

[thread/cleanup] Change GetThreadProvision to return OperationalDataset #17054

merged 2 commits into from
Apr 5, 2022

Conversation

Damian-Nordic
Copy link
Contributor

Problem

OpenThread ThreadStackManager has OperationalDataset member only used as a temporary object for returning a ByteSpan in GetThreadProvision method and all the method users covert it to OperationalDataset, anyway.

Change overview

Change the interface to simplify the code and save >250 bytes of RAM.
Additionally, add bound checks in OperationalDataset::GetExtendedPanId methods.

Testing

Verified commissioning still works on nRF Connect devices.

@Damian-Nordic
[thread] Change GetThreadProvision to return OperationalDataset
b912ff1 
OpenThread stack manager has OperationalDataset member only
used as a temporary object for returning an operational
dataset as ByteSpan in GetThreadProvision method. Change the
interface to save >250 bytes of RAM.

Additionally, add bound checks in
OperationalDataset::GetExtendedPanId methods.
@restyled-io restyled-io bot mentioned this pull request Apr 5, 2022
Closed
@github-actions
Copy link

github-actions bot commented Apr 5, 2022 •
edited
Loading

PR #17054: Size comparison from fae3888 to f5f3237

Increases (8 builds for linux)
platform target config section fae3888 f5f3237 change % change
linux all-clusters-app debug (read only) 2595705 2595721 16 0.0
.text 2206402 2206418 16 0.0
chip-tool debug (read only) 10601045 10601093 48 0.0
.text 9238725 9238773 48 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 10213932 10213996 64 0.0
.text 8604996 8605060 64 0.0
door-lock-app debug (read only) 2075777 2075969 192 0.0
.text 1736450 1736642 192 0.0
lighting-app debug+rpc (read only) 2255289 2255481 192 0.0
.text 1912386 1912578 192 0.0
ota-provider-app debug (read only) 2013697 2013889 192 0.0
.text 1687570 1687762 192 0.0
ota-requestor-app debug (read only) 2043145 2043321 176 0.0
.text 1718850 1719026 176 0.0
thermostat-no-ble arm64 (read only) 2326940 2327068 128 0.0
.text 1957760 1957888 128 0.0
Decreases (12 builds for cc13x2_26x2, efr32, k32w, linux, nrfconnect, telink)
platform target config section fae3888 f5f3237 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 670895 670799 -96 -0.0
(read/write) 180824 180664 -160 -0.1
.bss 81760 81504 -256 -0.3
.text 590376 590280 -96 -0.0
lock-mtd LP_CC2652R7 (read only) 620023 619927 -96 -0.0
(read/write) 154508 154252 -256 -0.2
.bss 77488 77232 -256 -0.3
.text 539608 539512 -96 -0.0
pump-app LP_CC2652R7 (read only) 690303 690207 -96 -0.0
(read/write) 162592 162432 -160 -0.1
.bss 82168 81912 -256 -0.3
.text 607700 607604 -96 -0.0
pump-controller-app LP_CC2652R7 (read only) 672543 672447 -96 -0.0
(read/write) 180080 179920 -160 -0.1
.bss 81896 81640 -256 -0.3
.text 593604 593508 -96 -0.0
efr32 lighting-app BRD4161A (read only) 919660 919532 -128 -0.0
(read/write) 129792 129536 -256 -0.2
.bss 127800 127544 -256 -0.2
.text 919652 919524 -128 -0.0
BRD4161A+rpc (read only) 947572 947428 -144 -0.0
(read/write) 145740 145484 -256 -0.2
.bss 143568 143312 -256 -0.2
.text 947564 947420 -144 -0.0
window-app BRD4161A (read only) 854860 854732 -128 -0.0
(read/write) 127816 127560 -256 -0.2
.bss 125944 125688 -256 -0.2
.text 854852 854724 -128 -0.0
k32w light k32w061+release (read/write) 710780 710428 -352 -0.0
.bss 77968 77712 -256 -0.3
.text 625108 625012 -96 -0.0
lock k32w061+release (read/write) 710168 709816 -352 -0.0
.bss 77960 77704 -256 -0.3
.text 624464 624368 -96 -0.0
linux tv-app debug (read only) 2764905 2764873 -32 -0.0
.text 2374610 2374578 -32 -0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1149827 1149491 -336 -0.0
bss 135812 135556 -256 -0.2
text 790312 790220 -92 -0.0
telink lighting-app tlsr9518adk80d (read/write) 795232 794888 -344 -0.0
bss 70300 70044 -256 -0.4
text 564698 564606 -92 -0.0
Full report (28 builds for cc13x2_26x2, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section fae3888 f5f3237 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 670895 670799 -96 -0.0
(read/write) 180824 180664 -160 -0.1
.bss 81760 81504 -256 -0.3
.data 3164 3164 0 0.0
.rodata 80039 80039 0 0.0
.text 590376 590280 -96 -0.0
lock-mtd LP_CC2652R7 (read only) 620023 619927 -96 -0.0
(read/write) 154508 154252 -256 -0.2
.bss 77488 77232 -256 -0.3
.data 3164 3164 0 0.0
.rodata 79927 79927 0 0.0
.text 539608 539512 -96 -0.0
pump-app LP_CC2652R7 (read only) 690303 690207 -96 -0.0
(read/write) 162592 162432 -160 -0.1
.bss 82168 81912 -256 -0.3
.data 3196 3196 0 0.0
.rodata 82119 82119 0 0.0
.text 607700 607604 -96 -0.0
pump-controller-app LP_CC2652R7 (read only) 672543 672447 -96 -0.0
(read/write) 180080 179920 -160 -0.1
.bss 81896 81640 -256 -0.3
.data 3160 3160 0 0.0
.rodata 78455 78455 0 0.0
.text 593604 593508 -96 -0.0
efr32 lighting-app BRD4161A (read only) 919660 919532 -128 -0.0
(read/write) 129792 129536 -256 -0.2
.bss 127800 127544 -256 -0.2
.data 1992 1992 0 0.0
.text 919652 919524 -128 -0.0
BRD4161A+rpc (read only) 947572 947428 -144 -0.0
(read/write) 145740 145484 -256 -0.2
.bss 143568 143312 -256 -0.2
.data 2172 2172 0 0.0
.text 947564 947420 -144 -0.0
window-app BRD4161A (read only) 854860 854732 -128 -0.0
(read/write) 127816 127560 -256 -0.2
.bss 125944 125688 -256 -0.2
.data 1872 1872 0 0.0
.text 854852 854724 -128 -0.0
esp32 all-clusters-app c3devkit (read only) 988422 988422 0 0.0
(read/write) 1460922 1460922 0 0.0
.dram0.bss 62952 62952 0 0.0
.dram0.data 14196 14196 0 0.0
.flash.rodata 198288 198288 0 0.0
.flash.text 988422 988422 0 0.0
.iram0.text 62572 62572 0 0.0
m5stack (read only) 1040787 1040787 0 0.0
(read/write) 461912 461912 0 0.0
.dram0.bss 68480 68480 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 227232 227232 0 0.0
.flash.text 1035403 1035403 0 0.0
.iram0.text 123415 123415 0 0.0
k32w light k32w061+release (read/write) 710780 710428 -352 -0.0
.bss 77968 77712 -256 -0.3
.data 1904 1904 0 0.0
.text 625108 625012 -96 -0.0
lock k32w061+release (read/write) 710168 709816 -352 -0.0
.bss 77960 77704 -256 -0.3
.data 1944 1944 0 0.0
.text 624464 624368 -96 -0.0
linux all-clusters-app debug (read only) 2595705 2595721 16 0.0
(read/write) 144872 144872 0 0.0
.bss 57664 57664 0 0.0
.data 1440 1440 0 0.0
.data.rel.ro 79880 79880 0 0.0
.dynamic 592 592 0 0.0
.got 4312 4312 0 0.0
.init 27 27 0 0.0
.init_array 960 960 0 0.0
.rodata 221285 221285 0 0.0
.text 2206402 2206418 16 0.0
bridge-app debug+rpc (read only) 1804485 1804485 0 0.0
(read/write) 90296 90296 0 0.0
.bss 44584 44584 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 38584 38584 0 0.0
.dynamic 592 592 0 0.0
.got 3928 3928 0 0.0
.init 27 27 0 0.0
.init_array 552 552 0 0.0
.rodata 147513 147513 0 0.0
.text 1540341 1540341 0 0.0
chip-tool debug (read only) 10601045 10601093 48 0.0
(read/write) 369752 369752 0 0.0
.bss 22016 22016 0 0.0
.data 1040 1040 0 0.0
.data.rel.ro 340464 340464 0 0.0
.dynamic 624 624 0 0.0
.got 4920 4920 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 536109 536109 0 0.0
.text 9238725 9238773 48 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 10213932 10213996 64 0.0
(read/write) 489633 489633 0 0.0
.bss 40337 40337 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 387360 387360 0 0.0
.dynamic 560 560 0 0.0
.got 57016 57016 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 510172 510172 0 0.0
.text 8604996 8605060 64 0.0
door-lock-app debug (read only) 2075777 2075969 192 0.0
(read/write) 118320 118320 0 0.0
.bss 47904 47904 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 63704 63704 0 0.0
.dynamic 592 592 0 0.0
.got 4256 4256 0 0.0
.init 27 27 0 0.0
.init_array 680 680 0 0.0
.rodata 185513 185513 0 0.0
.text 1736450 1736642 192 0.0
lighting-app debug+rpc (read only) 2255289 2255481 192 0.0
(read/write) 125312 125312 0 0.0
.bss 49216 49216 0 0.0
.data 1600 1600 0 0.0
.data.rel.ro 68808 68808 0 0.0
.dynamic 608 608 0 0.0
.got 4304 4304 0 0.0
.init 27 27 0 0.0
.init_array 760 760 0 0.0
.rodata 179625 179625 0 0.0
.text 1912386 1912578 192 0.0
ota-provider-app debug (read only) 2013697 2013889 192 0.0
(read/write) 113888 113888 0 0.0
.bss 47744 47744 0 0.0
.data 1384 1384 0 0.0
.data.rel.ro 59032 59032 0 0.0
.dynamic 608 608 0 0.0
.got 4456 4456 0 0.0
.init 27 27 0 0.0
.init_array 632 632 0 0.0
.rodata 171403 171403 0 0.0
.text 1687570 1687762 192 0.0
ota-requestor-app debug (read only) 2043145 2043321 176 0.0
(read/write) 117176 117176 0 0.0
.bss 48736 48736 0 0.0
.data 1608 1608 0 0.0
.data.rel.ro 61240 61240 0 0.0
.dynamic 592 592 0 0.0
.got 4296 4296 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 167892 167892 0 0.0
.text 1718850 1719026 176 0.0
shell debug (read only) 2491433 2491433 0 0.0
(read/write) 148336 148336 0 0.0
.bss 67336 67336 0 0.0
.data 848 848 0 0.0
.data.rel.ro 74424 74424 0 0.0
.dynamic 592 592 0 0.0
.got 4160 4160 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 212978 212978 0 0.0
.text 2119922 2119922 0 0.0
thermostat-no-ble arm64 (read only) 2326940 2327068 128 0.0
(read/write) 149521 149521 0 0.0
.bss 62977 62977 0 0.0
.data 1136 1136 0 0.0
.data.rel.ro 77760 77760 0 0.0
.dynamic 560 560 0 0.0
.got 4632 4632 0 0.0
.init 24 24 0 0.0
.init_array 368 368 0 0.0
.rodata 143340 143340 0 0.0
.text 1957760 1957888 128 0.0
tv-app debug (read only) 2764905 2764873 -32 -0.0
(read/write) 249984 249984 0 0.0
.bss 165360 165360 0 0.0
.data 3392 3392 0 0.0
.data.rel.ro 75040 75040 0 0.0
.dynamic 592 592 0 0.0
.got 4672 4672 0 0.0
.init 27 27 0 0.0
.init_array 904 904 0 0.0
.rodata 211467 211467 0 0.0
.text 2374610 2374578 -32 -0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2363084 2363084 0 0.0
.bss 185052 185052 0 0.0
.data 5784 5784 0 0.0
.text 1325684 1325684 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1149827 1149491 -336 -0.0
bss 135812 135556 -256 -0.2
rodata 145084 145084 0 0.0
text 790312 790220 -92 -0.0
p6 all-clusters-app default (read/write) 2507928 2507928 0 0.0
.bss 118480 118480 0 0.0
.data 2672 2672 0 0.0
.text 1466192 1466192 0 0.0
light-app default (read/write) 2409120 2409120 0 0.0
.bss 111944 111944 0 0.0
.data 2528 2528 0 0.0
.text 1367384 1367384 0 0.0
lock-app default (read/write) 2372816 2372816 0 0.0
.bss 111688 111688 0 0.0
.data 2488 2488 0 0.0
.text 1331080 1331080 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 795232 794888 -344 -0.0
bss 70300 70044 -256 -0.4
noinit 40416 40416 0 0.0
text 564698 564606 -92 -0.0

@bzbarsky-apple bzbarsky-apple merged commit a957a5f into project-chip:master Apr 5, 2022
chencheung pushed a commit to chencheung/connectedhomeip that referenced this pull request Apr 6, 2022
@Damian-Nordic @restyled-commits @chencheung
[thread/cleanup] Change GetThreadProvision to return OperationalDatas…
d9d00bf 
…et (project-chip#17054)

* [thread] Change GetThreadProvision to return OperationalDataset

OpenThread stack manager has OperationalDataset member only
used as a temporary object for returning an operational
dataset as ByteSpan in GetThreadProvision method. Change the
interface to save >250 bytes of RAM.

Additionally, add bound checks in
OperationalDataset::GetExtendedPanId methods.

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
chencheung pushed a commit to chencheung/connectedhomeip that referenced this pull request Apr 6, 2022
@Damian-Nordic @restyled-commits @chencheung
[thread/cleanup] Change GetThreadProvision to return OperationalDatas…
76a63b8 
…et (project-chip#17054)

* [thread] Change GetThreadProvision to return OperationalDataset

OpenThread stack manager has OperationalDataset member only
used as a temporary object for returning an operational
dataset as ByteSpan in GetThreadProvision method. Change the
interface to save >250 bytes of RAM.

Additionally, add bound checks in
OperationalDataset::GetExtendedPanId methods.

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
@Damian-Nordic @restyled-commits @andrei-menzopol
[thread/cleanup] Change GetThreadProvision to return OperationalDatas…
55ee01d 
…et (project-chip#17054)

* [thread] Change GetThreadProvision to return OperationalDataset

OpenThread stack manager has OperationalDataset member only
used as a temporary object for returning an operational
dataset as ByteSpan in GetThreadProvision method. Change the
interface to save >250 bytes of RAM.

Additionally, add bound checks in
OperationalDataset::GetExtendedPanId methods.

* Restyled by clang-format

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

@jmartinez-silabs jmartinez-silabs jmartinez-silabs approved these changes

@woody-apple woody-apple woody-apple 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

@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

@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

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

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

@kghost kghost Awaiting requested review from kghost

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@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

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

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

@wbschiller wbschiller Awaiting requested review from wbschiller

@xylophone21 xylophone21 Awaiting requested review from xylophone21

Assignees
No one assigned
Labels
lib linux openthread platform review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Damian-Nordic @woody-apple @jmartinez-silabs @bzbarsky-apple @restyled-commits