From 4437f075694e32129dfbd034ef5e7107bd964c01 Mon Sep 17 00:00:00 2001 From: Islam El-Ashi Date: Fri, 7 Jun 2024 14:16:59 +0200 Subject: [PATCH 1/3] fix: grow memory when necessary in `BaseVec::set`. ## Problem `BaseVec`, which is the underlying data structure used by `StableVec` and `StableMinHeap` provides a `set` method that allows the caller to change the value of an element at a specified index. The `set` method used the `memory.write` method, which assumed that there is always enough memory to rewrite the element. This assumption would've been fine if other methods like `push` always allocated the maximum amount of space for every element. Currently though, `push` only allocates the amount needed to store the element that's being pushed. ## Solution There are two possible solutions: 1) Modify `push` to always allocate the maximum size of an element. 2) Modify `set` to grow the memory as needed as it's rewriting an element. This commit implements solution #2, which was both simpler and more performant. --- src/base_vec.rs | 5 +++-- src/vec/tests.rs | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/base_vec.rs b/src/base_vec.rs index af6232ae..8a3a3b45 100644 --- a/src/base_vec.rs +++ b/src/base_vec.rs @@ -33,7 +33,8 @@ //! bytes required to represent integers up to that max size. use crate::storable::{bounds, bytes_to_store_size_bounded}; use crate::{ - read_u32, read_u64, safe_write, write_u32, write_u64, Address, GrowFailed, Memory, Storable, + read_u32, read_u64, safe_write, write, write_u32, write_u64, Address, GrowFailed, Memory, + Storable, }; use std::borrow::{Borrow, Cow}; use std::cmp::min; @@ -183,7 +184,7 @@ impl BaseVec { let data_offset = self .write_entry_size(offset, bytes.len() as u32) .expect("unreachable: cannot fail to write to pre-allocated area"); - self.memory.write(data_offset, bytes.borrow()); + write(&self.memory, data_offset, bytes.borrow()); } /// Returns the item at the specified index. diff --git a/src/vec/tests.rs b/src/vec/tests.rs index eb89d492..3cfc18d0 100644 --- a/src/vec/tests.rs +++ b/src/vec/tests.rs @@ -266,3 +266,18 @@ fn set_element_bigger_than_max_size_panics() { // Insert a struct where the serialized size is > `MAX_SIZE`. Should panic. sv.set(0, &BuggyStruct(vec![1, 2, 3, 4, 5])); } + +#[test] +fn set_last_element_to_large_blob() { + use crate::storable::Blob; + let sv = StableVec::, M>::new(M::default()).unwrap(); + + // Store a small blob. + sv.push(&Blob::default()).unwrap(); + + // Store a large blob that would require growing the memory. + sv.set( + 0, + &Blob::try_from(vec![1; 64 * 1024 as usize].as_slice()).unwrap(), + ); +} From 7f7bb8fa3e76a870f360ee111e5acfccd9333654 Mon Sep 17 00:00:00 2001 From: Islam El-Ashi Date: Fri, 7 Jun 2024 14:22:09 +0200 Subject: [PATCH 2/3] . --- src/vec/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vec/tests.rs b/src/vec/tests.rs index 3cfc18d0..e9fcae06 100644 --- a/src/vec/tests.rs +++ b/src/vec/tests.rs @@ -278,6 +278,6 @@ fn set_last_element_to_large_blob() { // Store a large blob that would require growing the memory. sv.set( 0, - &Blob::try_from(vec![1; 64 * 1024 as usize].as_slice()).unwrap(), + &Blob::try_from(vec![1; 65536].as_slice()).unwrap(), ); } From 93f546b5a5cd40f8f9c3e7eec6f34aa92df4c385 Mon Sep 17 00:00:00 2001 From: Islam El-Ashi Date: Fri, 7 Jun 2024 14:50:08 +0200 Subject: [PATCH 3/3] . --- src/vec/tests.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/vec/tests.rs b/src/vec/tests.rs index e9fcae06..d91e9ed0 100644 --- a/src/vec/tests.rs +++ b/src/vec/tests.rs @@ -276,8 +276,5 @@ fn set_last_element_to_large_blob() { sv.push(&Blob::default()).unwrap(); // Store a large blob that would require growing the memory. - sv.set( - 0, - &Blob::try_from(vec![1; 65536].as_slice()).unwrap(), - ); + sv.set(0, &Blob::try_from(vec![1; 65536].as_slice()).unwrap()); }