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

Change how the Matter framework handles Platform::Memory. #19273

Merged
merged 1 commit into from
Jun 8, 2022
Merged

Change how the Matter framework handles Platform::Memory. #19273

merged 1 commit into from
Jun 8, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

Since for the malloc case the Platform::Memory is just debugging bits,
and since its init is not particularly threadsafe, just do a
dispatch_once init whenever we might need it, and don't worry about
shutting it down.

Problem

See above.

Change overview

See above.

Testing

Ran Darwin framework tests.

@bzbarsky-apple
Change how the Matter framework handles Platform::Memory.
8a2182b 
Since for the malloc case the Platform::Memory is just debugging bits,
and since its init is not particularly threadsafe, just do a
dispatch_once init whenever we might need it, and don't worry about
shutting it down.
@boring-cyborg boring-cyborg bot added the darwin label Jun 7, 2022
@github-actions
Copy link

github-actions bot commented Jun 7, 2022 •
edited
Loading

PR #19273: Size comparison from 8cc291d to 8a2182b

Increases (1 build for telink)
platform target config section 8cc291d 8a2182b change % change
telink light-switch-app tlsr9518adk80d text 554350 554352 2 0.0
Decreases (1 build for nrfconnect)
platform target config section 8cc291d 8a2182b change % change
nrfconnect all-clusters-app nrf52840dk_nrf52840 text 818812 818808 -4 -0.0
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 8cc291d 8a2182b change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 692263 692263 0 0.0
(read/write) 159592 159592 0 0.0
.bss 74748 74748 0 0.0
.data 3408 3408 0 0.0
.rodata 112719 112719 0 0.0
.text 579232 579232 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 642887 642887 0 0.0
(read/write) 158228 158228 0 0.0
.bss 73964 73964 0 0.0
.data 3348 3348 0 0.0
.rodata 89759 89759 0 0.0
.text 552808 552808 0 0.0
lock-ftd LP_CC2652R7 (read only) 685783 685783 0 0.0
(read/write) 157136 157136 0 0.0
.bss 72700 72700 0 0.0
.data 3272 3272 0 0.0
.rodata 98583 98583 0 0.0
.text 586716 586716 0 0.0
lock-mtd LP_CC2652R7 (read only) 635199 635199 0 0.0
(read/write) 145824 145824 0 0.0
.bss 68436 68436 0 0.0
.data 3272 3272 0 0.0
.rodata 98471 98471 0 0.0
.text 536236 536236 0 0.0
pump-app LP_CC2652R7 (read only) 666827 666827 0 0.0
(read/write) 176996 176996 0 0.0
.bss 72836 72836 0 0.0
.data 3304 3304 0 0.0
.rodata 86771 86771 0 0.0
.text 579572 579572 0 0.0
pump-controller-app LP_CC2652R7 (read only) 657979 657979 0 0.0
(read/write) 185956 185956 0 0.0
.bss 72948 72948 0 0.0
.data 3268 3268 0 0.0
.rodata 84819 84819 0 0.0
.text 572676 572676 0 0.0
shell LP_CC2652R7 (read only) 684550 684550 0 0.0
(read/write) 162800 162800 0 0.0
.bss 77044 77044 0 0.0
.data 3412 3412 0 0.0
.rodata 109318 109318 0 0.0
.text 574920 574920 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 604210 604210 0 0.0
.app_xip_area 463140 463140 0 0.0
.bss 84008 84008 0 0.0
.data 748 748 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 601270 601270 0 0.0
.app_xip_area 460064 460064 0 0.0
.bss 84176 84176 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 599382 599382 0 0.0
.app_xip_area 459300 459300 0 0.0
.bss 83140 83140 0 0.0
.data 624 624 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 919152 919152 0 0.0
(read/write) 133280 133280 0 0.0
.bss 131176 131176 0 0.0
.data 2104 2104 0 0.0
.text 919144 919144 0 0.0
BRD4161A+rpc (read only) 953380 953380 0 0.0
(read/write) 149972 149972 0 0.0
.bss 147664 147664 0 0.0
.data 2308 2308 0 0.0
.text 953372 953372 0 0.0
BRD4161A+rs911x (read only) 794436 794436 0 0.0
(read/write) 129556 129556 0 0.0
.bss 127444 127444 0 0.0
.data 2112 2112 0 0.0
.text 794428 794428 0 0.0
lock-app BRD4161A+wf200 (read only) 960888 960888 0 0.0
(read/write) 128348 128348 0 0.0
.bss 126268 126268 0 0.0
.data 2080 2080 0 0.0
.text 960880 960880 0 0.0
window-app BRD4161A (read only) 904184 904184 0 0.0
(read/write) 133368 133368 0 0.0
.bss 131264 131264 0 0.0
.data 2100 2100 0 0.0
.text 904176 904176 0 0.0
esp32 all-clusters-app c3devkit (read only) 1009610 1009610 0 0.0
(read/write) 1482530 1482530 0 0.0
.dram0.bss 69256 69256 0 0.0
.dram0.data 14672 14672 0 0.0
.flash.rodata 213168 213168 0 0.0
.flash.text 1009610 1009610 0 0.0
.iram0.text 62954 62954 0 0.0
m5stack (read only) 1064455 1064455 0 0.0
(read/write) 484640 484640 0 0.0
.dram0.bss 74768 74768 0 0.0
.dram0.data 34216 34216 0 0.0
.flash.rodata 243660 243660 0 0.0
.flash.text 1059071 1059071 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 655688 655688 0 0.0
.bss 70052 70052 0 0.0
.data 2020 2020 0 0.0
.text 577816 577816 0 0.0
lock k32w061+release (read/write) 717196 717196 0 0.0
.bss 70492 70492 0 0.0
.data 1992 1992 0 0.0
.text 638912 638912 0 0.0
linux all-clusters-app debug (read only) 2793841 2793841 0 0.0
(read/write) 178768 178768 0 0.0
.bss 86432 86432 0 0.0
.data 2032 2032 0 0.0
.data.rel.ro 84120 84120 0 0.0
.dynamic 608 608 0 0.0
.got 4496 4496 0 0.0
.init 27 27 0 0.0
.init_array 1032 1032 0 0.0
.rodata 248093 248093 0 0.0
.text 2371554 2371554 0 0.0
all-clusters-minimal-app debug (read only) 2669465 2669465 0 0.0
(read/write) 170608 170608 0 0.0
.bss 85600 85600 0 0.0
.data 1904 1904 0 0.0
.data.rel.ro 76920 76920 0 0.0
.dynamic 608 608 0 0.0
.got 4496 4496 0 0.0
.init 27 27 0 0.0
.init_array 1032 1032 0 0.0
.rodata 249789 249789 0 0.0
.text 2246658 2246658 0 0.0
bridge-app debug+rpc (read only) 2047073 2047073 0 0.0
(read/write) 148160 148160 0 0.0
.bss 72960 72960 0 0.0
.data 3936 3936 0 0.0
.data.rel.ro 65672 65672 0 0.0
.dynamic 592 592 0 0.0
.got 4272 4272 0 0.0
.init 27 27 0 0.0
.init_array 696 696 0 0.0
.rodata 171712 171712 0 0.0
.text 1718530 1718530 0 0.0
chip-tool debug (read only) 9673373 9673373 0 0.0
(read/write) 606584 606584 0 0.0
.bss 23968 23968 0 0.0
.data 1120 1120 0 0.0
.data.rel.ro 575192 575192 0 0.0
.dynamic 624 624 0 0.0
.got 5016 5016 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 498941 498941 0 0.0
.text 7772341 7772341 0 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 9420476 9420476 0 0.0
(read/write) 672577 672577 0 0.0
.bss 42257 42257 0 0.0
.data 1176 1176 0 0.0
.data.rel.ro 610400 610400 0 0.0
.dynamic 528 528 0 0.0
.got 14920 14920 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 461900 461900 0 0.0
.text 7421764 7421764 0 0.0
lighting-app debug+rpc (read only) 2337329 2337329 0 0.0
(read/write) 153800 153800 0 0.0
.bss 74816 74816 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 71176 71176 0 0.0
.dynamic 608 608 0 0.0
.got 4344 4344 0 0.0
.init 27 27 0 0.0
.init_array 800 800 0 0.0
.rodata 190632 190632 0 0.0
.text 1980610 1980610 0 0.0
lock-app debug (read only) 2280553 2280553 0 0.0
(read/write) 148936 148936 0 0.0
.bss 73536 73536 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 68120 68120 0 0.0
.dynamic 592 592 0 0.0
.got 4336 4336 0 0.0
.init 27 27 0 0.0
.init_array 768 768 0 0.0
.rodata 204808 204808 0 0.0
.text 1914946 1914946 0 0.0
ota-provider-app debug (read only) 2081401 2081401 0 0.0
(read/write) 141816 141816 0 0.0
.bss 73088 73088 0 0.0
.data 1768 1768 0 0.0
.data.rel.ro 61160 61160 0 0.0
.dynamic 608 608 0 0.0
.got 4504 4504 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 183128 183128 0 0.0
.text 1738866 1738866 0 0.0
ota-requestor-app debug (read only) 2109465 2109465 0 0.0
(read/write) 144624 144624 0 0.0
.bss 73792 73792 0 0.0
.data 1960 1960 0 0.0
.data.rel.ro 63224 63224 0 0.0
.dynamic 592 592 0 0.0
.got 4344 4344 0 0.0
.init 27 27 0 0.0
.init_array 680 680 0 0.0
.rodata 179040 179040 0 0.0
.text 1769554 1769554 0 0.0
shell debug (read only) 2598681 2598681 0 0.0
(read/write) 202240 202240 0 0.0
.bss 117320 117320 0 0.0
.data 1376 1376 0 0.0
.data.rel.ro 77776 77776 0 0.0
.dynamic 608 608 0 0.0
.got 4192 4192 0 0.0
.init 27 27 0 0.0
.init_array 944 944 0 0.0
.rodata 228146 228146 0 0.0
.text 2210850 2210850 0 0.0
thermostat-no-ble arm64 (read only) 2381516 2381516 0 0.0
(read/write) 177793 177793 0 0.0
.bss 87985 87985 0 0.0
.data 1536 1536 0 0.0
.data.rel.ro 80440 80440 0 0.0
.dynamic 528 528 0 0.0
.got 4816 4816 0 0.0
.init 24 24 0 0.0
.init_array 384 384 0 0.0
.rodata 149948 149948 0 0.0
.text 2001504 2001504 0 0.0
tv-app debug (read only) 2912097 2912097 0 0.0
(read/write) 280536 280536 0 0.0
.bss 191112 191112 0 0.0
.data 4672 4672 0 0.0
.data.rel.ro 78472 78472 0 0.0
.dynamic 592 592 0 0.0
.got 4728 4728 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 224992 224992 0 0.0
.text 2504114 2504114 0 0.0
tv-casting-app debug (read only) 5345265 5345265 0 0.0
(read/write) 222936 222936 0 0.0
.bss 78696 78696 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 135584 135584 0 0.0
.dynamic 608 608 0 0.0
.got 4728 4728 0 0.0
.init 27 27 0 0.0
.init_array 872 872 0 0.0
.rodata 339264 339264 0 0.0
.text 4654498 4654498 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2431976 2431976 0 0.0
.bss 202716 202716 0 0.0
.data 5888 5888 0 0.0
.text 1394620 1394620 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1194327 1194327 0 0.0
bss 141385 141385 0 0.0
rodata 155216 155216 0 0.0
text 818812 818808 -4 -0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1142035 1142035 0 0.0
bss 140602 140602 0 0.0
rodata 131708 131708 0 0.0
text 790864 790864 0 0.0
p6 all-clusters-app default (read/write) 2548792 2548792 0 0.0
.bss 137208 137208 0 0.0
.data 2816 2816 0 0.0
.text 1507056 1507056 0 0.0
all-clusters-minimal-app default (read/write) 2493648 2493648 0 0.0
.bss 136424 136424 0 0.0
.data 2760 2760 0 0.0
.text 1451912 1451912 0 0.0
light-app default (read/write) 2425552 2425552 0 0.0
.bss 129520 129520 0 0.0
.data 2608 2608 0 0.0
.text 1383816 1383816 0 0.0
lock-app default (read/write) 2445856 2445856 0 0.0
.bss 129344 129344 0 0.0
.data 2584 2584 0 0.0
.text 1404120 1404120 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 784080 784080 0 0.0
bss 70652 70652 0 0.0
noinit 40416 40416 0 0.0
text 554350 554352 2 0.0
lighting-app tlsr9518adk80d (read/write) 804124 804124 0 0.0
bss 70912 70912 0 0.0
noinit 40416 40416 0 0.0
text 571092 571092 0 0.0

