connectedhomeip
Make chip::Optional be trivially destructible if the underlying type is.
#37017
Merged
Overview
Commits
5
Changes
View On GitHub

Make chip::Optional be trivially destructible if the underlying type is. #37017

mergify merged 5 commits into project-chip:master from andy31415:optional-trivial-destructible
andy31415
andy3141597 days ago

Previous implementation always had a destructor, so it was never trivially destructible.

Testing

  • Added asserts on trivial destructability in the TestOptional file
  • Unit tests are still expected to pass (TestOptional covers construction/destruction of non-trivial types)
  • Validated in a separate PR that SemanticTagStruct type becomes trivially destructible.
andreilitvin Make chip::Optional be trivially destructible if the underlying type is.
1d5e7182
semanticdiff-com
semanticdiff-com97 days ago

Review changes with  SemanticDiff

pullapprove pullapprove added review - pending
github-actions github-actions added lib
github-actions github-actions added core
github-actions
github-actions97 days ago (edited 97 days ago)

PR #37017: Size comparison from b83c27a to 1d5e718

Increases above 0.2%:

platform target config section b83c27a 1d5e718 change % change
efr32 lock-app BRD4338a FLASH 747208 749248 2040 0.3
Full report (29 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, qpg, stm32, telink)
platform target config section b83c27a 1d5e718 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1354956 1355220 264 0.0
RAM 104152 104144 -8 -0.0
bl702 lighting-app bl702+eth FLASH 726512 726030 -482 -0.1
RAM 25361 25353 -8 -0.0
bl702+wifi FLASH 913126 912900 -226 -0.0
RAM 14101 14093 -8 -0.1
bl706+mfd+rpc+littlefs FLASH 1173960 1173758 -202 -0.0
RAM 23941 23933 -8 -0.0
bl702l lighting-app bl702l+mfd+littlefs FLASH 1083028 1082786 -242 -0.0
RAM 16612 16604 -8 -0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 840256 840184 -72 -0.0
RAM 123696 123696 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 825796 825676 -120 -0.0
RAM 125584 125584 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 772632 772496 -136 -0.0
RAM 114060 114060 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 756812 756692 -120 -0.0
RAM 114260 114260 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 540113 539997 -116 -0.0
RAM 205800 205800 0 0.0
lock CC3235SF_LAUNCHXL FLASH 574265 574165 -100 -0.0
RAM 205944 205944 0 0.0
efr32 lock-app BRD4187C FLASH 932740 932612 -128 -0.0
RAM 160228 160228 0 0.0
BRD4338a FLASH 747208 749248 2040 0.3
RAM 233356 233356 0 0.0
window-app BRD4187C FLASH 1025656 1026904 1248 0.1
RAM 128332 128332 0 0.0
esp32 all-clusters-app c3devkit DRAM 95352 95352 0 0.0
FLASH 1541996 1542032 36 0.0
IRAM 82552 82552 0 0.0
m5stack DRAM 116332 116332 0 0.0
FLASH 1548218 1548286 68 0.0
IRAM 117039 117039 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 918156 917884 -272 -0.0
RAM 143332 143332 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 890664 890832 168 0.0
RAM 141519 141519 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 852220 852004 -216 -0.0
RAM 142244 142244 0 0.0
qpg lighting-app qpg6105+debug FLASH 664392 664272 -120 -0.0
RAM 105456 105456 0 0.0
lock-app qpg6105+debug FLASH 622204 622100 -104 -0.0
RAM 99908 99908 0 0.0
stm32 light STM32WB5MM-DK FLASH 485136 485040 -96 -0.0
RAM 144912 144912 0 0.0
telink bridge-app tlsr9258a FLASH 683698 683584 -114 -0.0
RAM 91248 91248 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 623938 623814 -124 -0.0
RAM 31488 31488 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 772772 772672 -100 -0.0
RAM 49348 49348 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 777388 777288 -100 -0.0
RAM 99812 99812 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 711380 711268 -112 -0.0
RAM 73544 73544 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 628384 628284 -100 -0.0
RAM 142180 142180 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 814398 814298 -100 -0.0
RAM 99724 99724 0 0.0
bzbarsky-apple
bzbarsky-apple commented on 2025-01-09
Conversation is marked as resolved
Show resolved
src/lib/core/Optional.h
251 union Value
252 {
253 Value() {}
254
 char _unused;
bzbarsky-apple97 days ago

Please document why that char is there. Because seems like it just makes things larger when T is a one-byte thing or something....

andy3141597 days ago

Remnant from when I was investigating how to make this code bend to my will (given that I cannot SFNIAE destructors). Removed the unused bits. Size-wise I believe chars cannot make things larger because values in C++ need unique addresses so they have no 0-sized structures (and this is a union, so it should have at least 1 byte of storage...). In any case, gone and tests still pass.

andy31415 Remove extra char
e6e42f10
andy31415 Remove unused variable
6f3b85ea
github-actions github-actions added app
andy31415 Merge branch 'master' into optional-trivial-destructible
a503615c
github-actions
github-actions97 days ago (edited 97 days ago)

PR #37017: Size comparison from e7082e2 to a503615

Full report (11 builds for cc13x4_26x4, cc32xx, qpg, stm32, tizen)
platform target config section e7082e2 a503615 change % change
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 840400 840320 -80 -0.0
RAM 123712 123712 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 825932 825804 -128 -0.0
RAM 125600 125600 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 772836 772700 -136 -0.0
RAM 114076 114076 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 757016 756912 -104 -0.0
RAM 114276 114276 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 540285 540169 -116 -0.0
RAM 205816 205816 0 0.0
lock CC3235SF_LAUNCHXL FLASH 574453 574337 -116 -0.0
RAM 205960 205960 0 0.0
qpg lighting-app qpg6105+debug FLASH 664392 664272 -120 -0.0
RAM 105456 105456 0 0.0
lock-app qpg6105+debug FLASH 622204 622100 -104 -0.0
RAM 99908 99908 0 0.0
stm32 light STM32WB5MM-DK FLASH 485136 485048 -88 -0.0
RAM 144912 144912 0 0.0
tizen all-clusters-app arm unknown 5160 5120 -40 -0.8
FLASH 1781748 1767248 -14500 -0.8
RAM 93720 93708 -12 -0.0
chip-tool-ubsan arm unknown 10924 10904 -20 -0.2
FLASH 18121646 17948374 -173272 -1.0
RAM 7909212 7841128 -68084 -0.9
andy31415 Remove one more unused variable
6eb101b6
github-actions github-actions added examples
github-actions
github-actions97 days ago (edited 97 days ago)

PR #37017: Size comparison from e7082e2 to 6eb101b

Increases above 0.2%:

platform target config section e7082e2 6eb101b change % change
efr32 lock-app BRD4338a FLASH 747208 749248 2040 0.3
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1647684 1652132 4448 0.3
Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section e7082e2 6eb101b change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1354946 1355206 260 0.0
RAM 104152 104144 -8 -0.0
bl702 lighting-app bl702+eth FLASH 726512 726022 -490 -0.1
RAM 25361 25353 -8 -0.0
bl702+wifi FLASH 913126 912892 -234 -0.0
RAM 14101 14093 -8 -0.1
bl706+mfd+rpc+littlefs FLASH 1173960 1173750 -210 -0.0
RAM 23941 23933 -8 -0.0
bl702l lighting-app bl702l+mfd+littlefs FLASH 1083028 1082778 -250 -0.0
RAM 16612 16604 -8 -0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 840400 840320 -80 -0.0
RAM 123712 123712 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 825932 825804 -128 -0.0
RAM 125600 125600 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 772836 772700 -136 -0.0
RAM 114076 114076 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 757016 756912 -104 -0.0
RAM 114276 114276 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 540285 540169 -116 -0.0
RAM 205816 205816 0 0.0
lock CC3235SF_LAUNCHXL FLASH 574453 574337 -116 -0.0
RAM 205960 205960 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 681809 681713 -96 -0.0
RAM 78756 78756 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 701653 701565 -88 -0.0
RAM 81396 81396 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 701653 701565 -88 -0.0
RAM 81396 81396 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 658589 658493 -96 -0.0
RAM 73824 73824 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 618425 618265 -160 -0.0
RAM 71748 71748 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 638053 637893 -160 -0.0
RAM 74292 74292 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 638053 637893 -160 -0.0
RAM 74292 74292 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 637825 637713 -112 -0.0
RAM 74756 74756 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 657533 657421 -112 -0.0
RAM 77300 77300 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 657533 657421 -112 -0.0
RAM 77300 77300 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 614445 614317 -128 -0.0
RAM 68844 68844 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 634297 634169 -128 -0.0
RAM 71476 71476 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 634297 634169 -128 -0.0
RAM 71476 71476 0 0.0
efr32 lock-app BRD4187C FLASH 932740 932612 -128 -0.0
RAM 160228 160228 0 0.0
BRD4338a FLASH 747208 749248 2040 0.3
RAM 233356 233356 0 0.0
window-app BRD4187C FLASH 1025656 1026904 1248 0.1
RAM 128332 128332 0 0.0
esp32 all-clusters-app c3devkit DRAM 95352 95352 0 0.0
FLASH 1541996 1541986 -10 -0.0
IRAM 82552 82552 0 0.0
m5stack DRAM 116332 116332 0 0.0
FLASH 1548246 1548298 52 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4752 4752 0 0.0
FLASH 2731685 2722723 -8962 -0.3
RAM 133160 133160 0 0.0
all-clusters-app debug unknown 5560 5560 0 0.0
FLASH 6020306 5994660 -25646 -0.4
RAM 526008 526008 0 0.0
all-clusters-minimal-app debug unknown 5456 5456 0 0.0
FLASH 5356400 5341140 -15260 -0.3
RAM 243072 243072 0 0.0
bridge-app debug unknown 5472 5472 0 0.0
FLASH 4704816 4696262 -8554 -0.2
RAM 221824 221824 0 0.0
chip-tool debug unknown 5992 5984 -8 -0.1
FLASH 12938676 12866734 -71942 -0.6
RAM 587002 587002 0 0.0
chip-tool-ipv6only arm64 unknown 21560 21536 -24 -0.1
FLASH 11053856 10988896 -64960 -0.6
RAM 638064 638048 -16 -0.0
fabric-admin debug unknown 5816 5808 -8 -0.1
FLASH 11334801 11273845 -60956 -0.5
RAM 587346 587346 0 0.0
fabric-bridge-app debug unknown 4728 4728 0 0.0
FLASH 4529968 4521214 -8754 -0.2
RAM 208928 208928 0 0.0
fabric-sync debug unknown 4968 4968 0 0.0
FLASH 5641333 5622421 -18912 -0.3
RAM 477880 477880 0 0.0
lighting-app debug+rpc+ui unknown 6136 6136 0 0.0
FLASH 5640945 5630785 -10160 -0.2
RAM 232072 232072 0 0.0
lock-app debug unknown 5408 5408 0 0.0
FLASH 4753184 4743994 -9190 -0.2
RAM 208072 208072 0 0.0
ota-provider-app debug unknown 4768 4768 0 0.0
FLASH 4379730 4371566 -8164 -0.2
RAM 201744 201744 0 0.0
ota-requestor-app debug unknown 4720 4720 0 0.0
FLASH 4518638 4509578 -9060 -0.2
RAM 206312 206312 0 0.0
shell debug unknown 4248 4248 0 0.0
FLASH 3037885 3022637 -15248 -0.5
RAM 160792 160792 0 0.0
thermostat-no-ble arm64 unknown 9584 9552 -32 -0.3
FLASH 4120104 4109704 -10400 -0.3
RAM 246384 246368 -16 -0.0
tv-app debug unknown 5736 5736 0 0.0
FLASH 5990677 5966213 -24464 -0.4
RAM 601312 601312 0 0.0
tv-casting-app debug unknown 5320 5312 -8 -0.2
FLASH 11167325 11101789 -65536 -0.6
RAM 700496 700496 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 918156 917880 -276 -0.0
RAM 143332 143332 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 890664 890828 164 0.0
RAM 141519 141519 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 852220 852004 -216 -0.0
RAM 142244 142244 0 0.0
nxp contact k32w0+release FLASH 586112 585984 -128 -0.0
RAM 71112 71112 0 0.0
mcxw71+release FLASH 601624 601512 -112 -0.0
RAM 63328 63328 0 0.0
light k32w0+release FLASH 612748 612636 -112 -0.0
RAM 70504 70504 0 0.0
k32w1+release FLASH 687384 687280 -104 -0.0
RAM 48920 48920 0 0.0
lock mcxw71+release FLASH 763704 763592 -112 -0.0
RAM 70956 70956 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1647684 1652132 4448 0.3
RAM 212144 212144 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1555316 1557892 2576 0.2
RAM 208960 208960 0 0.0
light cy8ckit_062s2_43012 FLASH 1470420 1472500 2080 0.1
RAM 200928 200928 0 0.0
lock cy8ckit_062s2_43012 FLASH 1468156 1470284 2128 0.1
RAM 225280 225280 0 0.0
qpg lighting-app qpg6105+debug FLASH 664392 664272 -120 -0.0
RAM 105456 105456 0 0.0
lock-app qpg6105+debug FLASH 622204 622100 -104 -0.0
RAM 99908 99908 0 0.0
stm32 light STM32WB5MM-DK FLASH 485136 485048 -88 -0.0
RAM 144912 144912 0 0.0
telink bridge-app tlsr9258a FLASH 683698 683576 -122 -0.0
RAM 91248 91248 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 623938 623806 -132 -0.0
RAM 31488 31488 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 772772 772664 -108 -0.0
RAM 49348 49348 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 777388 777280 -108 -0.0
RAM 99812 99812 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 711380 711260 -120 -0.0
RAM 73544 73544 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 628384 628276 -108 -0.0
RAM 142180 142180 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 814398 814290 -108 -0.0
RAM 99724 99724 0 0.0
tizen all-clusters-app arm unknown 5160 5120 -40 -0.8
FLASH 1781748 1767248 -14500 -0.8
RAM 93720 93708 -12 -0.0
chip-tool-ubsan arm unknown 10924 10904 -20 -0.2
FLASH 18121646 17948374 -173272 -1.0
RAM 7909212 7841128 -68084 -0.9
tehampson
tehampson approved these changes on 2025-01-10
bzbarsky-apple
bzbarsky-apple approved these changes on 2025-01-10
pullapprove pullapprove removed review - pending
pullapprove pullapprove added review - approved
jmartinez-silabs
jmartinez-silabs approved these changes on 2025-01-10
mergify mergify merged 448930e8 into master 96 days ago

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
No one assigned
Labels
examples lib app review - approved core
Milestone
No milestone
Terms Of Use Privacy Policy Refund Policy Impressum