Optionally avoid global constructors #29071
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
scripts
transport
platform
darwin
lib
controller
app
support
core
labels
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis and
gjc13
Damian-Nordic
approved these changes
Sep 7, 2023
|
Can you describe the problem this is aiming to fix? Wouldn't it be a lot cleaner to put them inside another object? e.g. something like #19784 You can also get lazy initialization by making them function scoped static variables (this way is also thread safe). |
src/credentials/attestation_verifier/DeviceAttestationVerifier.h
Outdated
Show resolved
Hide resolved
@mspang See previous discussion in this PR, which GitHub helpfully hides by default. |
ksperling-apple
force-pushed
the
darwin-no-inits
branch
from
4bf92a4 to
644c41c
Compare
|
PR #29071: Size comparison from d104699 to 644c41c Increases (3 builds for cc32xx, qpg)
Decreases (3 builds for cc32xx, qpg)
Full report (4 builds for cc32xx, mbed, qpg)
|
ksperling-apple
force-pushed
the
darwin-no-inits
branch
from
644c41c to
9d42b65
Compare
|
PR #29071: Size comparison from d104699 to 9d42b65 Increases (64 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, nrfconnect, psoc6, qpg, telink)
Decreases (37 builds for bl602, bl702, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, linux, psoc6, qpg)
Full report (65 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
The behavior of chip::Global can be configured via the platform configuration via the settings CHIP_CONFIG_GLOBALS_LAZY_INIT CHIP_CONFIG_GLOBALS_NO_DESTRUCT Both default to 0, retaining normal C++ global init behavior.
…/ destructors Note that there is a slight change to the API of ArrayAttestationTrustStore to make it easier to create and pass a constexpr array of root certificates.
ksperling-apple
force-pushed
the
darwin-no-inits
branch
from
01d1fb9 to
7987129
Compare
|
PR #29071: Size comparison from 6e3d045 to 7987129 Increases (31 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, k32w, linux, mbed, psoc6, qpg)
Decreases (23 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, k32w, linux, psoc6, qpg)
Full report (31 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, k32w, linux, mbed, psoc6, qpg)
|
ksperling-apple
force-pushed
the
darwin-no-inits
branch
from
7987129 to
ca5aea4
Compare
|
PR #29071: Size comparison from 6e3d045 to ca5aea4 Increases above 0.2%:
Increases (62 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, psoc6, qpg, telink)
Decreases (38 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, linux, psoc6, qpg)
Full report (62 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, psoc6, qpg, telink)
|
... by moving kTestAttestationTrustStoreRoots (formerly kTestPaaRoots) into CHIPAttCert_test_vectors.cpp where the certificate spans themselves are defined.
ksperling-apple
force-pushed
the
darwin-no-inits
branch
from
ca5aea4 to
87c04bb
Compare
|
PR #29071: Size comparison from 242a52b to 87c04bb Increases above 0.2%:
Increases (65 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Decreases (38 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, linux, psoc6, qpg)
Full report (65 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
approved these changes
Sep 18, 2023
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
|
PR #29071: Size comparison from 242a52b to acfcc71 Increases above 0.2%:
Increases (62 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, psoc6, qpg, telink)
Decreases (38 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, linux, psoc6, qpg)
Full report (65 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
HunsupJung
pushed a commit
to HunsupJung/connectedhomeip
that referenced
this pull request
Oct 23, 2023
* Use SystemClock() instead of creating a separate instance * Add chip::Global to make initialization of globals configurable The behavior of chip::Global can be configured via the platform configuration via the settings CHIP_CONFIG_GLOBALS_LAZY_INIT CHIP_CONFIG_GLOBALS_NO_DESTRUCT Both default to 0, retaining normal C++ global init behavior. * Darwin: Adopt chip::Global for platform / framework * Adopt chip::Global for core singletons with non-trivial constructors / destructors Note that there is a slight change to the API of ArrayAttestationTrustStore to make it easier to create and pass a constexpr array of root certificates. * Make Global::get() public and remove operator* * Avoid changing ArrayAttestationTrustStore API... ... by moving kTestAttestationTrustStoreRoots (formerly kTestPaaRoots) into CHIPAttCert_test_vectors.cpp where the certificate spans themselves are defined. * Comment wording Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce a chip::Global wrapper for managing global objects with non-trivial constructors / destructors.
The behavior of the wrapper can be configured via two
CHIP_CONFIG_GLOBALS_*options in the platform config header (the default is current standard C++ behavior). By settingCHIP_CONFIG_GLOBALS_LAZY_INIT = 1globals can instead be constructed lazily (note this is done in a thread-unsafe way as synchronization is assumed to be external).Adopt chip::Global in the Darwin platform layer and for key core singletons and enable CHIP_CONFIG_GLOBALS_LAZY_INIT = 1 on Darwin.