@mrjerryjohns
Copy link
Contributor

since its init is not particularly threadsafe

Don't both Memory::Init and Memory::Shutdown expect external synchronization? If this was sequenced correctly on the same dispatch queue, thread safety shouldn't be a concern?

The PR looks fine, but wanted to ensure my understanding of expectations there were correct.

@bzbarsky-apple
Copy link
Contributor Author

Don't both Memory::Init and Memory::Shutdown expect external synchronization?

That's ... extremely unclear. They also have atomic refcounting, which makes no sense with external synchronization. See also #18675

If this was sequenced correctly on the same dispatch queue, thread safety shouldn't be a concern?

Yes, and I considered doing just that (a special dispatch queue with dispatch_sync just for Platform::Memory), but decided that it just wasn't worth the complexity.

@bzbarsky-apple bzbarsky-apple merged commit fbce051 into project-chip:master Jun 8, 2022
@bzbarsky-apple bzbarsky-apple deleted the darwin-platform-memory branch June 8, 2022 01:30
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Jun 10, 2022
@bzbarsky-apple @mkardous-silabs
Change how the Matter framework handles Platform::Memory. (project-ch…
b286d6f 
…ip#19273)

Since for the malloc case the Platform::Memory is just debugging bits,
and since its init is not particularly threadsafe, just do a
dispatch_once init whenever we might need it, and don't worry about
shutting it down.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrjerryjohns mrjerryjohns mrjerryjohns approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

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

@arkq arkq Awaiting requested review from arkq

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

@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

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

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@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

@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

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

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@kghost kghost Awaiting requested review from kghost

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mspang mspang Awaiting requested review from mspang

@rgoliver rgoliver Awaiting requested review from rgoliver

@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

@tehampson tehampson Awaiting requested review from tehampson

@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

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

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

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

@msandstedt msandstedt Awaiting requested review from msandstedt

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

Successfully merging this pull request may close these issues.
3 participants
@bzbarsky-apple @mrjerryjohns @msandstedt