perf: optimize overflow check in RTS alloc_words, fix and update tests
#3085
Conversation
crusso
changed the title
crusso
changed the title
Seems worth it, maybe. And reclaims the cycle regression here: #3077 (comment) |
ggreif
changed the title
alloc_words, fix and update tests
|
See also #2998, where I was wondering about the no-inline too... |
|
|
||
| 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)); |
There was a problem hiding this comment.
We can use the same condition as in the assert in line 85: ptr <= u64::from(core::u32::MAX) (i.e. compare against 4GiB, not 8GiB)
There was a problem hiding this comment.
No, I'm deliberately comparing against 8GB because I want to test the overflow detection later on (otherwise we will get different faults on debug and release builds).
|
|
||
| // Grow memory if needed | ||
| grow_memory(new_hp as usize); | ||
| grow_memory(new_hp); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Right, but I'll leave that to #3090.
#3077 added an expensive, but necessary overflow check to
alloc_words, to prevent allocating addresses outside the 32-bit heap.relying on
grow_memoryto trap when asked to grow beyond 2^16 pages.run-drun/oom-updateto actually use an update (not query) method as advertised.I wonder why
grow_memoryis markedinline_never? Seems like we'd save a function call on each allocation...