Make disabling GC prevent even scheduling it #2862
Conversation
Also make the linkage of kFIRFirestoreCacheSizeUnlimited explicitly extern "C". This previously had no fixed linkage in the header and must have accidentally worked because the implementation had C++ linkage. Finally, move internal CacheSizeUnlimited to api::Settings and the definition of kFIRFirestoreCacheSizeUnlimited into the Objective-C language binding.
| @@ -35,6 +35,9 @@ | |||
| static const int64_t kMinimumCacheSizeBytes = 1 * 1024 * 1024; | |||
| static const BOOL kDefaultTimestampsInSnapshotsEnabled = YES; | |||
|
|
|||
| // Public constant | |||
| extern "C" const int64_t kFIRFirestoreCacheSizeUnlimited = api::Settings::CacheSizeUnlimited; | |||
There was a problem hiding this comment.
When I did the api::Settings port I was thinking about moving all the defaults to be defined there but wasn't sure if that would open the door for static initialization order fiasco, so I left it as-is. But I guess this is safe because it's a builtin value.
There was a problem hiding this comment.
The key to making that work is to ensure that the values are compiler initialized. Making values constexpr is one guaranteed way to do that. Another is to annotate the value with ABSL_CONST_INIT which adds an attribute that would cause a compiler error if the value could not be compiler initialized.
With that in place it's safe to refer to other values in the definition of a constant. I've ported all the settings defaults as well.
Ensure that all defaults are compiler initialized to ensure there are no initialization order problems.
|
PTAL |
|
It looks like some hexa-reworking ended up in this PR. I'm happy to review together, but checking if that was intentional? |
|
Whoops, not intentional. I'll remove it. |
* Make disabling GC prevent even scheduling it Make the linkage of kFIRFirestoreCacheSizeUnlimited explicitly extern "C". This previously had no fixed linkage in the header and must have accidentally worked because the implementation had C++ linkage. Move internal CacheSizeUnlimited to api::Settings and the definition of kFIRFirestoreCacheSizeUnlimited into the Objective-C language binding. Migrate settings defaults to C++ settings Ensure that all defaults are compiler initialized to ensure there are no initialization order problems.
We learned in #2846 that the current implementation isn't an effective way to disable GC, because the empty commit itself was failing.
Also make the linkage of kFIRFirestoreCacheSizeUnlimited explicitly extern "C". This previously had no fixed linkage in the header and only accidentally worked because the implementation had C++ linkage.
Move internal
CacheSizeUnlimitedtoapi::Settingsand the definition ofkFIRFirestoreCacheSizeUnlimitedinto the Objective-C language binding.Move the rest of the
FIRFirestoreSettingsdefaults toapi::Settingsas well.