Skip to content

Commit

Permalink
perf: optimize overflow check in RTS alloc_words, fix and update te…
Browse files Browse the repository at this point in the history
…sts (#3085)

#3077 added an expensive, but necessary overflow check to `alloc_words`, to prevent allocating addresses outside the 32-bit heap.

* This PR optimizes that check to avoid the conditional test by doing the page diff arithmetic in u64s, 
relying on `grow_memory` to trap when asked to grow beyond 2^16 pages.
* fixes test `run-drun/oom-update` to actually use an update (not query) method as advertised.

I wonder why `grow_memory` is marked `inline_never`? Seems like we'd save a function call on each allocation...
  • Loading branch information
crusso authored Jan 31, 2022
1 parent 316b193 commit d1fef02
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 17 deletions.
22 changes: 11 additions & 11 deletions rts/motoko-rts/src/memory/ic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,29 +72,29 @@ impl Memory for IcMemory {
unsafe fn alloc_words(&mut self, n: Words<u32>) -> Value {
let bytes = n.to_bytes();
// Update ALLOCATED
ALLOCATED += Bytes(u64::from(bytes.as_u32()));
let delta = u64::from(bytes.as_u32());
ALLOCATED += Bytes(delta);

// Update heap pointer
let old_hp = HP;
let (new_hp, overflow) = old_hp.overflowing_add(bytes.as_u32());
// Check for overflow
if overflow {
rts_trap_with("Out of memory");
}
HP = new_hp;
let old_hp = u64::from(HP);
let new_hp = old_hp + delta;

// Grow memory if needed
grow_memory(new_hp as usize);
grow_memory(new_hp);

debug_assert!(new_hp <= u64::from(core::u32::MAX));
HP = new_hp as u32;

Value::from_ptr(old_hp as usize)
}
}

/// Page allocation. Ensures that the memory up to, but excluding, the given pointer is allocated.
#[inline(never)]
unsafe fn grow_memory(ptr: usize) {
unsafe fn grow_memory(ptr: u64) {
debug_assert!(ptr <= 2 * u64::from(core::u32::MAX));
let page_size = u64::from(WASM_PAGE_SIZE.as_u32());
let total_pages_needed = (((ptr as u64) + page_size - 1) / page_size) as usize;
let total_pages_needed = ((ptr + page_size - 1) / page_size) as usize;
let current_pages = wasm32::memory_size(0);
if total_pages_needed > current_pages {
if wasm32::memory_grow(0, total_pages_needed - current_pages) == core::usize::MAX {
Expand Down
2 changes: 1 addition & 1 deletion test/run-drun/ok/oom-query.drun-run.ok
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
ingress Completed: Reply: 0x4449444c0000
Err: IC0503: Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: RTS error: Out of memory
Err: IC0503: Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: RTS error: Cannot grow memory
2 changes: 1 addition & 1 deletion test/run-drun/ok/oom-update.drun-run.ok
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
ingress Completed: Reply: 0x4449444c0000
Err: IC0503: Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: RTS error: Out of memory
ingress Err: IC0503: Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: RTS error: Cannot grow memory
4 changes: 2 additions & 2 deletions test/run-drun/oom-update.mo
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import P "mo:⛔";
actor {

public query func go() : async () {
public func go() : async () {
// allocate 3GB
var c = 3;
while(c > 0) {
Expand All @@ -25,5 +25,5 @@ actor {
//SKIP run-ir
// too slow on ic-ref-run:
//SKIP comp-ref
//CALL query go "DIDL\x00\x00"
//CALL ingress go "DIDL\x00\x00"

2 changes: 1 addition & 1 deletion test/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function normalize () {
# Normalize instruction locations on traps, added by ic-ref ad6ea9e
sed 's/region:0x[0-9a-fA-F]\+-0x[0-9a-fA-F]\+/region:0xXXX-0xXXX/g' |
# Delete everything after Oom
sed '/RTS error: Out of memory/q' |
sed '/RTS error: Cannot grow memory/q' |
cat > $1.norm
mv $1.norm $1
fi
Expand Down
2 changes: 1 addition & 1 deletion test/run/ok/inc-oom.wasm-run.ok
Original file line number Diff line number Diff line change
@@ -1 +1 @@
RTS error: Out of memory
RTS error: Cannot grow memory

0 comments on commit d1fef02

Please sign in to comment.