RTS: Introduce WASM_PAGES_SIZE and WASM_HEAP_SIZE constants#2632
RTS: Introduce WASM_PAGES_SIZE and WASM_HEAP_SIZE constants#2632mergify[bot] merged 3 commits intomasterfrom
Conversation
…magic numbers with constants
|
This PR does not affect the produced WebAssembly code. |
ulan
left a comment
ulan
left a comment
There was a problem hiding this comment.
lgtm, thanks!
Feel free to ignore the suggestions below.
| // Array payload should not be larger than half of the memory | ||
| if len > 1 << (32 - 2 - 1) { | ||
| // 2 for word size, 1 to divide by two | ||
| if Words(len) > WASM_HEAP_SIZE / 2 { |
There was a problem hiding this comment.
Nit (feel free to ignore): how about WASM_HEAP_SIZE_IN_WORDS to make it clear for the reader?
There was a problem hiding this comment.
I used the type Words, which is a newtype and requires explicit unwrapping or conversion to Bytes. Do you still think _IN_WORDS suffix would be helpful? I can add it, just wanted to make sure you know that Words is a newtype and won't be implicitly converted to anything else and when used in arithmetic with Bytes or scalars will cause compile errors.
| pub const WASM_PAGE_SIZE: Bytes<u32> = Bytes(64 * 1024); | ||
|
|
||
| /// Wasm heap size (4GiB) in words. Note that `to_bytes` on this value will overflow as 4GiB in | ||
| /// bytes is `u32::MAX + 1`. |
There was a problem hiding this comment.
The overflow is a bit of a footgun. Alternatively we could introduce MAX_HEAP_SIZE and make it smaller than WASM_HEAP_SIZE by a couple of pages, so that we don't have to worry about accidentally overflowing u32.
There was a problem hiding this comment.
I'm not sure if we want to reduce heap size to avoid a bug in the RTS. In Rust overflow in arithmetic is checked in debug mode, and we run the tests in both debug mode and release mode, so hopefully any bugs caused by this will be caught before getting merged.
|
|
||
| #[no_mangle] | ||
| pub unsafe extern "C" fn alloc_array(len: u32) -> SkewedPtr { | ||
| // Array payload should not be larger than half of the memory |
There was a problem hiding this comment.
I wish we documented why this is the case. I believe this is because the copying GC will want to copy it and overflow the heap if this is 2GiB, but I don't know if there are other reasons.
4 participants
No description provided.