-
Notifications
You must be signed in to change notification settings - Fork 135
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
Add support for persistent state in separate virtual memory #2321
Conversation
This PR adds support for the storage layout `v9` which keeps the persistent state in a separate virtual memory. The PR does _not_ include any migration. Existing II installations will keep layout `v8`.
63abcb9
to
ace8b62
Compare
@@ -145,7 +146,7 @@ pub const MAX_ENTRIES: u64 = (MAX_MANAGED_WASM_PAGES - BUCKET_SIZE_IN_PAGES as u | |||
|
|||
pub type Salt = [u8; 32]; | |||
|
|||
type ArchiveBufferMemory<M> = RestrictedMemory<VirtualMemory<RestrictedMemory<M>>>; | |||
type SingleBucketMemory<M> = RestrictedMemory<VirtualMemory<RestrictedMemory<M>>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be cleaner to wrap the underlying memory such that:
- the memory is guaranteed to be limited to a single bucket
- the memory cannot be written to
However, I'll do this in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
ii_canister | ||
} | ||
|
||
fn ii_canisters_under_test( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nice improvement! 🎉
@@ -29,16 +29,16 @@ fn should_report_max_number_of_entries_for_256gb() { | |||
} | |||
|
|||
#[test] | |||
fn should_serialize_header_v8() { | |||
fn should_serialize_header_v9() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: shouldn't we still keep the test for v8
? (i.e. add a new one for v9
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New storage is now automatically initialized in v9
. The ability to initialize v8
is removed.
Compatibility with existing v8
storage is tested in the (unchanged) should_recover_header_from_memory_v8
test.
This PR adds support for the storage layout
v9
which keeps the persistent state in a separate virtual memory.The PR does not include any migration. Existing II installations will keep layout
v8
.