Skip to content

Commit

Permalink
fix: grow memory when necessary in BaseVec::set. (#222)
Browse files Browse the repository at this point in the history
## 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 is both simpler and more
performant.
  • Loading branch information
ielashi authored Jun 7, 2024
1 parent ee8ea0b commit fbec750
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/base_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -183,7 +184,7 @@ impl<T: Storable, M: Memory> BaseVec<T, M> {
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.
Expand Down
12 changes: 12 additions & 0 deletions src/vec/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,15 @@ 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::<Blob<65536>, 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; 65536].as_slice()).unwrap());
}

0 comments on commit fbec750

Please sign in to comment.