From d4f1d58b71ebed720b2dee3dc21f9f1e6ae83b31 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 25 Jun 2021 14:28:33 -0400 Subject: [PATCH 01/39] Add bump allocator skeleton --- crates/allocator/Cargo.toml | 1 + crates/allocator/src/bump.rs | 79 ++++++++++++++++++++++++++++++++++++ crates/allocator/src/lib.rs | 2 + 3 files changed, 82 insertions(+) create mode 100644 crates/allocator/src/bump.rs diff --git a/crates/allocator/Cargo.toml b/crates/allocator/Cargo.toml index 4b7b61f3676..395bf4dab3e 100644 --- a/crates/allocator/Cargo.toml +++ b/crates/allocator/Cargo.toml @@ -16,6 +16,7 @@ include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] [dependencies] wee_alloc = { version = "0.4", default-features = false } +spin = { version = "0.9", default-features = false, features = ["mutex", "spin_mutex"] } [features] default = ["std"] diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs new file mode 100644 index 00000000000..c5008d720f5 --- /dev/null +++ b/crates/allocator/src/bump.rs @@ -0,0 +1,79 @@ +// Copyright 2018-2021 Parity Technologies (UK) Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! A simple bump allocator. +//! +//! It's goal to have a much smaller footprint than the admittedly more full-featured `wee_alloc` +//! allocator which is currently being used by ink! smart contracts. + +use core::alloc::{GlobalAlloc, Layout}; + +/// A page in Wasm is 64KiB +const PAGE_SIZE: usize = 64 * 1024; + +pub struct Locked { + inner: spin::Mutex, +} + +impl Locked { + pub const fn new(inner: A) -> Self { + Locked { + inner: spin::Mutex::new(inner), + } + } + + pub fn lock(&self) -> spin::MutexGuard { + self.inner.lock() + } +} + +pub struct BumpAllocator { + /// Points to the start of the next available allocation + next: usize, +} + +impl BumpAllocator { + pub fn new() -> Self { + Self { + next: Default::default(), + } + } + + /// Initalize the backing heap of the allocator. + // + // In this case we'll be backed by a page of Wasm memory which is all we'll use for the life of + // the contract. + pub fn init(&mut self) { + self.next = core::arch::wasm32::memory_grow(0, 1); + } +} + +unsafe impl GlobalAlloc for Locked { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + // Layout is `size: usize`, `align: NonZeroUsize`, and + // Number of bytes, `align` is always a power-of-two + // In our case `usize` will be `u32` or 4-bytes + + // This should be okay since we're in a single threaded context anyways + let mut bump = self.lock(); + + let alloc_start = bump.next; + bump.next += layout.size(); + alloc_start as *mut u8 + } + + unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) { + todo!(); + } +} diff --git a/crates/allocator/src/lib.rs b/crates/allocator/src/lib.rs index 413561ca3f0..c5dc9333d7c 100644 --- a/crates/allocator/src/lib.rs +++ b/crates/allocator/src/lib.rs @@ -28,3 +28,5 @@ static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; #[cfg(not(feature = "std"))] mod handlers; + +mod bump; From c2b019e2c9b0f0486e2411fe6aa3d6bcad2e8742 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 25 Jun 2021 15:40:29 -0400 Subject: [PATCH 02/39] Implement `alloc` for our bump allocator --- crates/allocator/src/bump.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index c5008d720f5..33f124bbd24 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -55,21 +55,33 @@ impl BumpAllocator { // In this case we'll be backed by a page of Wasm memory which is all we'll use for the life of // the contract. pub fn init(&mut self) { - self.next = core::arch::wasm32::memory_grow(0, 1); + let ptr = core::arch::wasm32::memory_grow(0, 1); + if ptr == usize::max_value() { + todo!("TODO: OOM") + } } } unsafe impl GlobalAlloc for Locked { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - // Layout is `size: usize`, `align: NonZeroUsize`, and - // Number of bytes, `align` is always a power-of-two - // In our case `usize` will be `u32` or 4-bytes - - // This should be okay since we're in a single threaded context anyways + // This should be okay performance wise since we're in a single threaded context anyways let mut bump = self.lock(); + let aligned_layout = layout.pad_to_align(); + let alloc_start = bump.next; - bump.next += layout.size(); + let alloc_end = match alloc_start.checked_add(aligned_layout.size()) { + Some(end) => end, + None => return core::ptr::null_mut(), + }; + + // Since we're using a single page as our entire heap if we exceed it we're effectively + // out-of-memory. + if alloc_end > PAGE_SIZE { + return core::ptr::null_mut(); + } + + bump.next = alloc_end; alloc_start as *mut u8 } From 3deb3d847fd0275de49159a23b88b5ecd3dcdd0c Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 28 Jun 2021 11:44:24 -0400 Subject: [PATCH 03/39] Make the allocator usable globally --- crates/allocator/Cargo.toml | 7 +++++-- crates/allocator/src/bump.rs | 19 +++++++++++++++++-- crates/allocator/src/lib.rs | 13 +++++++++++++ crates/env/Cargo.toml | 3 ++- 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/crates/allocator/Cargo.toml b/crates/allocator/Cargo.toml index 395bf4dab3e..ccaf1223a14 100644 --- a/crates/allocator/Cargo.toml +++ b/crates/allocator/Cargo.toml @@ -15,9 +15,12 @@ categories = ["no-std", "embedded"] include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] [dependencies] -wee_alloc = { version = "0.4", default-features = false } -spin = { version = "0.9", default-features = false, features = ["mutex", "spin_mutex"] } +wee_alloc = { version = "0.4", default-features = false, optional = true } +spin = { version = "0.9", default-features = false, features = ["mutex", "spin_mutex"], optional = true } +lazy_static = { version = "1.4", default-features = false, features = ["spin_no_std"], optional = true } [features] default = ["std"] std = [] +bump-allocator = ["spin", "lazy_static"] +wee-allocator = ["wee_alloc"] diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 33f124bbd24..5601fbd8074 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -22,6 +22,10 @@ use core::alloc::{GlobalAlloc, Layout}; /// A page in Wasm is 64KiB const PAGE_SIZE: usize = 64 * 1024; +lazy_static::lazy_static! { + pub static ref BUMP_ALLOC: Locked = Locked::new(BumpAllocator::new()); +} + pub struct Locked { inner: spin::Mutex, } @@ -41,12 +45,15 @@ impl Locked { pub struct BumpAllocator { /// Points to the start of the next available allocation next: usize, + /// Hacks + heap_initialized: bool, } impl BumpAllocator { - pub fn new() -> Self { + pub const fn new() -> Self { Self { - next: Default::default(), + next: 0, + heap_initialized: false, } } @@ -67,6 +74,14 @@ unsafe impl GlobalAlloc for Locked { // This should be okay performance wise since we're in a single threaded context anyways let mut bump = self.lock(); + // TODO: Figure out how to properly initalize the heap + if !bump.heap_initialized { + let ptr = core::arch::wasm32::memory_grow(0, 1); + if ptr == usize::max_value() { + todo!("TODO: OOM") + } + } + let aligned_layout = layout.pad_to_align(); let alloc_start = bump.next; diff --git a/crates/allocator/src/lib.rs b/crates/allocator/src/lib.rs index c5dc9333d7c..1864ad2149b 100644 --- a/crates/allocator/src/lib.rs +++ b/crates/allocator/src/lib.rs @@ -23,10 +23,23 @@ // We use `wee_alloc` as the global allocator since it is optimized for binary file size // so that contracts compiled with it as allocator do not grow too much in size. #[cfg(not(feature = "std"))] +#[cfg(feature = "wee-allocator")] +#[cfg(not(feature = "bump-allocator"))] #[global_allocator] static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; +#[cfg(not(feature = "std"))] +#[cfg(feature = "bump-allocator")] +#[cfg(not(feature = "wee-allocator"))] +#[global_allocator] +static ALLOC: bump::Locked = + bump::Locked::new(bump::BumpAllocator::new()); +// static ALLOC: bump::BUMP_ALLOC = bump::BUMP_ALLOC; + #[cfg(not(feature = "std"))] mod handlers; +#[cfg(not(feature = "std"))] +#[cfg(feature = "bump-allocator")] +#[cfg(not(feature = "wee-allocator"))] mod bump; diff --git a/crates/env/Cargo.toml b/crates/env/Cargo.toml index a42f3086aa5..8efd50c0db4 100644 --- a/crates/env/Cargo.toml +++ b/crates/env/Cargo.toml @@ -17,7 +17,8 @@ include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] [dependencies] ink_engine = { version = "3.0.0-rc3", path = "../engine/", default-features = false, optional = true } ink_metadata = { version = "3.0.0-rc3", path = "../metadata/", default-features = false, features = ["derive"], optional = true } -ink_allocator = { version = "3.0.0-rc3", path = "../allocator/", default-features = false } +# ink_allocator = { version = "3.0.0-rc3", path = "../allocator/", default-features = false, features = ["wee-allocator"] } +ink_allocator = { version = "3.0.0-rc3", path = "../allocator/", default-features = false, features = ["bump-allocator"] } ink_primitives = { version = "3.0.0-rc3", path = "../primitives/", default-features = false } ink_prelude = { version = "3.0.0-rc3", path = "../prelude/", default-features = false } From b4bec3d5381fc8eb70260eb43a65dfc88f812069 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 28 Jun 2021 16:45:19 -0400 Subject: [PATCH 04/39] Remove unused `init()` function --- crates/allocator/src/bump.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 5601fbd8074..50d5286b4a4 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -16,6 +16,10 @@ //! //! It's goal to have a much smaller footprint than the admittedly more full-featured `wee_alloc` //! allocator which is currently being used by ink! smart contracts. +//! +//! The heap which will be used by this allocator is a single page of memory, which in Wasm is +//! 64KiB. We do not expect contracts to use more memory than this (for now), so we will throw an +//! OOM error instead of requesting more memory. use core::alloc::{GlobalAlloc, Layout}; @@ -45,7 +49,8 @@ impl Locked { pub struct BumpAllocator { /// Points to the start of the next available allocation next: usize, - /// Hacks + /// We need some way to initialize our heap. However, I can't figure out how to get the + /// initialization working properly with `lazy_static` so this hack is the best I got for now. heap_initialized: bool, } @@ -56,17 +61,6 @@ impl BumpAllocator { heap_initialized: false, } } - - /// Initalize the backing heap of the allocator. - // - // In this case we'll be backed by a page of Wasm memory which is all we'll use for the life of - // the contract. - pub fn init(&mut self) { - let ptr = core::arch::wasm32::memory_grow(0, 1); - if ptr == usize::max_value() { - todo!("TODO: OOM") - } - } } unsafe impl GlobalAlloc for Locked { @@ -78,8 +72,9 @@ unsafe impl GlobalAlloc for Locked { if !bump.heap_initialized { let ptr = core::arch::wasm32::memory_grow(0, 1); if ptr == usize::max_value() { - todo!("TODO: OOM") + // todo!("TODO: OOM") } + bump.heap_initialized = true; } let aligned_layout = layout.pad_to_align(); @@ -101,6 +96,6 @@ unsafe impl GlobalAlloc for Locked { } unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) { - todo!(); + // todo!(); } } From 0267f886d6266a61ca37bbc19ac933c83fdd5536 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 28 Jun 2021 17:32:57 -0400 Subject: [PATCH 05/39] Nightly RustFmt --- crates/allocator/src/bump.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 50d5286b4a4..aa934dda72b 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -21,7 +21,10 @@ //! 64KiB. We do not expect contracts to use more memory than this (for now), so we will throw an //! OOM error instead of requesting more memory. -use core::alloc::{GlobalAlloc, Layout}; +use core::alloc::{ + GlobalAlloc, + Layout, +}; /// A page in Wasm is 64KiB const PAGE_SIZE: usize = 64 * 1024; @@ -88,7 +91,7 @@ unsafe impl GlobalAlloc for Locked { // Since we're using a single page as our entire heap if we exceed it we're effectively // out-of-memory. if alloc_end > PAGE_SIZE { - return core::ptr::null_mut(); + return core::ptr::null_mut() } bump.next = alloc_end; From 5b87b12d61f6eae8c566463fb77c470ec814fdde Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 29 Jun 2021 12:48:51 -0400 Subject: [PATCH 06/39] Use global mutable static instead of Mutex This will reduce our use of dependencies which will hopefully reduce our final Wasm binary size. Also, apparently spinlocks aren't actually all that efficient. See: https://matklad.github.io/2020/01/02/spinlocks-considered-harmful.html --- crates/allocator/Cargo.toml | 4 +-- crates/allocator/src/bump.rs | 47 ++++++++++++------------------------ crates/allocator/src/lib.rs | 4 +-- 3 files changed, 17 insertions(+), 38 deletions(-) diff --git a/crates/allocator/Cargo.toml b/crates/allocator/Cargo.toml index ccaf1223a14..abc014b781d 100644 --- a/crates/allocator/Cargo.toml +++ b/crates/allocator/Cargo.toml @@ -16,11 +16,9 @@ include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] [dependencies] wee_alloc = { version = "0.4", default-features = false, optional = true } -spin = { version = "0.9", default-features = false, features = ["mutex", "spin_mutex"], optional = true } -lazy_static = { version = "1.4", default-features = false, features = ["spin_no_std"], optional = true } [features] default = ["std"] std = [] -bump-allocator = ["spin", "lazy_static"] +bump-allocator = [] wee-allocator = ["wee_alloc"] diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index aa934dda72b..2c88f5239de 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -29,60 +29,47 @@ use core::alloc::{ /// A page in Wasm is 64KiB const PAGE_SIZE: usize = 64 * 1024; -lazy_static::lazy_static! { - pub static ref BUMP_ALLOC: Locked = Locked::new(BumpAllocator::new()); -} +static mut INNER: InnerAlloc = InnerAlloc::new(); -pub struct Locked { - inner: spin::Mutex, -} +pub struct BumpAllocator; -impl Locked { - pub const fn new(inner: A) -> Self { - Locked { - inner: spin::Mutex::new(inner), - } +unsafe impl GlobalAlloc for BumpAllocator { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + INNER.alloc(layout) } - pub fn lock(&self) -> spin::MutexGuard { - self.inner.lock() - } + unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {} } -pub struct BumpAllocator { - /// Points to the start of the next available allocation +struct InnerAlloc { + /// Points to the start of the next available allocation. next: usize, /// We need some way to initialize our heap. However, I can't figure out how to get the /// initialization working properly with `lazy_static` so this hack is the best I got for now. heap_initialized: bool, } -impl BumpAllocator { +impl InnerAlloc { pub const fn new() -> Self { Self { next: 0, heap_initialized: false, } } -} - -unsafe impl GlobalAlloc for Locked { - unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - // This should be okay performance wise since we're in a single threaded context anyways - let mut bump = self.lock(); + unsafe fn alloc(&mut self, layout: Layout) -> *mut u8 { // TODO: Figure out how to properly initalize the heap - if !bump.heap_initialized { + if !self.heap_initialized { let ptr = core::arch::wasm32::memory_grow(0, 1); if ptr == usize::max_value() { - // todo!("TODO: OOM") + // todo!("OOM") } - bump.heap_initialized = true; + self.heap_initialized = true; } let aligned_layout = layout.pad_to_align(); - let alloc_start = bump.next; + let alloc_start = self.next; let alloc_end = match alloc_start.checked_add(aligned_layout.size()) { Some(end) => end, None => return core::ptr::null_mut(), @@ -94,11 +81,7 @@ unsafe impl GlobalAlloc for Locked { return core::ptr::null_mut() } - bump.next = alloc_end; + self.next = alloc_end; alloc_start as *mut u8 } - - unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) { - // todo!(); - } } diff --git a/crates/allocator/src/lib.rs b/crates/allocator/src/lib.rs index 1864ad2149b..172a6979a90 100644 --- a/crates/allocator/src/lib.rs +++ b/crates/allocator/src/lib.rs @@ -32,9 +32,7 @@ static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; #[cfg(feature = "bump-allocator")] #[cfg(not(feature = "wee-allocator"))] #[global_allocator] -static ALLOC: bump::Locked = - bump::Locked::new(bump::BumpAllocator::new()); -// static ALLOC: bump::BUMP_ALLOC = bump::BUMP_ALLOC; +static mut ALLOC: bump::BumpAllocator = bump::BumpAllocator {}; #[cfg(not(feature = "std"))] mod handlers; From 605aafe7a0c60171e4a8afa1dcce03ac7482c5b8 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 29 Jun 2021 17:07:03 -0400 Subject: [PATCH 07/39] Stop assuming that memory is allocated at address `0` --- crates/allocator/src/bump.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 2c88f5239de..2825dccccd8 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -43,33 +43,29 @@ unsafe impl GlobalAlloc for BumpAllocator { struct InnerAlloc { /// Points to the start of the next available allocation. - next: usize, - /// We need some way to initialize our heap. However, I can't figure out how to get the - /// initialization working properly with `lazy_static` so this hack is the best I got for now. - heap_initialized: bool, + /// + /// If the heap hasn't been initialized yet this value will be `None`. + next: Option, } impl InnerAlloc { pub const fn new() -> Self { - Self { - next: 0, - heap_initialized: false, - } + Self { next: None } } unsafe fn alloc(&mut self, layout: Layout) -> *mut u8 { // TODO: Figure out how to properly initalize the heap - if !self.heap_initialized { - let ptr = core::arch::wasm32::memory_grow(0, 1); - if ptr == usize::max_value() { - // todo!("OOM") + let alloc_start = if let Some(start) = self.next { + start; + } else { + let prev_page = core::arch::wasm32::memory_grow(0, 1); + if prev_page == usize::max_value() { + panic!("OOM") } - self.heap_initialized = true; - } + prev_page.checked_mul(PAGE_SIZE).expect("OOM") + }; let aligned_layout = layout.pad_to_align(); - - let alloc_start = self.next; let alloc_end = match alloc_start.checked_add(aligned_layout.size()) { Some(end) => end, None => return core::ptr::null_mut(), @@ -81,7 +77,7 @@ impl InnerAlloc { return core::ptr::null_mut() } - self.next = alloc_end; + self.next = Some(alloc_end); alloc_start as *mut u8 } } From de3095bd1033179d645da20d19f47cde14c5ea11 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 29 Jun 2021 17:12:28 -0400 Subject: [PATCH 08/39] Remove semicolon --- crates/allocator/src/bump.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 2825dccccd8..bcd5638c1a5 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -56,7 +56,7 @@ impl InnerAlloc { unsafe fn alloc(&mut self, layout: Layout) -> *mut u8 { // TODO: Figure out how to properly initalize the heap let alloc_start = if let Some(start) = self.next { - start; + start } else { let prev_page = core::arch::wasm32::memory_grow(0, 1); if prev_page == usize::max_value() { From d668dac52d71e11697c22c052ff73534690844de Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 29 Jun 2021 17:31:29 -0400 Subject: [PATCH 09/39] Use correct address when checking if we're OOM --- crates/allocator/src/bump.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index bcd5638c1a5..58261e712b9 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -46,11 +46,19 @@ struct InnerAlloc { /// /// If the heap hasn't been initialized yet this value will be `None`. next: Option, + + /// The address of the upper limit of our heap. + /// + /// If the heap hasn't been initialized yet this value will be `None`. + upper_limit: Option, } impl InnerAlloc { pub const fn new() -> Self { - Self { next: None } + Self { + next: None, + upper_limit: None, + } } unsafe fn alloc(&mut self, layout: Layout) -> *mut u8 { @@ -62,7 +70,10 @@ impl InnerAlloc { if prev_page == usize::max_value() { panic!("OOM") } - prev_page.checked_mul(PAGE_SIZE).expect("OOM") + + let start = prev_page.checked_mul(PAGE_SIZE).expect("OOM"); + self.upper_limit = Some(start + PAGE_SIZE); + start }; let aligned_layout = layout.pad_to_align(); @@ -71,9 +82,9 @@ impl InnerAlloc { None => return core::ptr::null_mut(), }; - // Since we're using a single page as our entire heap if we exceed it we're effectively - // out-of-memory. - if alloc_end > PAGE_SIZE { + let upper_limit = self.upper_limit + .expect("Since we're here the heap must've been initialized, therefore this must exist."); + if alloc_end > upper_limit { return core::ptr::null_mut() } From 1d776d933fd207b822760d2409ea1e6b9f9377c2 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Jun 2021 13:38:05 -0400 Subject: [PATCH 10/39] Remove unnecessary unsafe block --- crates/allocator/src/bump.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 58261e712b9..9f11190cbd6 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -61,7 +61,7 @@ impl InnerAlloc { } } - unsafe fn alloc(&mut self, layout: Layout) -> *mut u8 { + fn alloc(&mut self, layout: Layout) -> *mut u8 { // TODO: Figure out how to properly initalize the heap let alloc_start = if let Some(start) = self.next { start From 1ff4965e5c73afd5236c6aea2d60f827d4be719a Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Jun 2021 18:00:11 -0400 Subject: [PATCH 11/39] Return null pointers instead of panicking Panicking in the global allocator is considered undefined behaviour. --- crates/allocator/src/bump.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 9f11190cbd6..27e7f382f68 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -68,22 +68,26 @@ impl InnerAlloc { } else { let prev_page = core::arch::wasm32::memory_grow(0, 1); if prev_page == usize::max_value() { - panic!("OOM") + return core::ptr::null_mut() } - let start = prev_page.checked_mul(PAGE_SIZE).expect("OOM"); + let start = prev_page + .checked_mul(PAGE_SIZE) + .unwrap_or_else(|| return core::ptr::null_mut()); + self.upper_limit = Some(start + PAGE_SIZE); start }; let aligned_layout = layout.pad_to_align(); - let alloc_end = match alloc_start.checked_add(aligned_layout.size()) { - Some(end) => end, - None => return core::ptr::null_mut(), - }; + let alloc_end = alloc_start + .checked_add(aligned_layout.size()) + .unwrap_or_else(|| return core::ptr::null_mut()); + + let upper_limit = self + .upper_limit + .unwrap_or_else(|| return core::ptr::null_mut()); - let upper_limit = self.upper_limit - .expect("Since we're here the heap must've been initialized, therefore this must exist."); if alloc_end > upper_limit { return core::ptr::null_mut() } From 6bff7cda24dfb1253c2e64e114a2c99981e35326 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Jun 2021 18:04:21 -0400 Subject: [PATCH 12/39] Use `checked_add` when getting upper limit memory address --- crates/allocator/src/bump.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 27e7f382f68..1552b1a1bca 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -75,7 +75,10 @@ impl InnerAlloc { .checked_mul(PAGE_SIZE) .unwrap_or_else(|| return core::ptr::null_mut()); - self.upper_limit = Some(start + PAGE_SIZE); + self.upper_limit = start + .checked_add(PAGE_SIZE) + .map_or_else(|| return core::ptr::null_mut()); + start }; From 46f0e1d48c9a2cd751a23dbdc74503c026694501 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Jun 2021 18:05:44 -0400 Subject: [PATCH 13/39] Use `MAX` associated const instead of `max_value` --- crates/allocator/src/bump.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 1552b1a1bca..65c92ffcce5 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -67,7 +67,7 @@ impl InnerAlloc { start } else { let prev_page = core::arch::wasm32::memory_grow(0, 1); - if prev_page == usize::max_value() { + if prev_page == usize::MAX() { return core::ptr::null_mut() } From a608499a02e35b9ce693597d1d46b56ac986764c Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Jun 2021 18:21:02 -0400 Subject: [PATCH 14/39] Inline `GlobalAlloc` methods --- crates/allocator/src/bump.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 65c92ffcce5..5a34cfbd56d 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -34,10 +34,12 @@ static mut INNER: InnerAlloc = InnerAlloc::new(); pub struct BumpAllocator; unsafe impl GlobalAlloc for BumpAllocator { + #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { INNER.alloc(layout) } + #[inline] unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {} } @@ -54,7 +56,7 @@ struct InnerAlloc { } impl InnerAlloc { - pub const fn new() -> Self { + const fn new() -> Self { Self { next: None, upper_limit: None, From 52d05de4e3caf6267a8e37a7a4129dd2a697f5e8 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Jun 2021 18:35:29 -0400 Subject: [PATCH 15/39] =?UTF-8?q?Turns=20out=20I=20can't=20early=20return?= =?UTF-8?q?=20from=20`unwrap=5For=5Felse`=20=F0=9F=A4=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/allocator/src/bump.rs | 30 +++++++++++++++++------------- scripts/check-examples.sh | 16 ++++++++-------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 5a34cfbd56d..30e3252233c 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -69,29 +69,33 @@ impl InnerAlloc { start } else { let prev_page = core::arch::wasm32::memory_grow(0, 1); - if prev_page == usize::MAX() { + if prev_page == usize::MAX { return core::ptr::null_mut() } - let start = prev_page - .checked_mul(PAGE_SIZE) - .unwrap_or_else(|| return core::ptr::null_mut()); + let start = match prev_page.checked_mul(PAGE_SIZE) { + Some(s) => s, + None => return core::ptr::null_mut(), + }; - self.upper_limit = start - .checked_add(PAGE_SIZE) - .map_or_else(|| return core::ptr::null_mut()); + self.upper_limit = match start.checked_add(PAGE_SIZE) { + Some(u) => Some(u), + None => return core::ptr::null_mut(), + }; start }; let aligned_layout = layout.pad_to_align(); - let alloc_end = alloc_start - .checked_add(aligned_layout.size()) - .unwrap_or_else(|| return core::ptr::null_mut()); + let alloc_end = match alloc_start.checked_add(aligned_layout.size()) { + Some(end) => end, + None => return core::ptr::null_mut(), + }; - let upper_limit = self - .upper_limit - .unwrap_or_else(|| return core::ptr::null_mut()); + let upper_limit = match self.upper_limit { + Some(u) => u, + None => return core::ptr::null_mut(), + }; if alloc_end > upper_limit { return core::ptr::null_mut() diff --git a/scripts/check-examples.sh b/scripts/check-examples.sh index 45112944ab2..2200c98a4e2 100755 --- a/scripts/check-examples.sh +++ b/scripts/check-examples.sh @@ -71,26 +71,26 @@ metadata() { for example in $(ls -d examples/*/ | grep -v delegator); do build $example - run_tests $example - metadata $example + # run_tests $example + # metadata $example done # the delegator is a special case, we need to build it's sub-contracts first for example in $(ls -d examples/delegator/{accumulator,adder,subber}/); do - build $example - run_tests $example + # build $example + # run_tests $example done build examples/delegator/ -run_tests examples/delegator/ -metadata examples/delegator/ +# run_tests examples/delegator/ +# metadata examples/delegator/ banner="---------------" echo "Example Results" echo "$banner" for entry in ${!results_wasm[@]}; do echo "- $entry (wasm): ${results_wasm[$entry]}" - echo "- $entry (test): ${results_test[$entry]}" - echo "- $entry (metadata): ${results_metadata[$entry]}" + # echo "- $entry (test): ${results_test[$entry]}" + # echo "- $entry (metadata): ${results_metadata[$entry]}" done echo "" if [ $all_checks_passed -eq 0 ] From d382bae87a78646739eb72aff8db80689692c307 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Jun 2021 18:37:29 -0400 Subject: [PATCH 16/39] Rollback my build script hacks --- scripts/check-examples.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/check-examples.sh b/scripts/check-examples.sh index 2200c98a4e2..45112944ab2 100755 --- a/scripts/check-examples.sh +++ b/scripts/check-examples.sh @@ -71,26 +71,26 @@ metadata() { for example in $(ls -d examples/*/ | grep -v delegator); do build $example - # run_tests $example - # metadata $example + run_tests $example + metadata $example done # the delegator is a special case, we need to build it's sub-contracts first for example in $(ls -d examples/delegator/{accumulator,adder,subber}/); do - # build $example - # run_tests $example + build $example + run_tests $example done build examples/delegator/ -# run_tests examples/delegator/ -# metadata examples/delegator/ +run_tests examples/delegator/ +metadata examples/delegator/ banner="---------------" echo "Example Results" echo "$banner" for entry in ${!results_wasm[@]}; do echo "- $entry (wasm): ${results_wasm[$entry]}" - # echo "- $entry (test): ${results_test[$entry]}" - # echo "- $entry (metadata): ${results_metadata[$entry]}" + echo "- $entry (test): ${results_test[$entry]}" + echo "- $entry (metadata): ${results_metadata[$entry]}" done echo "" if [ $all_checks_passed -eq 0 ] From 1c62e7f4c80be6f90f9e8900d83bc3e6310e8284 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 1 Jul 2021 11:07:43 -0400 Subject: [PATCH 17/39] Add initialization function to allocator --- crates/allocator/src/bump.rs | 76 +++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 30e3252233c..17a2c324602 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -33,6 +33,19 @@ static mut INNER: InnerAlloc = InnerAlloc::new(); pub struct BumpAllocator; +impl BumpAllocator { + /// Initialize the backing heap of the bump allocator. + /// + /// This function must only be called **once**, and it **must** be called before any + /// allocations are made. + #[inline] + pub fn init(&self) { + // SAFETY: We are in a single threaded context, so we don't have to worry about this being + // concurrently mutated by multiple threads. + unsafe { INNER.init() } + } +} + unsafe impl GlobalAlloc for BumpAllocator { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { @@ -45,63 +58,54 @@ unsafe impl GlobalAlloc for BumpAllocator { struct InnerAlloc { /// Points to the start of the next available allocation. - /// - /// If the heap hasn't been initialized yet this value will be `None`. - next: Option, + next: usize, /// The address of the upper limit of our heap. - /// - /// If the heap hasn't been initialized yet this value will be `None`. - upper_limit: Option, + upper_limit: usize, } impl InnerAlloc { const fn new() -> Self { Self { - next: None, - upper_limit: None, + next: 0, + upper_limit: 0, } } - fn alloc(&mut self, layout: Layout) -> *mut u8 { - // TODO: Figure out how to properly initalize the heap - let alloc_start = if let Some(start) = self.next { - start - } else { - let prev_page = core::arch::wasm32::memory_grow(0, 1); - if prev_page == usize::MAX { - return core::ptr::null_mut() - } - - let start = match prev_page.checked_mul(PAGE_SIZE) { - Some(s) => s, - None => return core::ptr::null_mut(), - }; - - self.upper_limit = match start.checked_add(PAGE_SIZE) { - Some(u) => Some(u), - None => return core::ptr::null_mut(), - }; - - start + fn init(&mut self) { + let prev_page = core::arch::wasm32::memory_grow(0, 1); + if prev_page == usize::MAX { + return + } + + let start = match prev_page.checked_mul(PAGE_SIZE) { + Some(s) => s, + None => return, }; + self.upper_limit = match start.checked_add(PAGE_SIZE) { + Some(u) => u, + None => return, + }; + + self.next = start; + } + + /// Note: This function assumes that the allocator has already been initialized properly. + fn alloc(&mut self, layout: Layout) -> *mut u8 { + let alloc_start = self.next; + let aligned_layout = layout.pad_to_align(); let alloc_end = match alloc_start.checked_add(aligned_layout.size()) { Some(end) => end, None => return core::ptr::null_mut(), }; - let upper_limit = match self.upper_limit { - Some(u) => u, - None => return core::ptr::null_mut(), - }; - - if alloc_end > upper_limit { + if alloc_end > self.upper_limit { return core::ptr::null_mut() } - self.next = Some(alloc_end); + self.next = alloc_end; alloc_start as *mut u8 } } From 68741fe6819286dc4e4a2db10950b0e9d0a884b2 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 1 Jul 2021 11:26:56 -0400 Subject: [PATCH 18/39] Add some docs --- crates/allocator/src/bump.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 17a2c324602..e6ea45b861a 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -14,7 +14,7 @@ //! A simple bump allocator. //! -//! It's goal to have a much smaller footprint than the admittedly more full-featured `wee_alloc` +//! Its goal to have a much smaller footprint than the admittedly more full-featured `wee_alloc` //! allocator which is currently being used by ink! smart contracts. //! //! The heap which will be used by this allocator is a single page of memory, which in Wasm is @@ -31,6 +31,7 @@ const PAGE_SIZE: usize = 64 * 1024; static mut INNER: InnerAlloc = InnerAlloc::new(); +/// A bump allocator suitable for use in a Wasm environment. pub struct BumpAllocator; impl BumpAllocator { @@ -72,26 +73,33 @@ impl InnerAlloc { } } + /// Initialize the heap which backs the bump allocator. + /// + /// Our heap is a single page of Wasm memory (64KiB) and will not grow beyond that. + /// + /// Note that this function must be called before any allocations can take place, otherwise any + /// attempts to perform an allocation will fail. fn init(&mut self) { let prev_page = core::arch::wasm32::memory_grow(0, 1); if prev_page == usize::MAX { - return + todo!() } let start = match prev_page.checked_mul(PAGE_SIZE) { Some(s) => s, - None => return, + None => todo!(), }; self.upper_limit = match start.checked_add(PAGE_SIZE) { Some(u) => u, - None => return, + None => todo!(), }; self.next = start; } - /// Note: This function assumes that the allocator has already been initialized properly. + /// Note: This function assumes that the allocator has already been initialized properly (see + /// [Self::init()]. fn alloc(&mut self, layout: Layout) -> *mut u8 { let alloc_start = self.next; From 95adb7111bddd67d7bf675fc9daa9e4eaeaa556d Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 1 Jul 2021 11:43:51 -0400 Subject: [PATCH 19/39] Make the bump allocator the default allocator --- crates/allocator/Cargo.toml | 3 +-- crates/allocator/src/lib.rs | 20 +++++++++----------- crates/env/Cargo.toml | 4 ++-- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/crates/allocator/Cargo.toml b/crates/allocator/Cargo.toml index abc014b781d..76551a33894 100644 --- a/crates/allocator/Cargo.toml +++ b/crates/allocator/Cargo.toml @@ -20,5 +20,4 @@ wee_alloc = { version = "0.4", default-features = false, optional = true } [features] default = ["std"] std = [] -bump-allocator = [] -wee-allocator = ["wee_alloc"] +wee-alloc = ["wee_alloc"] diff --git a/crates/allocator/src/lib.rs b/crates/allocator/src/lib.rs index 172a6979a90..c40dce954bc 100644 --- a/crates/allocator/src/lib.rs +++ b/crates/allocator/src/lib.rs @@ -12,10 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Crate providing `WEE_ALLOC` support for all Wasm compilations of ink! smart contract. +//! Crate providing allocator support for all Wasm compilations of ink! smart contracts. //! -//! The Wee allocator is an allocator specifically designed to have a low footprint albeit -//! being less efficient for allocation and deallocation operations. +//! The default allocator is a bump allocator whose goal is to have a small size footprint. If you +//! are not concerned about the size of your final Wasm binaries you may opt into using the more +//! full-featured `wee_alloc` allocator by activating the `wee-alloc` crate feature. #![cfg_attr(not(feature = "std"), no_std)] #![cfg_attr(not(feature = "std"), feature(alloc_error_handler, core_intrinsics))] @@ -23,21 +24,18 @@ // We use `wee_alloc` as the global allocator since it is optimized for binary file size // so that contracts compiled with it as allocator do not grow too much in size. #[cfg(not(feature = "std"))] -#[cfg(feature = "wee-allocator")] -#[cfg(not(feature = "bump-allocator"))] +#[cfg(feature = "wee-alloc")] #[global_allocator] static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; #[cfg(not(feature = "std"))] -#[cfg(feature = "bump-allocator")] -#[cfg(not(feature = "wee-allocator"))] +#[cfg(not(feature = "wee-alloc"))] #[global_allocator] static mut ALLOC: bump::BumpAllocator = bump::BumpAllocator {}; #[cfg(not(feature = "std"))] -mod handlers; +#[cfg(not(feature = "wee-alloc"))] +mod bump; #[cfg(not(feature = "std"))] -#[cfg(feature = "bump-allocator")] -#[cfg(not(feature = "wee-allocator"))] -mod bump; +mod handlers; diff --git a/crates/env/Cargo.toml b/crates/env/Cargo.toml index 8efd50c0db4..40aeb08a3dd 100644 --- a/crates/env/Cargo.toml +++ b/crates/env/Cargo.toml @@ -17,8 +17,7 @@ include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] [dependencies] ink_engine = { version = "3.0.0-rc3", path = "../engine/", default-features = false, optional = true } ink_metadata = { version = "3.0.0-rc3", path = "../metadata/", default-features = false, features = ["derive"], optional = true } -# ink_allocator = { version = "3.0.0-rc3", path = "../allocator/", default-features = false, features = ["wee-allocator"] } -ink_allocator = { version = "3.0.0-rc3", path = "../allocator/", default-features = false, features = ["bump-allocator"] } +ink_allocator = { version = "3.0.0-rc3", path = "../allocator/", default-features = false } ink_primitives = { version = "3.0.0-rc3", path = "../primitives/", default-features = false } ink_prelude = { version = "3.0.0-rc3", path = "../prelude/", default-features = false } @@ -66,3 +65,4 @@ std = [ # Enable contract debug messages via `debug_print!` and `debug_println!`. ink-debug = [] ink-experimental-engine = ["ink_engine"] +wee-alloc = ["ink_allocator/wee-alloc"] From 80ee405840313cec1c8df4a6df37e4550f6d8a0d Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 6 Jul 2021 16:31:14 -0400 Subject: [PATCH 20/39] Allow bump allocator to be tested on Unix platforms --- crates/allocator/Cargo.toml | 8 ++- crates/allocator/src/bump.rs | 132 +++++++++++++++++++++++++++++------ crates/allocator/src/lib.rs | 2 - 3 files changed, 117 insertions(+), 25 deletions(-) diff --git a/crates/allocator/Cargo.toml b/crates/allocator/Cargo.toml index 76551a33894..5bb3e3f9f0a 100644 --- a/crates/allocator/Cargo.toml +++ b/crates/allocator/Cargo.toml @@ -15,9 +15,15 @@ categories = ["no-std", "embedded"] include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] [dependencies] +cfg-if = "1.0" wee_alloc = { version = "0.4", default-features = false, optional = true } +[target.'cfg(all(unix, not(target_arch = "wasm32")))'.dependencies.libc] +version = "0.2" +default-features = false +optional = true + [features] default = ["std"] -std = [] +std = ["libc/std"] wee-alloc = ["wee_alloc"] diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index e6ea45b861a..32007efd51a 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -73,34 +73,92 @@ impl InnerAlloc { } } - /// Initialize the heap which backs the bump allocator. - /// - /// Our heap is a single page of Wasm memory (64KiB) and will not grow beyond that. - /// - /// Note that this function must be called before any allocations can take place, otherwise any - /// attempts to perform an allocation will fail. - fn init(&mut self) { - let prev_page = core::arch::wasm32::memory_grow(0, 1); - if prev_page == usize::MAX { - todo!() + cfg_if::cfg_if! { + if #[cfg(all(not(feature = "std"), target_arch = "wasm32"))] { + /// Initialize the heap which backs the bump allocator. + /// + /// Our heap is a single page of Wasm memory (64KiB) and will not grow beyond that. + /// + /// Note that this function must be called before any allocations can take place, otherwise any + /// attempts to perform an allocation will fail. + fn init(&mut self) { + let prev_page = core::arch::wasm32::memory_grow(0, 1); + if prev_page == usize::MAX { + todo!() + } + + let start = match prev_page.checked_mul(PAGE_SIZE) { + Some(s) => s, + None => todo!(), + }; + + self.upper_limit = match start.checked_add(PAGE_SIZE) { + Some(u) => u, + None => todo!(), + }; + + self.next = start; + } + + } else if #[cfg(all(feature = "std", unix))] { + /// Initialize the heap which backs the bump allocator. + /// + /// Our heap is 64KiB of memory (to match the size of a Wasm page), and will not grow + /// beyond that. + /// + /// Note that this function must be called before any allocations can take place, otherwise any + /// attempts to perform an allocation will fail. + /// + /// This implementation is only meant to be used for testing, since we cannot (easily) + /// test the `wasm32` implementation. + fn init(&mut self) { + let start = unsafe { + let protection_bits = libc::PROT_WRITE | libc::PROT_READ; + let flags = libc::MAP_ANONYMOUS | libc::MAP_PRIVATE; + let fd = -1; + let offset = 0; + + // _Technically_ the `PAGE_SIZE` here will more than likely *not* match the page + // size of our non-wasm32 architecture, but it's fine to request that many bytes + // from `mmap`. + libc::mmap( + core::ptr::null_mut(), + PAGE_SIZE, + protection_bits, + flags, + fd, + offset, + ) + }; + + if start == libc::MAP_FAILED { + panic!("`mmap` failed to allocate memory.") + } + + let start = start as usize; + + self.upper_limit = match start.checked_add(PAGE_SIZE) { + Some(u) => u, + None => todo!(), + }; + + self.next = start; + } + } else { + compile_error! { + "ink! only supports compilation as `std` or `no_std` + `wasm32-unknown`" + } } - - let start = match prev_page.checked_mul(PAGE_SIZE) { - Some(s) => s, - None => todo!(), - }; - - self.upper_limit = match start.checked_add(PAGE_SIZE) { - Some(u) => u, - None => todo!(), - }; - - self.next = start; } /// Note: This function assumes that the allocator has already been initialized properly (see /// [Self::init()]. fn alloc(&mut self, layout: Layout) -> *mut u8 { + // TODO: Init properly + unsafe { + INNER.init(); + } + let alloc_start = self.next; let aligned_layout = layout.pad_to_align(); @@ -117,3 +175,33 @@ impl InnerAlloc { alloc_start as *mut u8 } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn can_alloc_a_box() { + let _b = Box::new(1); + } + + #[test] + fn can_alloc_a_vec() { + let mut v = Vec::new(); + v.push(1) + } + + #[test] + fn can_alloc_a_big_vec() { + let mut v = Vec::with_capacity(PAGE_SIZE); + v.push(true) + } + + // TODO: This fails, as expected, but I get `SIGABRT`-ed, need to figure out how to set up a + // handler to deal with this correctly + // #[test] + // fn cannot_alloc_a_bigger_vec() { + // let mut v = Vec::with_capacity(PAGE_SIZE + 1); + // v.push(true) + // } +} diff --git a/crates/allocator/src/lib.rs b/crates/allocator/src/lib.rs index c40dce954bc..7b8ac70317f 100644 --- a/crates/allocator/src/lib.rs +++ b/crates/allocator/src/lib.rs @@ -28,12 +28,10 @@ #[global_allocator] static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; -#[cfg(not(feature = "std"))] #[cfg(not(feature = "wee-alloc"))] #[global_allocator] static mut ALLOC: bump::BumpAllocator = bump::BumpAllocator {}; -#[cfg(not(feature = "std"))] #[cfg(not(feature = "wee-alloc"))] mod bump; From 767f234f9450266a32dbe3584d3d24bf60402714 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 6 Jul 2021 16:52:16 -0400 Subject: [PATCH 21/39] Remove unecessary checked_add --- crates/allocator/src/bump.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 32007efd51a..848c5da8409 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -136,12 +136,7 @@ impl InnerAlloc { } let start = start as usize; - - self.upper_limit = match start.checked_add(PAGE_SIZE) { - Some(u) => u, - None => todo!(), - }; - + self.upper_limit = start + PAGE_SIZE; self.next = start; } } else { From 53af55037ab3139a000eaf5d7a6b544877c57621 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 6 Jul 2021 17:20:05 -0400 Subject: [PATCH 22/39] Add error messages to unrecoverable errors --- crates/allocator/src/bump.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 848c5da8409..8f67c6bf78f 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -84,17 +84,17 @@ impl InnerAlloc { fn init(&mut self) { let prev_page = core::arch::wasm32::memory_grow(0, 1); if prev_page == usize::MAX { - todo!() + panic!("Unable to grow Wasm memory.") } let start = match prev_page.checked_mul(PAGE_SIZE) { Some(s) => s, - None => todo!(), + None => panic!("Start of page boundary is invalid."), }; self.upper_limit = match start.checked_add(PAGE_SIZE) { Some(u) => u, - None => todo!(), + None => panic!("Not enough memory left to allocate Wasm page."), }; self.next = start; From fb49543bb2ced2a7849fddaa50445f251bfd2b57 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 7 Jul 2021 18:31:08 -0400 Subject: [PATCH 23/39] Remove `init` function from allocator Instead we now request a new page whenver we need it, regardless of whether or not it's the first time we're allocating memory. --- crates/allocator/src/bump.rs | 75 +++++++++++++----------------------- 1 file changed, 26 insertions(+), 49 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 8f67c6bf78f..ca13c63a771 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -34,23 +34,13 @@ static mut INNER: InnerAlloc = InnerAlloc::new(); /// A bump allocator suitable for use in a Wasm environment. pub struct BumpAllocator; -impl BumpAllocator { - /// Initialize the backing heap of the bump allocator. - /// - /// This function must only be called **once**, and it **must** be called before any - /// allocations are made. - #[inline] - pub fn init(&self) { - // SAFETY: We are in a single threaded context, so we don't have to worry about this being - // concurrently mutated by multiple threads. - unsafe { INNER.init() } - } -} - unsafe impl GlobalAlloc for BumpAllocator { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - INNER.alloc(layout) + match INNER.alloc(layout) { + Some(start) => start as *mut u8, + None => core::ptr::null_mut(), + } } #[inline] @@ -75,29 +65,16 @@ impl InnerAlloc { cfg_if::cfg_if! { if #[cfg(all(not(feature = "std"), target_arch = "wasm32"))] { - /// Initialize the heap which backs the bump allocator. - /// - /// Our heap is a single page of Wasm memory (64KiB) and will not grow beyond that. + /// Request a new page of Wasm memory (64KiB). /// - /// Note that this function must be called before any allocations can take place, otherwise any - /// attempts to perform an allocation will fail. - fn init(&mut self) { + /// Returns `None` if a page isn't available. + fn request_page(&mut self) -> Option { let prev_page = core::arch::wasm32::memory_grow(0, 1); if prev_page == usize::MAX { - panic!("Unable to grow Wasm memory.") + return None; } - let start = match prev_page.checked_mul(PAGE_SIZE) { - Some(s) => s, - None => panic!("Start of page boundary is invalid."), - }; - - self.upper_limit = match start.checked_add(PAGE_SIZE) { - Some(u) => u, - None => panic!("Not enough memory left to allocate Wasm page."), - }; - - self.next = start; + prev_page.checked_mul(PAGE_SIZE) } } else if #[cfg(all(feature = "std", unix))] { @@ -111,7 +88,10 @@ impl InnerAlloc { /// /// This implementation is only meant to be used for testing, since we cannot (easily) /// test the `wasm32` implementation. - fn init(&mut self) { + fn request_page(&mut self) -> Option { + // TODO + return None; + let start = unsafe { let protection_bits = libc::PROT_WRITE | libc::PROT_READ; let flags = libc::MAP_ANONYMOUS | libc::MAP_PRIVATE; @@ -146,28 +126,25 @@ impl InnerAlloc { } } - /// Note: This function assumes that the allocator has already been initialized properly (see - /// [Self::init()]. - fn alloc(&mut self, layout: Layout) -> *mut u8 { - // TODO: Init properly - unsafe { - INNER.init(); - } - + /// Tries to allocate enough memory on the heap for the given `Layout`. If there isn't enough + /// room on the heap it'll try and grow it by a page. + // NOTE: I think we'll end up with fragmentation here + fn alloc(&mut self, layout: Layout) -> Option { let alloc_start = self.next; let aligned_layout = layout.pad_to_align(); - let alloc_end = match alloc_start.checked_add(aligned_layout.size()) { - Some(end) => end, - None => return core::ptr::null_mut(), - }; + let alloc_end = alloc_start.checked_add(aligned_layout.size())?; if alloc_end > self.upper_limit { - return core::ptr::null_mut() - } + let alloc_start = self.request_page()?; + self.upper_limit = alloc_start.checked_add(PAGE_SIZE)?; + self.next = alloc_start.checked_add(aligned_layout.size())?; - self.next = alloc_end; - alloc_start as *mut u8 + Some(alloc_start) + } else { + self.next = alloc_end; + Some(alloc_start) + } } } From b1c4de13f71f1ca541bb0d8f9762577f7750f22a Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 7 Jul 2021 19:18:38 -0400 Subject: [PATCH 24/39] Try switching from `mmap` to `malloc` when in `std` env --- crates/allocator/src/bump.rs | 61 ++++++++++-------------------------- 1 file changed, 17 insertions(+), 44 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index ca13c63a771..94907e0ac52 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -78,46 +78,21 @@ impl InnerAlloc { } } else if #[cfg(all(feature = "std", unix))] { - /// Initialize the heap which backs the bump allocator. + /// Request a new Wasm page sized section (64KiB) of memory. /// - /// Our heap is 64KiB of memory (to match the size of a Wasm page), and will not grow - /// beyond that. - /// - /// Note that this function must be called before any allocations can take place, otherwise any - /// attempts to perform an allocation will fail. + /// Returns `None` if a page isn't available. /// /// This implementation is only meant to be used for testing, since we cannot (easily) /// test the `wasm32` implementation. fn request_page(&mut self) -> Option { - // TODO - return None; - + // _Technically_ the `PAGE_SIZE` here will more than likely *not* match the page + // size of our non-wasm32 architecture, but it's fine to request that many bytes + // from `malloc`. let start = unsafe { - let protection_bits = libc::PROT_WRITE | libc::PROT_READ; - let flags = libc::MAP_ANONYMOUS | libc::MAP_PRIVATE; - let fd = -1; - let offset = 0; - - // _Technically_ the `PAGE_SIZE` here will more than likely *not* match the page - // size of our non-wasm32 architecture, but it's fine to request that many bytes - // from `mmap`. - libc::mmap( - core::ptr::null_mut(), - PAGE_SIZE, - protection_bits, - flags, - fd, - offset, - ) + libc::malloc(PAGE_SIZE) }; - if start == libc::MAP_FAILED { - panic!("`mmap` failed to allocate memory.") - } - - let start = start as usize; - self.upper_limit = start + PAGE_SIZE; - self.next = start; + start.is_null().then(|| start as usize) } } else { compile_error! { @@ -150,24 +125,22 @@ impl InnerAlloc { #[cfg(test)] mod tests { - use super::*; - #[test] fn can_alloc_a_box() { let _b = Box::new(1); } - #[test] - fn can_alloc_a_vec() { - let mut v = Vec::new(); - v.push(1) - } + // #[test] + // fn can_alloc_a_vec() { + // let mut v = Vec::new(); + // v.push(1) + // } - #[test] - fn can_alloc_a_big_vec() { - let mut v = Vec::with_capacity(PAGE_SIZE); - v.push(true) - } + // #[test] + // fn can_alloc_a_big_vec() { + // let mut v = Vec::with_capacity(PAGE_SIZE); + // v.push(true) + // } // TODO: This fails, as expected, but I get `SIGABRT`-ed, need to figure out how to set up a // handler to deal with this correctly From 3ea81ccd47859ad9f1d67e327bab0cec98a4ae9b Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 9 Jul 2021 11:25:25 -0400 Subject: [PATCH 25/39] Fix `is_null()` check when requesting memory --- crates/allocator/src/bump.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 94907e0ac52..35481ef63be 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -92,7 +92,7 @@ impl InnerAlloc { libc::malloc(PAGE_SIZE) }; - start.is_null().then(|| start as usize) + (!start.is_null()).then(|| start as usize) } } else { compile_error! { @@ -111,11 +111,11 @@ impl InnerAlloc { let alloc_end = alloc_start.checked_add(aligned_layout.size())?; if alloc_end > self.upper_limit { - let alloc_start = self.request_page()?; - self.upper_limit = alloc_start.checked_add(PAGE_SIZE)?; - self.next = alloc_start.checked_add(aligned_layout.size())?; + let page_start = self.request_page()?; + self.upper_limit = page_start.checked_add(PAGE_SIZE)?; + self.next = page_start.checked_add(aligned_layout.size())?; - Some(alloc_start) + Some(page_start) } else { self.next = alloc_end; Some(alloc_start) @@ -127,7 +127,7 @@ impl InnerAlloc { mod tests { #[test] fn can_alloc_a_box() { - let _b = Box::new(1); + // let _b = Box::new(1); } // #[test] From 509c9c4d89d139393c613cd535fa731994bf7274 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 9 Jul 2021 16:57:37 -0400 Subject: [PATCH 26/39] Stop requesting real memory for `std` testing Instead this tracks pages internally in the same way that the Wasm environment would. This means we can test our allocator implementation instead of fighting with `libc`. --- crates/allocator/Cargo.toml | 7 +- crates/allocator/src/bump.rs | 130 +++++++++++++++++++++++++++-------- crates/allocator/src/lib.rs | 1 + 3 files changed, 102 insertions(+), 36 deletions(-) diff --git a/crates/allocator/Cargo.toml b/crates/allocator/Cargo.toml index 5bb3e3f9f0a..97a120d4ed1 100644 --- a/crates/allocator/Cargo.toml +++ b/crates/allocator/Cargo.toml @@ -18,12 +18,7 @@ include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] cfg-if = "1.0" wee_alloc = { version = "0.4", default-features = false, optional = true } -[target.'cfg(all(unix, not(target_arch = "wasm32")))'.dependencies.libc] -version = "0.2" -default-features = false -optional = true - [features] default = ["std"] -std = ["libc/std"] +std = [] wee-alloc = ["wee_alloc"] diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 35481ef63be..e7db8160bf3 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -47,12 +47,17 @@ unsafe impl GlobalAlloc for BumpAllocator { unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {} } +#[cfg_attr(feature = "std", derive(Debug, Copy, Clone))] struct InnerAlloc { /// Points to the start of the next available allocation. next: usize, /// The address of the upper limit of our heap. upper_limit: usize, + + /// How many page requests have we made? + #[cfg(feature = "std")] + page_requests: usize, } impl InnerAlloc { @@ -60,6 +65,8 @@ impl InnerAlloc { Self { next: 0, upper_limit: 0, + #[cfg(feature = "std")] + page_requests: 0, } } @@ -85,14 +92,9 @@ impl InnerAlloc { /// This implementation is only meant to be used for testing, since we cannot (easily) /// test the `wasm32` implementation. fn request_page(&mut self) -> Option { - // _Technically_ the `PAGE_SIZE` here will more than likely *not* match the page - // size of our non-wasm32 architecture, but it's fine to request that many bytes - // from `malloc`. - let start = unsafe { - libc::malloc(PAGE_SIZE) - }; - - (!start.is_null()).then(|| start as usize) + let prev_page = self.page_requests.checked_mul(PAGE_SIZE); + self.page_requests += 1; + prev_page } } else { compile_error! { @@ -107,7 +109,7 @@ impl InnerAlloc { fn alloc(&mut self, layout: Layout) -> Option { let alloc_start = self.next; - let aligned_layout = layout.pad_to_align(); + let aligned_layout = dbg!(layout.pad_to_align()); let alloc_end = alloc_start.checked_add(aligned_layout.size())?; if alloc_end > self.upper_limit { @@ -125,28 +127,96 @@ impl InnerAlloc { #[cfg(test)] mod tests { + use super::*; + #[test] - fn can_alloc_a_box() { - // let _b = Box::new(1); + fn can_alloc_a_byte() { + let mut inner = InnerAlloc::new(); + + let layout = Layout::new::(); + assert!(inner.alloc(layout).is_some()); + + let expected_limit = inner.page_requests * PAGE_SIZE; + assert_eq!(inner.upper_limit, expected_limit); + + let expected_alloc_start = 1 * std::mem::size_of::(); + assert_eq!(inner.next, expected_alloc_start); + } + + #[test] + fn can_alloc_a_foobarbaz() { + let mut inner = InnerAlloc::new(); + + struct FooBarBaz { + _foo: u32, + _bar: u128, + _baz: (u16, bool), + } + + let layout = Layout::new::(); + + let allocations = 3; + for _ in 0..allocations { + assert!(inner.alloc(layout).is_some()); + } + + let expected_limit = inner.page_requests * PAGE_SIZE; + assert_eq!(inner.upper_limit, expected_limit); + + let expected_alloc_start = allocations * std::mem::size_of::(); + assert_eq!(inner.next, expected_alloc_start); } - // #[test] - // fn can_alloc_a_vec() { - // let mut v = Vec::new(); - // v.push(1) - // } - - // #[test] - // fn can_alloc_a_big_vec() { - // let mut v = Vec::with_capacity(PAGE_SIZE); - // v.push(true) - // } - - // TODO: This fails, as expected, but I get `SIGABRT`-ed, need to figure out how to set up a - // handler to deal with this correctly - // #[test] - // fn cannot_alloc_a_bigger_vec() { - // let mut v = Vec::with_capacity(PAGE_SIZE + 1); - // v.push(true) - // } + #[test] + fn can_alloc_across_pages() { + let mut inner = InnerAlloc::new(); + + struct Foo { + _foo: [u8; PAGE_SIZE - 1], + } + + let layout = Layout::new::(); + dbg!(layout); + + assert!(inner.alloc(layout).is_some()); + + let expected_limit = inner.page_requests * PAGE_SIZE; + assert_eq!(inner.upper_limit, expected_limit); + + let expected_alloc_start = 1 * std::mem::size_of::(); + assert_eq!(inner.next, expected_alloc_start); + + dbg!(inner); + + // Since this is two bytes it'll push us over to the next page + let layout = Layout::new::(); + assert!(inner.alloc(layout).is_some()); + + let expected_limit = inner.page_requests * PAGE_SIZE; + assert_eq!(inner.upper_limit, expected_limit); + + // TODO: Fix size hack + let expected_alloc_start = + 1 * std::mem::size_of::() + 1 * std::mem::size_of::() + 1; + assert_eq!(inner.next, expected_alloc_start); + } + + // TODO: Don't think this actually quite works as expected at the moment... + #[test] + fn can_alloc_multiple_pages() { + let mut inner = InnerAlloc::new(); + + struct Foo { + _foo: [u8; 2 * PAGE_SIZE - 1], + } + + let layout = Layout::new::(); + assert!(inner.alloc(layout).is_some()); + + let expected_limit = inner.page_requests * PAGE_SIZE; + assert_eq!(inner.upper_limit, expected_limit); + + let expected_alloc_start = 1 * std::mem::size_of::(); + assert_eq!(inner.next, expected_alloc_start); + } } diff --git a/crates/allocator/src/lib.rs b/crates/allocator/src/lib.rs index 7b8ac70317f..c7b3b75c471 100644 --- a/crates/allocator/src/lib.rs +++ b/crates/allocator/src/lib.rs @@ -29,6 +29,7 @@ static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; #[cfg(not(feature = "wee-alloc"))] +#[cfg(not(test))] #[global_allocator] static mut ALLOC: bump::BumpAllocator = bump::BumpAllocator {}; From 8e8af79baf7a69b838a5bd34a76ca0006e9e66ee Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 12:08:39 -0400 Subject: [PATCH 27/39] Gate the global bump allocator when not in `std` --- crates/allocator/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/allocator/src/lib.rs b/crates/allocator/src/lib.rs index c7b3b75c471..79a427a03d0 100644 --- a/crates/allocator/src/lib.rs +++ b/crates/allocator/src/lib.rs @@ -28,8 +28,8 @@ #[global_allocator] static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; +#[cfg(not(feature = "std"))] #[cfg(not(feature = "wee-alloc"))] -#[cfg(not(test))] #[global_allocator] static mut ALLOC: bump::BumpAllocator = bump::BumpAllocator {}; From 22082291cb8181238d65473769fd8b09cf35710a Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 12:09:09 -0400 Subject: [PATCH 28/39] Allow for multi-page allocations --- crates/allocator/src/bump.rs | 70 +++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index e7db8160bf3..6d798def9c1 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -72,11 +72,11 @@ impl InnerAlloc { cfg_if::cfg_if! { if #[cfg(all(not(feature = "std"), target_arch = "wasm32"))] { - /// Request a new page of Wasm memory (64KiB). + /// Request a `pages` number of pages of Wasm memory. Each page is 64KiB in size. /// /// Returns `None` if a page isn't available. - fn request_page(&mut self) -> Option { - let prev_page = core::arch::wasm32::memory_grow(0, 1); + fn request_pages(&mut self, pages: usize) -> Option { + let prev_page = core::arch::wasm32::memory_grow(0, pages); if prev_page == usize::MAX { return None; } @@ -84,16 +84,16 @@ impl InnerAlloc { prev_page.checked_mul(PAGE_SIZE) } - } else if #[cfg(all(feature = "std", unix))] { - /// Request a new Wasm page sized section (64KiB) of memory. + } else if #[cfg(feature = "std")] { + /// Request a `pages` number of page sized sections of Wasm memory. Each page is 64KiB in size. /// /// Returns `None` if a page isn't available. /// /// This implementation is only meant to be used for testing, since we cannot (easily) /// test the `wasm32` implementation. - fn request_page(&mut self) -> Option { + fn request_page(&mut self, pages: usize) -> Option { let prev_page = self.page_requests.checked_mul(PAGE_SIZE); - self.page_requests += 1; + self.page_requests += pages; prev_page } } else { @@ -105,17 +105,22 @@ impl InnerAlloc { /// Tries to allocate enough memory on the heap for the given `Layout`. If there isn't enough /// room on the heap it'll try and grow it by a page. - // NOTE: I think we'll end up with fragmentation here + /// + /// Note: This implementation results in internal fragmentation when allocating across pages. fn alloc(&mut self, layout: Layout) -> Option { let alloc_start = self.next; - let aligned_layout = dbg!(layout.pad_to_align()); - let alloc_end = alloc_start.checked_add(aligned_layout.size())?; + let aligned_size = layout.pad_to_align().size(); + let alloc_end = alloc_start.checked_add(aligned_size)?; if alloc_end > self.upper_limit { - let page_start = self.request_page()?; - self.upper_limit = page_start.checked_add(PAGE_SIZE)?; - self.next = page_start.checked_add(aligned_layout.size())?; + let required_pages = (aligned_size + PAGE_SIZE - 1) / PAGE_SIZE; + let page_start = self.request_page(required_pages)?; + + self.upper_limit = required_pages + .checked_mul(PAGE_SIZE) + .and_then(|pages| page_start.checked_add(pages))?; + self.next = page_start.checked_add(aligned_size)?; Some(page_start) } else { @@ -139,7 +144,7 @@ mod tests { let expected_limit = inner.page_requests * PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); - let expected_alloc_start = 1 * std::mem::size_of::(); + let expected_alloc_start = std::mem::size_of::(); assert_eq!(inner.next, expected_alloc_start); } @@ -175,48 +180,55 @@ mod tests { _foo: [u8; PAGE_SIZE - 1], } + // First, let's allocate a struct which is _almost_ a full page let layout = Layout::new::(); - dbg!(layout); - - assert!(inner.alloc(layout).is_some()); + assert_eq!(inner.alloc(layout), Some(0)); let expected_limit = inner.page_requests * PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); - let expected_alloc_start = 1 * std::mem::size_of::(); + let expected_alloc_start = std::mem::size_of::(); assert_eq!(inner.next, expected_alloc_start); - dbg!(inner); - - // Since this is two bytes it'll push us over to the next page + // Now we'll allocate two bytes which will push us over to the next page let layout = Layout::new::(); - assert!(inner.alloc(layout).is_some()); + assert_eq!(inner.alloc(layout), Some(PAGE_SIZE)); let expected_limit = inner.page_requests * PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); - // TODO: Fix size hack - let expected_alloc_start = - 1 * std::mem::size_of::() + 1 * std::mem::size_of::() + 1; + // Notice that we start the allocation on the second page, instead of making use of the + // remaining byte on the first page + let expected_alloc_start = PAGE_SIZE + std::mem::size_of::(); assert_eq!(inner.next, expected_alloc_start); } - // TODO: Don't think this actually quite works as expected at the moment... #[test] fn can_alloc_multiple_pages() { let mut inner = InnerAlloc::new(); struct Foo { - _foo: [u8; 2 * PAGE_SIZE - 1], + _foo: [u8; 2 * PAGE_SIZE], } let layout = Layout::new::(); - assert!(inner.alloc(layout).is_some()); + assert_eq!(inner.alloc(layout), Some(0)); + + let expected_limit = inner.page_requests * PAGE_SIZE; + assert_eq!(inner.upper_limit, expected_limit); + + let expected_alloc_start = std::mem::size_of::(); + assert_eq!(inner.next, expected_alloc_start); + + // Now we want to make sure that the state of our allocator is correct for any subsequent + // allocations + let layout = Layout::new::(); + assert_eq!(inner.alloc(layout), Some(2 * PAGE_SIZE)); let expected_limit = inner.page_requests * PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); - let expected_alloc_start = 1 * std::mem::size_of::(); + let expected_alloc_start = 2 * PAGE_SIZE + std::mem::size_of::(); assert_eq!(inner.next, expected_alloc_start); } } From f0425265e063e9f663bf0130bc6e8e65da61d78b Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 12:14:39 -0400 Subject: [PATCH 29/39] Update the module documentation --- crates/allocator/src/bump.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 6d798def9c1..f78b418670b 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -17,9 +17,9 @@ //! Its goal to have a much smaller footprint than the admittedly more full-featured `wee_alloc` //! allocator which is currently being used by ink! smart contracts. //! -//! The heap which will be used by this allocator is a single page of memory, which in Wasm is -//! 64KiB. We do not expect contracts to use more memory than this (for now), so we will throw an -//! OOM error instead of requesting more memory. +//! The heap which is used by this allocator is built from pages of Wasm memory (each page is 64KiB). +//! We will request new pages of memory as needed until we run out of memory, at which point we +//! will crash with an OOM error instead of freeing any memory. use core::alloc::{ GlobalAlloc, @@ -55,7 +55,9 @@ struct InnerAlloc { /// The address of the upper limit of our heap. upper_limit: usize, - /// How many page requests have we made? + /// The number of page requests made so far. + /// + /// This is meant to mimic the behaviour exhibited by `core::arch::wasm32::memory_grow` #[cfg(feature = "std")] page_requests: usize, } From bcf728b9f496228dae0afa63589e57b0bd629147 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 12:30:48 -0400 Subject: [PATCH 30/39] Override `alloc_zeroed` implementation --- crates/allocator/src/bump.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index f78b418670b..063cb4dc2f1 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -43,6 +43,13 @@ unsafe impl GlobalAlloc for BumpAllocator { } } + #[inline] + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + // A new page in Wasm is guaranteed to already be zero initialized, so we can just use our + // regular `alloc` call here and save a bit of work. + self.alloc(layout) + } + #[inline] unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {} } From ae097bfde1c18a0073e78cfe94478530b18b0c56 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 12:37:05 -0400 Subject: [PATCH 31/39] Forgot to update Wasm target function name --- crates/allocator/src/bump.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 063cb4dc2f1..3bad39e29b1 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -100,7 +100,7 @@ impl InnerAlloc { /// /// This implementation is only meant to be used for testing, since we cannot (easily) /// test the `wasm32` implementation. - fn request_page(&mut self, pages: usize) -> Option { + fn request_pages(&mut self, pages: usize) -> Option { let prev_page = self.page_requests.checked_mul(PAGE_SIZE); self.page_requests += pages; prev_page @@ -124,7 +124,7 @@ impl InnerAlloc { if alloc_end > self.upper_limit { let required_pages = (aligned_size + PAGE_SIZE - 1) / PAGE_SIZE; - let page_start = self.request_page(required_pages)?; + let page_start = self.request_pages(required_pages)?; self.upper_limit = required_pages .checked_mul(PAGE_SIZE) From f4e6a317bf90042335b867b3194840b2df2756c5 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 12:43:48 -0400 Subject: [PATCH 32/39] Appease the spellchecker --- .config/cargo_spellcheck.dic | 1 + crates/allocator/src/bump.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.config/cargo_spellcheck.dic b/.config/cargo_spellcheck.dic index e93a999382f..e49d139e743 100644 --- a/.config/cargo_spellcheck.dic +++ b/.config/cargo_spellcheck.dic @@ -42,6 +42,7 @@ getter growable interoperate invariants +isn't kB layed multisig diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 3bad39e29b1..0d46937bb51 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -17,16 +17,16 @@ //! Its goal to have a much smaller footprint than the admittedly more full-featured `wee_alloc` //! allocator which is currently being used by ink! smart contracts. //! -//! The heap which is used by this allocator is built from pages of Wasm memory (each page is 64KiB). +//! The heap which is used by this allocator is built from pages of Wasm memory (each page is `64KiB`). //! We will request new pages of memory as needed until we run out of memory, at which point we -//! will crash with an OOM error instead of freeing any memory. +//! will crash with an `OOM` error instead of freeing any memory. use core::alloc::{ GlobalAlloc, Layout, }; -/// A page in Wasm is 64KiB +/// A page in Wasm is `64KiB` const PAGE_SIZE: usize = 64 * 1024; static mut INNER: InnerAlloc = InnerAlloc::new(); @@ -64,7 +64,7 @@ struct InnerAlloc { /// The number of page requests made so far. /// - /// This is meant to mimic the behaviour exhibited by `core::arch::wasm32::memory_grow` + /// This is meant to mimic the behavior exhibited by `core::arch::wasm32::memory_grow` #[cfg(feature = "std")] page_requests: usize, } @@ -81,7 +81,7 @@ impl InnerAlloc { cfg_if::cfg_if! { if #[cfg(all(not(feature = "std"), target_arch = "wasm32"))] { - /// Request a `pages` number of pages of Wasm memory. Each page is 64KiB in size. + /// Request a `pages` number of pages of Wasm memory. Each page is `64KiB` in size. /// /// Returns `None` if a page isn't available. fn request_pages(&mut self, pages: usize) -> Option { @@ -94,7 +94,7 @@ impl InnerAlloc { } } else if #[cfg(feature = "std")] { - /// Request a `pages` number of page sized sections of Wasm memory. Each page is 64KiB in size. + /// Request a `pages` number of page sized sections of Wasm memory. Each page is `64KiB` in size. /// /// Returns `None` if a page isn't available. /// From baceccd494c57737ea1b4032c535f9367626d9c5 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 12:46:29 -0400 Subject: [PATCH 33/39] Use proper English I guess --- .config/cargo_spellcheck.dic | 1 - crates/allocator/src/bump.rs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.config/cargo_spellcheck.dic b/.config/cargo_spellcheck.dic index e49d139e743..e93a999382f 100644 --- a/.config/cargo_spellcheck.dic +++ b/.config/cargo_spellcheck.dic @@ -42,7 +42,6 @@ getter growable interoperate invariants -isn't kB layed multisig diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 0d46937bb51..d425361163f 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -83,7 +83,7 @@ impl InnerAlloc { if #[cfg(all(not(feature = "std"), target_arch = "wasm32"))] { /// Request a `pages` number of pages of Wasm memory. Each page is `64KiB` in size. /// - /// Returns `None` if a page isn't available. + /// Returns `None` if a page is not available. fn request_pages(&mut self, pages: usize) -> Option { let prev_page = core::arch::wasm32::memory_grow(0, pages); if prev_page == usize::MAX { @@ -96,7 +96,7 @@ impl InnerAlloc { } else if #[cfg(feature = "std")] { /// Request a `pages` number of page sized sections of Wasm memory. Each page is `64KiB` in size. /// - /// Returns `None` if a page isn't available. + /// Returns `None` if a page is not available. /// /// This implementation is only meant to be used for testing, since we cannot (easily) /// test the `wasm32` implementation. @@ -112,7 +112,7 @@ impl InnerAlloc { } } - /// Tries to allocate enough memory on the heap for the given `Layout`. If there isn't enough + /// Tries to allocate enough memory on the heap for the given `Layout`. If there is not enough /// room on the heap it'll try and grow it by a page. /// /// Note: This implementation results in internal fragmentation when allocating across pages. From 560fe2f57fc4848c89817c7fe04392b5e2815f98 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 12 Jul 2021 17:15:44 -0400 Subject: [PATCH 34/39] Get rid of `page_requests` field --- crates/allocator/src/bump.rs | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index d425361163f..f412af4e139 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -61,12 +61,6 @@ struct InnerAlloc { /// The address of the upper limit of our heap. upper_limit: usize, - - /// The number of page requests made so far. - /// - /// This is meant to mimic the behavior exhibited by `core::arch::wasm32::memory_grow` - #[cfg(feature = "std")] - page_requests: usize, } impl InnerAlloc { @@ -74,8 +68,6 @@ impl InnerAlloc { Self { next: 0, upper_limit: 0, - #[cfg(feature = "std")] - page_requests: 0, } } @@ -100,10 +92,8 @@ impl InnerAlloc { /// /// This implementation is only meant to be used for testing, since we cannot (easily) /// test the `wasm32` implementation. - fn request_pages(&mut self, pages: usize) -> Option { - let prev_page = self.page_requests.checked_mul(PAGE_SIZE); - self.page_requests += pages; - prev_page + fn request_pages(&mut self, _pages: usize) -> Option { + Some(self.upper_limit) } } else { compile_error! { @@ -150,7 +140,7 @@ mod tests { let layout = Layout::new::(); assert!(inner.alloc(layout).is_some()); - let expected_limit = inner.page_requests * PAGE_SIZE; + let expected_limit = PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); let expected_alloc_start = std::mem::size_of::(); @@ -174,7 +164,7 @@ mod tests { assert!(inner.alloc(layout).is_some()); } - let expected_limit = inner.page_requests * PAGE_SIZE; + let expected_limit = PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); let expected_alloc_start = allocations * std::mem::size_of::(); @@ -193,7 +183,7 @@ mod tests { let layout = Layout::new::(); assert_eq!(inner.alloc(layout), Some(0)); - let expected_limit = inner.page_requests * PAGE_SIZE; + let expected_limit = PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); let expected_alloc_start = std::mem::size_of::(); @@ -203,7 +193,7 @@ mod tests { let layout = Layout::new::(); assert_eq!(inner.alloc(layout), Some(PAGE_SIZE)); - let expected_limit = inner.page_requests * PAGE_SIZE; + let expected_limit = 2 * PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); // Notice that we start the allocation on the second page, instead of making use of the @@ -223,7 +213,7 @@ mod tests { let layout = Layout::new::(); assert_eq!(inner.alloc(layout), Some(0)); - let expected_limit = inner.page_requests * PAGE_SIZE; + let expected_limit = 2 * PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); let expected_alloc_start = std::mem::size_of::(); @@ -234,7 +224,7 @@ mod tests { let layout = Layout::new::(); assert_eq!(inner.alloc(layout), Some(2 * PAGE_SIZE)); - let expected_limit = inner.page_requests * PAGE_SIZE; + let expected_limit = 3 * PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); let expected_alloc_start = 2 * PAGE_SIZE + std::mem::size_of::(); From c9718682cfe1726b3195a820b365eafd91b2db15 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 13 Jul 2021 11:17:13 -0400 Subject: [PATCH 35/39] Explicitly allow test builds to use test implementation --- crates/allocator/src/bump.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index f412af4e139..ce429428cc1 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -84,8 +84,7 @@ impl InnerAlloc { prev_page.checked_mul(PAGE_SIZE) } - - } else if #[cfg(feature = "std")] { + } else if #[cfg(any(test, feature = "std"))] { /// Request a `pages` number of page sized sections of Wasm memory. Each page is `64KiB` in size. /// /// Returns `None` if a page is not available. From 13b8bd35d8251e8e2b03b293d7c27319fe3814c0 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 13 Jul 2021 11:20:36 -0400 Subject: [PATCH 36/39] All link to zero'd Wasm memory reference --- crates/allocator/src/bump.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index ce429428cc1..059520ab89c 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -47,6 +47,8 @@ unsafe impl GlobalAlloc for BumpAllocator { unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { // A new page in Wasm is guaranteed to already be zero initialized, so we can just use our // regular `alloc` call here and save a bit of work. + // + // See: https://webassembly.github.io/spec/core/exec/modules.html#growing-memories self.alloc(layout) } From 6b452a6edf0f9f7202be120d0031ee29f8d97c64 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 13 Jul 2021 11:26:09 -0400 Subject: [PATCH 37/39] Check that our initial pointer is 0 in a test --- crates/allocator/src/bump.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 059520ab89c..4be6a980cf0 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -139,7 +139,7 @@ mod tests { let mut inner = InnerAlloc::new(); let layout = Layout::new::(); - assert!(inner.alloc(layout).is_some()); + assert_eq!(inner.alloc(layout), Some(0)); let expected_limit = PAGE_SIZE; assert_eq!(inner.upper_limit, expected_limit); From 7fce248353b023d491349627661097007af0ef28 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 16 Jul 2021 10:18:47 -0400 Subject: [PATCH 38/39] Add `cfg_if` branch for non-test, `std` enabled builds --- crates/allocator/src/bump.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 4be6a980cf0..14190f397ac 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -86,7 +86,7 @@ impl InnerAlloc { prev_page.checked_mul(PAGE_SIZE) } - } else if #[cfg(any(test, feature = "std"))] { + } else if #[cfg(test)] { /// Request a `pages` number of page sized sections of Wasm memory. Each page is `64KiB` in size. /// /// Returns `None` if a page is not available. @@ -96,6 +96,13 @@ impl InnerAlloc { fn request_pages(&mut self, _pages: usize) -> Option { Some(self.upper_limit) } + } else if #[cfg(all(not(test), feature = "std"))] { + fn request_pages(&mut self, _pages: usize) -> Option { + unreachable!( + "This branch is only used to keep the compiler happy when building tests, and + should never actually be called outside of a test run." + ) + } } else { compile_error! { "ink! only supports compilation as `std` or `no_std` + `wasm32-unknown`" From 8e84f341960c43676ad0fc22df554b5f49b80d0e Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 19 Jul 2021 11:16:02 -0400 Subject: [PATCH 39/39] Simplify `cfg_if` statement --- crates/allocator/src/bump.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/allocator/src/bump.rs b/crates/allocator/src/bump.rs index 14190f397ac..63b4d241019 100644 --- a/crates/allocator/src/bump.rs +++ b/crates/allocator/src/bump.rs @@ -74,19 +74,7 @@ impl InnerAlloc { } cfg_if::cfg_if! { - if #[cfg(all(not(feature = "std"), target_arch = "wasm32"))] { - /// Request a `pages` number of pages of Wasm memory. Each page is `64KiB` in size. - /// - /// Returns `None` if a page is not available. - fn request_pages(&mut self, pages: usize) -> Option { - let prev_page = core::arch::wasm32::memory_grow(0, pages); - if prev_page == usize::MAX { - return None; - } - - prev_page.checked_mul(PAGE_SIZE) - } - } else if #[cfg(test)] { + if #[cfg(test)] { /// Request a `pages` number of page sized sections of Wasm memory. Each page is `64KiB` in size. /// /// Returns `None` if a page is not available. @@ -96,13 +84,25 @@ impl InnerAlloc { fn request_pages(&mut self, _pages: usize) -> Option { Some(self.upper_limit) } - } else if #[cfg(all(not(test), feature = "std"))] { + } else if #[cfg(feature = "std")] { fn request_pages(&mut self, _pages: usize) -> Option { unreachable!( "This branch is only used to keep the compiler happy when building tests, and should never actually be called outside of a test run." ) } + } else if #[cfg(target_arch = "wasm32")] { + /// Request a `pages` number of pages of Wasm memory. Each page is `64KiB` in size. + /// + /// Returns `None` if a page is not available. + fn request_pages(&mut self, pages: usize) -> Option { + let prev_page = core::arch::wasm32::memory_grow(0, pages); + if prev_page == usize::MAX { + return None; + } + + prev_page.checked_mul(PAGE_SIZE) + } } else { compile_error! { "ink! only supports compilation as `std` or `no_std` + `wasm32-unknown`"