Use resilient storage for globals #648
Conversation
Size Comparison Report (x86_64)
|
kstenerud
force-pushed
the
karl/resilient-globals
branch
2 times, most recently
from
918a9a8 to
1d55c45
Compare
Size Comparison Report (x86_64)
|
kstenerud
requested review from
FranAguilera,
Copilot,
kattrali,
mattklein123 and
snowp
There was a problem hiding this comment.
Pull Request Overview
Replaces platform-specific storage implementations (UserDefaults on iOS and Preferences on Android) with a crash-resilient key-value storage solution that stores data in the SDK directory as storage.jrna and storage.jrnb files.
- Updates both iOS/Swift and Android/JVM platforms to use
ResilientKvStorageinstead of platform-specific storage - Bumps shared-core dependency to include the resilient storage implementation
- Ensures SDK directory creation on Android before initializing storage
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| platform/swift/source/src/bridge.rs | Replaces UserDefaultsStorage with ResilientKvStorage and moves path extraction earlier |
| platform/jvm/src/jni.rs | Replaces PreferencesHandle with ResilientKvStorage and adds directory creation |
| Cargo.toml | Updates shared-core dependency revision to include resilient storage support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
kstenerud
force-pushed
the
karl/resilient-globals
branch
2 times, most recently
from
e3e4c37 to
673cd6f
Compare
Size Comparison Report (x86_64)
|
| let preferences = new_global!(PreferencesHandle, &mut env, preferences)?; | ||
| let store = Arc::new(bd_key_value::Store::new(Box::new(preferences))); | ||
|
|
||
| let storage = Box::new(bd_key_value::resilient_kv::ResilientKvStorage::new( |
There was a problem hiding this comment.
If we're no longer passing the storage implementation from the platform layer should we just move this initialization into builder.rs instead? That would let us unify all the setup and sequence the initialization better with the rest of the logger setup
There was a problem hiding this comment.
I had a look, but the relationships between the various objects are surprisingly complicated here (and arranged differently in JVM vs Swift). I think this would be better done in a separate PR as it could get quite hairy...
kstenerud
force-pushed
the
karl/resilient-globals
branch
from
673cd6f to
680023d
Compare
Size Comparison Report (x86_64)
|
kstenerud
force-pushed
the
karl/resilient-globals
branch
5 times, most recently
from
8102248 to
81d8a54
Compare
kstenerud
force-pushed
the
karl/resilient-globals
branch
from
81d8a54 to
9b9165f
Compare
| ); | ||
|
|
||
| // Create the SDK directory if it doesn't exist | ||
| std::fs::create_dir_all(&sdk_directory)?; |
There was a problem hiding this comment.
Is this required? I see it exists here but not on the iOS side?
2 participants
This PR makes use of bitdriftlabs/shared-core#255 to provide alternatives for the current
Storageimplementations for storing global data on mobile.Instead of
Preferences(Android) andUserDefaults(iOS), we use the crash-resilient KV store, which is stored in the sdk directory asstorage.jrnaandstorage.jrnb.