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.

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

Darwin: Avoid autorelease handling around LoggerForModule #35884

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

ksperling-apple
Copy link
Contributor

We don't want the LoggerForModule() implementation to call retain/autorelease on the logger, because its unnecessary, and because LoggerForModule() can be called from a C++ context without an auto-release pool in place.

To avoid this, we can have LoggerForModule() return a raw pointer, and handle the bridge cast to os_log_t on the caller side. This also avoids unnecessary code on the caller side that would otherwise attempt to rescue the presumed-autoreleased logger object from the ARP.

We don't want the LoggerForModule() implementation to call retain/autorelease
on the logger, because its unnecessary, and because LoggerForModule() can be
called from a C++ context without an auto-release pool in place.

To avoid this, we can have LoggerForModule() return a raw pointer, and handle
the bridge cast to os_log_t on the caller side. This also avoids unnecessary
code on the caller side that would otherwise attempt to rescue the
presumed-autoreleased logger object from the ARP.
@ksperling-apple ksperling-apple requested a review from a team as a code owner October 2, 2024 22:35
Copy link

Review changes with SemanticDiff.

Copy link

github-actions bot commented Oct 2, 2024

PR #35884: Size comparison from c565622 to b1f9c9f

Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section c565622 b1f9c9f change % change
bl602 lighting-app bl602 FLASH 1280504 1280504 0 0.0
RAM 95808 95808 0 0.0
bl602+mfd FLASH 1308552 1308552 0 0.0
RAM 95464 95464 0 0.0
bl602+rpc FLASH 1335172 1335172 0 0.0
RAM 103784 103784 0 0.0
bl702 lighting-app bl702 FLASH 944908 944908 0 0.0
RAM 15105 15105 0 0.0
bl702+mfd FLASH 957362 957362 0 0.0
RAM 14769 14769 0 0.0
bl702+rpc FLASH 1051066 1051066 0 0.0
RAM 23661 23661 0 0.0
bl706-eth FLASH 658040 658040 0 0.0
RAM 24745 24745 0 0.0
bl706-wifi FLASH 824836 824836 0 0.0
RAM 13965 13965 0 0.0
bl702l lighting-app bl702l FLASH 961844 961844 0 0.0
RAM 16804 16804 0 0.0
bl702l+mfd FLASH 974606 974606 0 0.0
RAM 16468 16468 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 829516 829516 0 0.0
RAM 123300 123300 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 814592 814592 0 0.0
RAM 125172 125172 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 761140 761140 0 0.0
RAM 113664 113664 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 745376 745376 0 0.0
RAM 113856 113856 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 616842 616842 0 0.0
RAM 205396 205396 0 0.0
lock CC3235SF_LAUNCHXL FLASH 656962 656962 0 0.0
RAM 205548 205548 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 678485 678485 0 0.0
RAM 78516 78516 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 698337 698337 0 0.0
RAM 81148 81148 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 698337 698337 0 0.0
RAM 81148 81148 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 655273 655273 0 0.0
RAM 73584 73584 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 614557 614557 0 0.0
RAM 71468 71468 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 634193 634193 0 0.0
RAM 74020 74020 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 634193 634193 0 0.0
RAM 74020 74020 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 633861 633861 0 0.0
RAM 74516 74516 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 653577 653577 0 0.0
RAM 77068 77068 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 653577 653577 0 0.0
RAM 77068 77068 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 609173 609173 0 0.0
RAM 68604 68604 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 629033 629033 0 0.0
RAM 71236 71236 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 629033 629033 0 0.0
RAM 71236 71236 0 0.0
efr32 lighting-app BRD4187C FLASH 934012 934012 0 0.0
RAM 135884 135884 0 0.0
lock-app BRD2605a FLASH 741840 741832 -8 -0.0
RAM 230240 230240 0 0.0
BRD4338a FLASH 742488 742480 -8 -0.0
RAM 230272 230272 0 0.0
window-app BRD4187C FLASH 1017128 1017120 -8 -0.0
RAM 127804 127804 0 0.0
esp32 all-clusters-app c3devkit DRAM 94080 94080 0 0.0
FLASH 1538324 1538324 0 0.0
IRAM 82538 82538 0 0.0
m5stack DRAM 115032 115032 0 0.0
FLASH 1548758 1548758 0 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4688 4688 0 0.0
FLASH 2776667 2776667 0 0.0
RAM 129424 129424 0 0.0
all-clusters-app debug unknown 5528 5528 0 0.0
FLASH 6080128 6080128 0 0.0
RAM 507872 507872 0 0.0
all-clusters-minimal-app debug unknown 5424 5424 0 0.0
FLASH 5413928 5413928 0 0.0
RAM 241136 241136 0 0.0
bridge-app debug unknown 5408 5408 0 0.0
FLASH 4747346 4747346 0 0.0
RAM 218320 218320 0 0.0
chip-tool debug unknown 5960 5960 0 0.0
FLASH 12586654 12586654 0 0.0
RAM 556402 556402 0 0.0
chip-tool-ipv6only arm64 unknown 20560 20560 0 0.0
FLASH 11238160 11238160 0 0.0
RAM 606672 606672 0 0.0
fabric-admin debug unknown 5792 5792 0 0.0
FLASH 10941109 10941109 0 0.0
RAM 555514 555514 0 0.0
fabric-bridge-app debug unknown 4640 4640 0 0.0
FLASH 4570546 4570546 0 0.0
RAM 204984 204984 0 0.0
lighting-app debug+rpc+ui unknown 6056 6056 0 0.0
FLASH 5688209 5688209 0 0.0
RAM 228584 228584 0 0.0
lock-app debug unknown 5344 5344 0 0.0
FLASH 4796768 4796768 0 0.0
RAM 204392 204392 0 0.0
ota-provider-app debug unknown 4720 4720 0 0.0
FLASH 4426300 4426300 0 0.0
RAM 198128 198128 0 0.0
ota-requestor-app debug unknown 4656 4656 0 0.0
FLASH 4565148 4565148 0 0.0
RAM 202696 202696 0 0.0
shell debug unknown 4216 4216 0 0.0
FLASH 3106733 3106733 0 0.0
RAM 159216 159216 0 0.0
thermostat-no-ble arm64 unknown 9448 9448 0 0.0
FLASH 4315120 4315120 0 0.0
RAM 242816 242816 0 0.0
tv-app debug unknown 5624 5624 0 0.0
FLASH 6024229 6024229 0 0.0
RAM 581424 581424 0 0.0
tv-casting-app debug unknown 5208 5208 0 0.0
FLASH 10781309 10781309 0 0.0
RAM 644320 644320 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 914148 914148 0 0.0
RAM 142199 142199 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 884952 884952 0 0.0
RAM 140338 140338 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 847564 847564 0 0.0
RAM 141093 141093 0 0.0
nxp contact k32w0+release FLASH 581912 581912 0 0.0
RAM 70784 70784 0 0.0
k32w1+release FLASH 596632 596632 0 0.0
RAM 63008 63008 0 0.0
mcxw71+release FLASH 596392 596392 0 0.0
RAM 63008 63008 0 0.0
light k32w0+release FLASH 618500 618500 0 0.0
RAM 70256 70256 0 0.0
k32w1+release FLASH 682592 682592 0 0.0
RAM 48648 48648 0 0.0
mcxw71+release FLASH 682608 682608 0 0.0
RAM 48648 48648 0 0.0
lock k32w1+release FLASH 704968 704968 0 0.0
RAM 67148 67148 0 0.0
mcxw71+release FLASH 704992 704992 0 0.0
RAM 67148 67148 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1646028 1646028 0 0.0
RAM 210888 210888 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1552068 1552068 0 0.0
RAM 207688 207688 0 0.0
light cy8ckit_062s2_43012 FLASH 1467820 1467820 0 0.0
RAM 200696 200696 0 0.0
lock cy8ckit_062s2_43012 FLASH 1464612 1464612 0 0.0
RAM 225048 225048 0 0.0
qpg lighting-app qpg6105+debug FLASH 660240 660240 0 0.0
RAM 105236 105236 0 0.0
lock-app qpg6105+debug FLASH 618260 618260 0 0.0
RAM 99704 99704 0 0.0
stm32 light STM32WB5MM-DK FLASH 481792 481792 0 0.0
RAM 144692 144692 0 0.0
telink air-quality-sensor-app tlsr9528a_retention FLASH 620636 620636 0 0.0
RAM 50488 50488 0 0.0
all-clusters-app tlsr9118bdk40d FLASH 687832 687832 0 0.0
RAM 148328 148328 0 0.0
all-clusters-minimal-app tlsr9528a FLASH 780824 780824 0 0.0
RAM 110280 110280 0 0.0
bridge-app tlsr9258a FLASH 680734 680734 0 0.0
RAM 91144 91144 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 620400 620400 0 0.0
RAM 50440 50440 0 0.0
light-switch-app-ota-shell-factory-data tlsr9528a FLASH 708340 708340 0 0.0
RAM 73780 73780 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 625514 625514 0 0.0
RAM 144316 144316 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 811274 811274 0 0.0
RAM 98948 98948 0 0.0
lock-app-dfu tlsr9528a FLASH 656284 656284 0 0.0
RAM 66500 66500 0 0.0
ota-requestor-app tlsr9258a FLASH 696710 696710 0 0.0
RAM 90736 90736 0 0.0
pump-app-usb tlsr9518adk80d FLASH 634032 634032 0 0.0
RAM 55316 55316 0 0.0
pump-controller-app tlsr9518adk80d FLASH 611222 611222 0 0.0
RAM 52560 52560 0 0.0
shell tlsr9518adk80d FLASH 467862 467862 0 0.0
RAM 68168 68168 0 0.0
smoke_co_alarm-app tlsr9528a_retention FLASH 627530 627530 0 0.0
RAM 52160 52160 0 0.0
temperature-measurement-app-mars-ota tlsr9518adk80d FLASH 653358 653358 0 0.0
RAM 56108 56108 0 0.0
thermostat tlsr9518adk80d FLASH 637954 637954 0 0.0
RAM 52952 52952 0 0.0
window-covering tlsr9118bdk40d FLASH 524096 524096 0 0.0
RAM 97284 97284 0 0.0
tizen all-clusters-app arm unknown 4904 4904 0 0.0
FLASH 1725668 1725668 0 0.0
RAM 89076 89076 0 0.0
chip-tool-ubsan arm unknown 10368 10368 0 0.0
FLASH 17413834 17413834 0 0.0
RAM 7582052 7582052 0 0.0

@ksperling-apple
Copy link
Contributor Author

Fast-tracking Darwin-only fix.

@mergify mergify bot merged commit af1b0ee into project-chip:master Oct 3, 2024
69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants