Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow the allocator to track the heap changes. #9291

Merged
5 commits merged into from
Jul 7, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions client/allocator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ targets = ["x86_64-unknown-linux-gnu"]
sp-std = { version = "3.0.0", path = "../../primitives/std", default-features = false }
sp-core = { version = "3.0.0", path = "../../primitives/core", default-features = false }
sp-wasm-interface = { version = "3.0.0", path = "../../primitives/wasm-interface", default-features = false }
log = { version = "0.4.11", optional = true }
log = { version = "0.4.11" }
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
thiserror = { version = "1.0.21" }

[features]
Expand All @@ -26,5 +26,5 @@ std = [
"sp-std/std",
"sp-core/std",
"sp-wasm-interface/std",
"log",
"log/std",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"log/std",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh about this: now the allocator is even in client. Should this crate really have a separate "std" feature? I can just remove that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The std feature is not needed. We should remove it IMO

]
55 changes: 40 additions & 15 deletions client/allocator/src/freeing_bump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,7 @@ fn error(msg: &'static str) -> Error {
Error::Other(msg)
}

/// A custom "trace" implementation that is only activated when `feature = std`.
///
/// Uses `wasm-heap` as default target.
macro_rules! trace {
( $( $args:expr ),+ ) => {
sp_std::if_std! {
log::trace!(target: "wasm-heap", $( $args ),+);
}
}
}
const LOG_TARGET: &'static str = "wasm-heap";

// The minimum possible allocation size is chosen to be 8 bytes because in that case we would have
// easier time to provide the guaranteed alignment of 8.
Expand Down Expand Up @@ -146,6 +137,7 @@ impl Order {
/// `MIN_POSSIBLE_ALLOCATION <= size <= MAX_POSSIBLE_ALLOCATION`
fn from_size(size: u32) -> Result<Self, Error> {
let clamped_size = if size > MAX_POSSIBLE_ALLOCATION {
log::warn!(target: LOG_TARGET, "going to fail due to allocating {:?}", size);
return Err(Error::RequestedAllocationTooLarge);
} else if size < MIN_POSSIBLE_ALLOCATION {
MIN_POSSIBLE_ALLOCATION
Expand Down Expand Up @@ -331,6 +323,19 @@ pub struct FreeingBumpHeapAllocator {
free_lists: FreeLists,
total_size: u32,
poisoned: bool,
max_total_size: u32,
max_bumper: u32,
}

impl Drop for FreeingBumpHeapAllocator {
fn drop(&mut self) {
log::debug!(
emostov marked this conversation as resolved.
Show resolved Hide resolved
target: LOG_TARGET,
"allocator being destroyed, max_total_size {}, max_bumper {}",
self.max_total_size,
self.max_bumper,
)
}
}

impl FreeingBumpHeapAllocator {
Expand All @@ -347,6 +352,8 @@ impl FreeingBumpHeapAllocator {
free_lists: FreeLists::new(),
total_size: 0,
poisoned: false,
max_total_size: 0,
max_bumper: aligned_heap_base,
}
}

Expand Down Expand Up @@ -404,7 +411,21 @@ impl FreeingBumpHeapAllocator {
Header::Occupied(order).write_into(mem, header_ptr)?;

self.total_size += order.size() + HEADER_SIZE;
trace!("Heap size is {} bytes after allocation", self.total_size);

log::trace!(
target: LOG_TARGET,
"after allocation, total_size = {}, bumper = {}.",
self.total_size,
self.bumper,
);
emostov marked this conversation as resolved.
Show resolved Hide resolved

// update trackers if needed.
if self.total_size > self.max_total_size {
self.max_total_size = self.total_size;
}
if self.bumper > self.max_bumper {
self.max_bumper = self.bumper;
}
emostov marked this conversation as resolved.
Show resolved Hide resolved

bomb.disarm();
Ok(Pointer::new(header_ptr + HEADER_SIZE))
Expand Down Expand Up @@ -442,19 +463,23 @@ impl FreeingBumpHeapAllocator {
.total_size
.checked_sub(order.size() + HEADER_SIZE)
.ok_or_else(|| error("Unable to subtract from total heap size without overflow"))?;
trace!("Heap size is {} bytes after deallocation", self.total_size);
log::trace!(
"after deallocation, total_size = {}, bumper = {}.",
self.total_size,
self.bumper,
);

bomb.disarm();
Ok(())
}

/// Increases the `bumper` by `size`.
///
/// Returns the `bumper` from before the increase.
/// Returns an `Error::AllocatorOutOfSpace` if the operation
/// would exhaust the heap.
/// Returns the `bumper` from before the increase. Returns an `Error::AllocatorOutOfSpace` if
/// the operation would exhaust the heap.
fn bump(bumper: &mut u32, size: u32, heap_end: u32) -> Result<u32, Error> {
if *bumper + size > heap_end {
log::error!(target: LOG_TARGET, "running out of space with current bumper {}, mem size {}", bumper, heap_end);
return Err(Error::AllocatorOutOfSpace);
}

Expand Down
2 changes: 1 addition & 1 deletion client/executor/wasmtime/src/instance_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ fn get_table(instance: &Instance) -> Option<Table> {
.cloned()
}

/// Functions realted to memory.
/// Functions related to memory.
impl InstanceWrapper {
/// Read data from a slice of memory into a destination buffer.
///
Expand Down