From 8149ec3723a7ce0b090ba007886b017396894686 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 13 Oct 2025 17:54:49 +0800 Subject: [PATCH 01/13] Use boolean allocation options We remove OnAllocationFail and add three boolean fields to AllocationOptions. --- src/policy/lockfreeimmortalspace.rs | 2 +- src/policy/space.rs | 55 +++++++---- src/util/alloc/allocator.rs | 94 ++++++++++++------- src/util/alloc/mod.rs | 1 - ...mock_test_allocate_no_gc_oom_on_acquire.rs | 5 +- .../mock_test_allocate_no_gc_simple.rs | 5 +- 6 files changed, 103 insertions(+), 59 deletions(-) diff --git a/src/policy/lockfreeimmortalspace.rs b/src/policy/lockfreeimmortalspace.rs index f62520a1c1..1a34d179e1 100644 --- a/src/policy/lockfreeimmortalspace.rs +++ b/src/policy/lockfreeimmortalspace.rs @@ -151,7 +151,7 @@ impl Space for LockFreeImmortalSpace { }) .expect("update cursor failed"); if start + bytes > self.limit { - if alloc_options.on_fail.allow_oom_call() { + if alloc_options.allow_oom_call() { panic!("OutOfMemory"); } else { return Address::ZERO; diff --git a/src/policy/space.rs b/src/policy/space.rs index 784a37a33d..73549024d5 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -85,7 +85,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { alloc_options: AllocationOptions, ) -> bool { if self.will_oom_on_acquire(size) { - if alloc_options.on_fail.allow_oom_call() { + if alloc_options.allow_oom_call() { VM::VMCollection::out_of_memory( tls, crate::util::alloc::AllocationError::HeapOutOfMemory, @@ -108,35 +108,44 @@ pub trait Space: 'static + SFT + Sync + Downcast { "The requested pages is larger than the max heap size. Is will_go_oom_on_acquire used before acquring memory?" ); - // Should we poll to attempt to GC? + trace!("Reserving pages"); + let pr = self.get_page_resource(); + let pages_reserved = pr.reserve_pages(pages); + trace!("Pages reserved"); + + // Should we poll before acquring pages from page resources so that it can trigger a GC? // - If tls is collector, we cannot attempt a GC. // - If gc is disabled, we cannot attempt a GC. - // - If overcommit is allowed, we don't attempt a GC. - // FIXME: We should allow polling while also allowing over-committing. - // We should change the allocation interface. + // - If the allocation option explicitly disables eager polling, we don't poll now. let should_poll = VM::VMActivePlan::is_mutator(tls) && VM::VMCollection::is_collection_enabled() - && !alloc_options.on_fail.allow_overcommit(); + && alloc_options.eager_polling; - trace!("Reserving pages"); - let pr = self.get_page_resource(); - let pages_reserved = pr.reserve_pages(pages); - trace!("Pages reserved"); - trace!("Polling .."); + // If we should poll eagerly, do it now. Record if it has triggered a GC. + // If we should not poll eagerly, GC is not triggered. + let gc_triggered = should_poll && { + trace!("Polling .."); + self.get_gc_trigger().poll(false, Some(self.as_space())) + }; - // The actual decision tree. - if should_poll && self.get_gc_trigger().poll(false, Some(self.as_space())) { - self.not_acquiring(tls, alloc_options, pr, pages_reserved, false); - Address::ZERO - } else { - debug!("Collection not required"); + // We can try to get pages if + // - GC is not triggered, or + // - GC is triggered, but we allow over-committing. + let should_get_pages = !gc_triggered || alloc_options.allow_overcommit; + // Get new pages if we should. If we didn't get new pages from the page resource for any + // reason (if we decided not to, or if we tried and failed), this function shall return a + // null address. + if should_get_pages { if let Some(addr) = self.get_new_pages_and_initialize(tls, pages, pr, pages_reserved) { addr } else { - self.not_acquiring(tls, alloc_options, pr, pages_reserved, true); + self.not_acquiring(tls, alloc_options, pr, pages_reserved, false); Address::ZERO } + } else { + self.not_acquiring(tls, alloc_options, pr, pages_reserved, true); + Address::ZERO } } @@ -268,11 +277,17 @@ pub trait Space: 'static + SFT + Sync + Downcast { pages_reserved: usize, attempted_allocation_and_failed: bool, ) { + assert!( + VM::VMActivePlan::is_mutator(tls), + "A non-mutator thread failed to get pages from page resource. \ + Copying GC plans should compute the copying headroom carefully to prevent this." + ); + // Clear the request pr.clear_request(pages_reserved); - // If we do not want GC on fail, just return. - if !alloc_options.on_fail.allow_gc() { + // If we are not at a safepoint, return immediately. + if !alloc_options.at_safepoint { return; } diff --git a/src/util/alloc/allocator.rs b/src/util/alloc/allocator.rs index bae77beb22..4460946dbc 100644 --- a/src/util/alloc/allocator.rs +++ b/src/util/alloc/allocator.rs @@ -28,45 +28,75 @@ pub enum AllocationError { MmapOutOfMemory, } -/// Behavior when an allocation fails, and a GC is expected. -#[repr(u8)] -#[derive(Copy, Clone, Default, PartialEq, bytemuck::NoUninit, Debug)] -pub enum OnAllocationFail { - /// Request the GC. This is the default behavior. - #[default] - RequestGC, - /// Instead of requesting GC, the allocation request returns with a failure value. - ReturnFailure, - /// Instead of requesting GC, the allocation request simply overcommits the memory, - /// and return a valid result at its best efforts. - OverCommit, -} - -impl OnAllocationFail { - pub(crate) fn allow_oom_call(&self) -> bool { - *self == Self::RequestGC - } - pub(crate) fn allow_gc(&self) -> bool { - *self == Self::RequestGC - } - pub(crate) fn allow_overcommit(&self) -> bool { - *self == Self::OverCommit - } -} - /// Allow specifying different behaviors with [`Allocator::alloc_with_options`]. #[repr(C)] -#[derive(Copy, Clone, Default, PartialEq, bytemuck::NoUninit, Debug)] +#[derive(Copy, Clone, PartialEq, bytemuck::NoUninit, Debug)] pub struct AllocationOptions { - /// When the allocation fails and a GC is originally expected, on_fail - /// allows a different behavior to avoid the GC. - pub on_fail: OnAllocationFail, + /// Should we poll *before* trying to acquire more pages from the page resource? + /// + /// **The default is `true`**. + /// + /// If `true`, the allocation will let the GC trigger poll before acquiring pages from the page + /// resource, giving the GC trigger a chance to schedule a collection. + /// + /// If `false`, the allocation will not notify the GC trigger *before* acquiring pages from the + /// page resource. Note that if the allocation is at a safepoint (i.e. [`Self::at_safepoint`] + /// is true), it will still poll and force a GC *after* failing to get pages from the page + /// resource due to physical memory exhaustion. + pub eager_polling: bool, + + /// Whether over-committing is allowed at this allocation site. + /// + /// **The default is `false`**. + /// + /// This option is only meaningful if [`Self::eager_polling`] is true. It has no effect if + /// `eager_polling == false`. + /// + /// If `true`, the allocation will still try to acquire pages from page resources even + /// if the eager polling triggers a GC. + /// + /// If `false` the allocation will not try to get pages from page resource as long as GC + /// is triggered. + pub allow_overcommit: bool, + + /// Whether the allocation is at a safepoint. + /// + /// **The default is `true`**. + /// + /// If `true`, the allocation attempt will block for GC if GC is triggered. It will also force + /// triggering GC and block after failing to get pages from the page resource due to physical + /// memory exhaustion. It will also call [`Collection::out_of_memory`] when out of memory. + /// + /// If `false`, the allocation attempt will immediately return a null address if the allocation + /// cannot be satisfied without a GC. It will never block for GC, never force a GC, and never + /// call [`Collection::out_of_memory`]. Note that the VM can always force a GC by calling + /// [`crate::MMTK::handle_user_collection_request`] with the argument `force` being `true`. + pub at_safepoint: bool, +} + +/// The default value for `AllocationOptions` has the same semantics as calling [`Allocator::alloc`] +/// directly. +impl Default for AllocationOptions { + fn default() -> Self { + Self { + eager_polling: true, + allow_overcommit: false, + at_safepoint: true, + } + } } impl AllocationOptions { pub(crate) fn is_default(&self) -> bool { *self == AllocationOptions::default() } + + /// Whether this allocation allows calling [`Collection::out_of_memory`]. + /// + /// It is allowed if and only if the allocation is at safepoint. + pub(crate) fn allow_oom_call(&self) -> bool { + self.at_safepoint + } } pub fn align_allocation_no_fill( @@ -367,9 +397,7 @@ pub trait Allocator: Downcast { return result; } - if result.is_zero() - && self.get_context().get_alloc_options().on_fail == OnAllocationFail::ReturnFailure - { + if result.is_zero() && !self.get_context().get_alloc_options().allow_oom_call() { return result; } diff --git a/src/util/alloc/mod.rs b/src/util/alloc/mod.rs index d3148dceed..2f905a913d 100644 --- a/src/util/alloc/mod.rs +++ b/src/util/alloc/mod.rs @@ -6,7 +6,6 @@ pub use allocator::fill_alignment_gap; pub use allocator::AllocationError; pub use allocator::AllocationOptions; pub use allocator::Allocator; -pub use allocator::OnAllocationFail; /// A list of all the allocators, embedded in Mutator pub(crate) mod allocators; diff --git a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs index 6aa6a3f207..a98cba47ec 100644 --- a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs +++ b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs @@ -1,6 +1,6 @@ use super::mock_test_prelude::*; -use crate::util::alloc::allocator::{AllocationOptions, OnAllocationFail}; +use crate::util::alloc::allocator::{AllocationOptions}; use crate::AllocationSemantics; /// This test will allocate an object that is larger than the heap size. The call will fail. @@ -21,7 +21,8 @@ pub fn allocate_no_gc_oom_on_acquire() { 0, AllocationSemantics::Default, AllocationOptions { - on_fail: OnAllocationFail::ReturnFailure, + at_safepoint: false, + ..Default::default() }, ); // We should get zero. diff --git a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_simple.rs b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_simple.rs index f9492c7e01..14a1dfc6d1 100644 --- a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_simple.rs +++ b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_simple.rs @@ -1,6 +1,6 @@ use super::mock_test_prelude::*; -use crate::util::alloc::allocator::{AllocationOptions, OnAllocationFail}; +use crate::util::alloc::allocator::AllocationOptions; use crate::AllocationSemantics; /// This test will do alloc_with_options in a loop, and evetually fill up the heap. @@ -26,7 +26,8 @@ pub fn allocate_no_gc_simple() { 0, AllocationSemantics::Default, AllocationOptions { - on_fail: OnAllocationFail::ReturnFailure, + at_safepoint: false, + ..Default::default() }, ); if last_result.is_zero() { From fa47af722198edfb16bf3c545b2715ca3a0e1f6e Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 13 Oct 2025 18:02:19 +0800 Subject: [PATCH 02/13] Enable overcommit test --- src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs | 7 +++++-- src/vm/tests/mock_tests/mod.rs | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs b/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs index 6f41413116..6d31d4a4db 100644 --- a/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs +++ b/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs @@ -1,6 +1,8 @@ +// GITHUB-CI: MMTK_PLAN=NoGC + use super::mock_test_prelude::*; -use crate::util::alloc::allocator::{AllocationOptions, OnAllocationFail}; +use crate::util::alloc::allocator::AllocationOptions; use crate::AllocationSemantics; /// This test will do alloc_with_options in a loop, and evetually fill up the heap. @@ -26,7 +28,8 @@ pub fn allocate_overcommit() { 0, AllocationSemantics::Default, AllocationOptions { - on_fail: OnAllocationFail::ReturnFailure, + allow_overcommit: true, + ..Default::default() }, ); assert!(!last_result.is_zero()); diff --git a/src/vm/tests/mock_tests/mod.rs b/src/vm/tests/mock_tests/mod.rs index af00a83422..0fa03b517f 100644 --- a/src/vm/tests/mock_tests/mod.rs +++ b/src/vm/tests/mock_tests/mod.rs @@ -27,6 +27,7 @@ mod mock_test_allocate_align_offset; mod mock_test_allocate_no_gc_oom_on_acquire; mod mock_test_allocate_no_gc_simple; mod mock_test_allocate_nonmoving; +mod mock_test_allocate_overcommit; mod mock_test_allocate_with_disable_collection; mod mock_test_allocate_with_initialize_collection; mod mock_test_allocate_with_re_enable_collection; From 6f863665ef3f3cea2172bf4abf0c47c37f395318 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 13 Oct 2025 18:08:15 +0800 Subject: [PATCH 03/13] Formatting --- .../tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs index a98cba47ec..b8898ef0ea 100644 --- a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs +++ b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs @@ -1,6 +1,6 @@ use super::mock_test_prelude::*; -use crate::util::alloc::allocator::{AllocationOptions}; +use crate::util::alloc::allocator::AllocationOptions; use crate::AllocationSemantics; /// This test will allocate an object that is larger than the heap size. The call will fail. From 3b13237c3c499915f1134b5327e4119267400fe8 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 14 Oct 2025 14:41:46 +0800 Subject: [PATCH 04/13] Holder --- src/util/alloc/allocator.rs | 65 ++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/src/util/alloc/allocator.rs b/src/util/alloc/allocator.rs index 4460946dbc..20fdf5afde 100644 --- a/src/util/alloc/allocator.rs +++ b/src/util/alloc/allocator.rs @@ -6,7 +6,7 @@ use crate::util::heap::gc_trigger::GCTrigger; use crate::util::options::Options; use crate::MMTK; -use atomic::Atomic; +use std::cell::RefCell; use std::sync::atomic::Ordering; use std::sync::Arc; @@ -30,7 +30,7 @@ pub enum AllocationError { /// Allow specifying different behaviors with [`Allocator::alloc_with_options`]. #[repr(C)] -#[derive(Copy, Clone, PartialEq, bytemuck::NoUninit, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct AllocationOptions { /// Should we poll *before* trying to acquire more pages from the page resource? /// @@ -99,6 +99,56 @@ impl AllocationOptions { } } +/// A wrapper for [`AllocatorContext`] to hold a [`AllocationOptions`] that can be modified by the +/// same mutator thread. +/// +/// All [`Allocator`] instances in `Allocators` share one `AllocationOptions` instance, and it will +/// only be accessed by the mutator (via `Mutator::allocators`) or the GC worker (via +/// `GCWorker::copy`) that owns it. Rust doesn't like multiple mutable references pointing to a +/// shared data structure. We cannot use [`atomic::Atomic`] because `AllocationOptions` has +/// multiple fields. We wrap it in a `RefCell` to make it internally mutable. +/// +/// Note: The allocation option is called every time [`Allocator::alloc_with_options`] is called. +/// Because API functions should only be called on allocation slow paths, we believe that `RefCell` +/// should be good enough for performance. If this is too slow, we may consider `UnsafeCell`. If +/// that's still too slow, we should consider changing the API to make the allocation options a +/// persistent per-mutator value, and allow the VM binding set its value via a new API function. +struct AllocationOptionsHolder { + alloc_options: RefCell, +} + +/// Strictly speaking, `AllocationOptionsHolder` isn't `Sync`. Two threads cannot set or clear the +/// same `AllocationOptionsHolder` at the same time. However, both `Mutator` and `GCWorker` are +/// `Send`, and both of which own `Allocators` and require its field `Arc` to be +/// `Send`, which requires `AllocationContext` to be `Sync`, which requires +/// `AllocationOptionsHolder` to be `Sync`. (Note that `Arc` can be cloned and given to another +/// thread, and Rust expects `T` to be `Sync`, too. But we never share `AllocationContext` between +/// threads, but only between multiple `Allocator` instances within the same `Allocators` instance. +/// Rust can't figure this out.) +unsafe impl Sync for AllocationOptionsHolder {} + +impl AllocationOptionsHolder { + pub fn new(alloc_options: AllocationOptions) -> Self { + Self { + alloc_options: RefCell::new(alloc_options), + } + } + pub fn set_alloc_options(&self, options: AllocationOptions) { + let mut alloc_options = self.alloc_options.borrow_mut(); + *alloc_options = options; + } + + pub fn clear_alloc_options(&self) { + let mut alloc_options = self.alloc_options.borrow_mut(); + *alloc_options = AllocationOptions::default(); + } + + pub fn get_alloc_options(&self) -> AllocationOptions { + let alloc_options = self.alloc_options.borrow(); + *alloc_options + } +} + pub fn align_allocation_no_fill( region: Address, alignment: usize, @@ -210,7 +260,7 @@ pub(crate) fn assert_allocation_args(size: usize, align: usize, o /// The context an allocator needs to access in order to perform allocation. pub struct AllocatorContext { - pub alloc_options: Atomic, + alloc_options: AllocationOptionsHolder, pub state: Arc, pub options: Arc, pub gc_trigger: Arc>, @@ -221,7 +271,7 @@ pub struct AllocatorContext { impl AllocatorContext { pub fn new(mmtk: &MMTK) -> Self { Self { - alloc_options: Atomic::new(AllocationOptions::default()), + alloc_options: AllocationOptionsHolder::new(AllocationOptions::default()), state: mmtk.state.clone(), options: mmtk.options.clone(), gc_trigger: mmtk.gc_trigger.clone(), @@ -231,16 +281,15 @@ impl AllocatorContext { } pub fn set_alloc_options(&self, options: AllocationOptions) { - self.alloc_options.store(options, Ordering::Relaxed); + self.alloc_options.set_alloc_options(options); } pub fn clear_alloc_options(&self) { - self.alloc_options - .store(AllocationOptions::default(), Ordering::Relaxed); + self.alloc_options.clear_alloc_options(); } pub fn get_alloc_options(&self) -> AllocationOptions { - self.alloc_options.load(Ordering::Relaxed) + self.alloc_options.get_alloc_options() } } From 655288d6412f649136e81a19dfc4b6fe58223878 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 15 Oct 2025 12:51:05 +0800 Subject: [PATCH 05/13] Run overcommit test with all plans. --- src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs b/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs index 6d31d4a4db..829381d96a 100644 --- a/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs +++ b/src/vm/tests/mock_tests/mock_test_allocate_overcommit.rs @@ -1,4 +1,4 @@ -// GITHUB-CI: MMTK_PLAN=NoGC +// GITHUB-CI: MMTK_PLAN=all use super::mock_test_prelude::*; From 9f65d4c5bc3934cf7d680a93cc63a93574a68df4 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 15 Oct 2025 13:28:31 +0800 Subject: [PATCH 06/13] Remove eager_polling option Now polling cannot be disabled. It will always poll. --- src/policy/space.rs | 10 ++++----- src/util/alloc/allocator.rs | 44 +++++++++++++++---------------------- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/policy/space.rs b/src/policy/space.rs index 73549024d5..ea495bc26f 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -116,13 +116,11 @@ pub trait Space: 'static + SFT + Sync + Downcast { // Should we poll before acquring pages from page resources so that it can trigger a GC? // - If tls is collector, we cannot attempt a GC. // - If gc is disabled, we cannot attempt a GC. - // - If the allocation option explicitly disables eager polling, we don't poll now. - let should_poll = VM::VMActivePlan::is_mutator(tls) - && VM::VMCollection::is_collection_enabled() - && alloc_options.eager_polling; + let should_poll = + VM::VMActivePlan::is_mutator(tls) && VM::VMCollection::is_collection_enabled(); - // If we should poll eagerly, do it now. Record if it has triggered a GC. - // If we should not poll eagerly, GC is not triggered. + // If we should poll, do it now. Record if it has triggered a GC. + // If we should not poll, GC is not triggered. let gc_triggered = should_poll && { trace!("Polling .."); self.get_gc_trigger().poll(false, Some(self.as_space())) diff --git a/src/util/alloc/allocator.rs b/src/util/alloc/allocator.rs index 20fdf5afde..174f248cf7 100644 --- a/src/util/alloc/allocator.rs +++ b/src/util/alloc/allocator.rs @@ -32,44 +32,37 @@ pub enum AllocationError { #[repr(C)] #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct AllocationOptions { - /// Should we poll *before* trying to acquire more pages from the page resource? - /// - /// **The default is `true`**. - /// - /// If `true`, the allocation will let the GC trigger poll before acquiring pages from the page - /// resource, giving the GC trigger a chance to schedule a collection. - /// - /// If `false`, the allocation will not notify the GC trigger *before* acquiring pages from the - /// page resource. Note that if the allocation is at a safepoint (i.e. [`Self::at_safepoint`] - /// is true), it will still poll and force a GC *after* failing to get pages from the page - /// resource due to physical memory exhaustion. - pub eager_polling: bool, - /// Whether over-committing is allowed at this allocation site. /// /// **The default is `false`**. /// - /// This option is only meaningful if [`Self::eager_polling`] is true. It has no effect if - /// `eager_polling == false`. + /// If `true`, the allocation will still try to acquire pages from page resources even when a GC + /// is triggered by the polling. /// - /// If `true`, the allocation will still try to acquire pages from page resources even - /// if the eager polling triggers a GC. + /// If `false` the allocation will not try to get pages from page resource as long as GC is + /// triggered. /// - /// If `false` the allocation will not try to get pages from page resource as long as GC - /// is triggered. + /// Note that MMTk lets the GC trigger poll before trying to acquire pages from the page + /// resource. This gives the GC trigger a chance to trigger GC if needed. `allow_overcommit` + /// does not disable polling, but only controls whether to try acquiring pages when GC is + /// triggered. pub allow_overcommit: bool, /// Whether the allocation is at a safepoint. /// /// **The default is `true`**. /// - /// If `true`, the allocation attempt will block for GC if GC is triggered. It will also force - /// triggering GC and block after failing to get pages from the page resource due to physical - /// memory exhaustion. It will also call [`Collection::out_of_memory`] when out of memory. + /// If `true`, the allocation is allowed to block for GC, and call [`Collection::out_of_memory`] + /// when out of memory. Specifically, it may block for GC if any of the following happens: + /// + /// - The GC trigger polled and triggered a GC before the allocation tries to get more pages + /// from the page resource, and the allocation does not allow over-committing. + /// - The allocation tried to get more pages from the page resource, but failed. In this + /// case, it will force a GC. /// - /// If `false`, the allocation attempt will immediately return a null address if the allocation - /// cannot be satisfied without a GC. It will never block for GC, never force a GC, and never - /// call [`Collection::out_of_memory`]. Note that the VM can always force a GC by calling + /// If `false`, the allocation will immediately return a null address if the allocation cannot + /// be satisfied without a GC. It will never block for GC, never force a GC, and never call + /// [`Collection::out_of_memory`]. Note that the VM can always force a GC by calling /// [`crate::MMTK::handle_user_collection_request`] with the argument `force` being `true`. pub at_safepoint: bool, } @@ -79,7 +72,6 @@ pub struct AllocationOptions { impl Default for AllocationOptions { fn default() -> Self { Self { - eager_polling: true, allow_overcommit: false, at_safepoint: true, } From c0ba0c7c711e6659f12d6589b817120bc41c2d1b Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 15 Oct 2025 13:46:06 +0800 Subject: [PATCH 07/13] Migration guide --- docs/userguide/src/migration/prefix.md | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/userguide/src/migration/prefix.md b/docs/userguide/src/migration/prefix.md index cca4364a04..fab23ceb58 100644 --- a/docs/userguide/src/migration/prefix.md +++ b/docs/userguide/src/migration/prefix.md @@ -32,6 +32,38 @@ Notes for the mmtk-core developers: ## 0.32.0 +### Allocation options changed + +```admonish tldr +`AllocationOptions` now has multiple boolean fields instead of one `OnAllocationFail` field. Now +polling cannot be disabled. Instead we can now poll and over-commit in one allocation. +``` + +API changes: + +- module `util::alloc::allocator` + + `OnAllocationFail`: Removed. + + `AllocationOptions`: It now has two boolean fields: + * `allow_overcommit` + * `at_safepoint` + +Variants of the old `enum OnAllocationFail` should be migrated to the new API according to the +following table: + +| variant | `allow_overcommit` | `at_safepoint` | +|-----------------|--------------------|----------------| +| `RequestGC` | `false` | `true` | +| `ReturnFailure` | `false` | `false` | +| `OverCommit` | `true` | `false` | + +Note that MMTk now always polls before trying to get more pages from the page resource, and it may +trigger GC. The old `OnAllocationFail::OverCommit` used to prevent polling, but it is no longer +possible. + +See also: + +- PR: + ### Removed the notion of "mmap chunk" ```admonish tldr From f50dd219103bdb5d3813edcfdf7815943366312d Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 Oct 2025 10:14:49 +0800 Subject: [PATCH 08/13] Simplify the description of `allow_overcommit`. --- src/util/alloc/allocator.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/util/alloc/allocator.rs b/src/util/alloc/allocator.rs index 174f248cf7..edf89642f0 100644 --- a/src/util/alloc/allocator.rs +++ b/src/util/alloc/allocator.rs @@ -32,20 +32,14 @@ pub enum AllocationError { #[repr(C)] #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct AllocationOptions { - /// Whether over-committing is allowed at this allocation site. + /// Whether over-committing is allowed at this allocation site. Over-committing means the + /// allocation is allowed to go beyond the current heap size. But it is not guaranteed to + /// succeed. /// /// **The default is `false`**. /// - /// If `true`, the allocation will still try to acquire pages from page resources even when a GC - /// is triggered by the polling. - /// - /// If `false` the allocation will not try to get pages from page resource as long as GC is - /// triggered. - /// - /// Note that MMTk lets the GC trigger poll before trying to acquire pages from the page - /// resource. This gives the GC trigger a chance to trigger GC if needed. `allow_overcommit` - /// does not disable polling, but only controls whether to try acquiring pages when GC is - /// triggered. + /// Note that regardless of the value of `allow_overcommit`, the allocation may trigger GC if + /// the GC trigger considers it needed. pub allow_overcommit: bool, /// Whether the allocation is at a safepoint. From 34d29c4e630877c34dd48d506bba9e9180b26188 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 Oct 2025 13:27:42 +0800 Subject: [PATCH 09/13] Simplify the description of `at_safepoint`. --- src/util/alloc/allocator.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/util/alloc/allocator.rs b/src/util/alloc/allocator.rs index edf89642f0..15231b777d 100644 --- a/src/util/alloc/allocator.rs +++ b/src/util/alloc/allocator.rs @@ -46,18 +46,10 @@ pub struct AllocationOptions { /// /// **The default is `true`**. /// - /// If `true`, the allocation is allowed to block for GC, and call [`Collection::out_of_memory`] - /// when out of memory. Specifically, it may block for GC if any of the following happens: - /// - /// - The GC trigger polled and triggered a GC before the allocation tries to get more pages - /// from the page resource, and the allocation does not allow over-committing. - /// - The allocation tried to get more pages from the page resource, but failed. In this - /// case, it will force a GC. + /// If `true`, the allocation is allowed to block for GC. /// /// If `false`, the allocation will immediately return a null address if the allocation cannot - /// be satisfied without a GC. It will never block for GC, never force a GC, and never call - /// [`Collection::out_of_memory`]. Note that the VM can always force a GC by calling - /// [`crate::MMTK::handle_user_collection_request`] with the argument `force` being `true`. + /// be satisfied without a GC. pub at_safepoint: bool, } From 226b12c245e07cc6bedab381cf1b3ada04e56daa Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 Oct 2025 13:57:48 +0800 Subject: [PATCH 10/13] Change the condition to return early from `alloc_slow` It should be "at_safepoint" instead of "allow_oom_call". --- src/util/alloc/allocator.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/util/alloc/allocator.rs b/src/util/alloc/allocator.rs index 15231b777d..80e689e833 100644 --- a/src/util/alloc/allocator.rs +++ b/src/util/alloc/allocator.rs @@ -424,10 +424,6 @@ pub trait Allocator: Downcast { return result; } - if result.is_zero() && !self.get_context().get_alloc_options().allow_oom_call() { - return result; - } - if !result.is_zero() { // Report allocation success to assist OutOfMemory handling. if !self @@ -482,6 +478,17 @@ pub trait Allocator: Downcast { return result; } + // From here on, we handle the case that alloc_once failed. + assert!(result.is_zero()); + + if !self.get_context().get_alloc_options().at_safepoint { + // If the allocation is not at safepoint, it will not be able to block for GC. But + // the code beyond this point tests OOM conditions and, if not OOM, try to allocate + // again. Since we didn't block for GC, the allocation will fail again if we try + // again. So we return null immediately. + return Address::ZERO; + } + // It is possible to have cases where a thread is blocked for another GC (non emergency) // immediately after being blocked for a GC (emergency) (e.g. in stress test), that is saying // the thread does not leave this loop between the two GCs. The local var 'emergency_collection' From 9d7d4ac5a1cceadb2adea2e16fc1e3931489e7d6 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 Oct 2025 13:13:31 +0800 Subject: [PATCH 11/13] Add allow_oom_call as an option --- src/policy/lockfreeimmortalspace.rs | 2 +- src/policy/space.rs | 2 +- src/util/alloc/allocator.rs | 18 ++++--- ...mock_test_allocate_no_gc_oom_on_acquire.rs | 48 ++++++++++++++++++- 4 files changed, 60 insertions(+), 10 deletions(-) diff --git a/src/policy/lockfreeimmortalspace.rs b/src/policy/lockfreeimmortalspace.rs index 1a34d179e1..5395273d91 100644 --- a/src/policy/lockfreeimmortalspace.rs +++ b/src/policy/lockfreeimmortalspace.rs @@ -151,7 +151,7 @@ impl Space for LockFreeImmortalSpace { }) .expect("update cursor failed"); if start + bytes > self.limit { - if alloc_options.allow_oom_call() { + if alloc_options.allow_oom_call { panic!("OutOfMemory"); } else { return Address::ZERO; diff --git a/src/policy/space.rs b/src/policy/space.rs index ea495bc26f..77f5cf049a 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -85,7 +85,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { alloc_options: AllocationOptions, ) -> bool { if self.will_oom_on_acquire(size) { - if alloc_options.allow_oom_call() { + if alloc_options.allow_oom_call { VM::VMCollection::out_of_memory( tls, crate::util::alloc::AllocationError::HeapOutOfMemory, diff --git a/src/util/alloc/allocator.rs b/src/util/alloc/allocator.rs index 80e689e833..32a4138a47 100644 --- a/src/util/alloc/allocator.rs +++ b/src/util/alloc/allocator.rs @@ -51,6 +51,16 @@ pub struct AllocationOptions { /// If `false`, the allocation will immediately return a null address if the allocation cannot /// be satisfied without a GC. pub at_safepoint: bool, + + /// Whether the allocation is allowed to call [`Collection::out_of_memory`]. + /// + /// **The default is `true`**. + /// + /// If `true`, the allocation will call [`Collection::out_of_memory`] when out of memory and + /// return null. + /// + /// If `fasle`, the allocation will return null immediately when out of memory. + pub allow_oom_call: bool, } /// The default value for `AllocationOptions` has the same semantics as calling [`Allocator::alloc`] @@ -60,6 +70,7 @@ impl Default for AllocationOptions { Self { allow_overcommit: false, at_safepoint: true, + allow_oom_call: true, } } } @@ -68,13 +79,6 @@ impl AllocationOptions { pub(crate) fn is_default(&self) -> bool { *self == AllocationOptions::default() } - - /// Whether this allocation allows calling [`Collection::out_of_memory`]. - /// - /// It is allowed if and only if the allocation is at safepoint. - pub(crate) fn allow_oom_call(&self) -> bool { - self.at_safepoint - } } /// A wrapper for [`AllocatorContext`] to hold a [`AllocationOptions`] that can be modified by the diff --git a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs index b8898ef0ea..890bf38d83 100644 --- a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs +++ b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs @@ -4,8 +4,53 @@ use crate::util::alloc::allocator::AllocationOptions; use crate::AllocationSemantics; /// This test will allocate an object that is larger than the heap size. The call will fail. +/// It will call `Collection::out_of_memory` and return null. #[test] -pub fn allocate_no_gc_oom_on_acquire() { +pub fn allocate_no_gc_oom_on_acquire_allow_oom_call() { + //let mut out_of_memory_called = false; + // 1MB heap + with_mockvm( + || -> MockVM { + MockVM { + out_of_memory: MockMethod::new_default(), + ..MockVM::default() + } + }, + || { + const KB: usize = 1024; + let mut fixture = MutatorFixture::create_with_heapsize(KB); + + // Attempt to allocate an object that is larger than the heap size. + let addr = memory_manager::alloc_with_options( + &mut fixture.mutator, + 1024 * 10, + 8, + 0, + AllocationSemantics::Default, + AllocationOptions { + at_safepoint: false, + ..Default::default() + }, + ); + // We should get zero. + assert!(addr.is_zero()); + // block_for_gc won't be called. + read_mockvm(|mock| { + assert!(!mock.block_for_gc.is_called()); + }); + // out_of_memory should be called. + read_mockvm(|mock| { + assert!(mock.out_of_memory.is_called()); + }); + }, + no_cleanup, + ) +} + +/// This test will allocate an object that is larger than the heap size. The call will fail by +/// returning null. +#[test] +pub fn allocate_no_gc_oom_on_acquire_no_oom_call() { // 1MB heap with_mockvm( default_setup, @@ -22,6 +67,7 @@ pub fn allocate_no_gc_oom_on_acquire() { AllocationSemantics::Default, AllocationOptions { at_safepoint: false, + allow_oom_call: false, ..Default::default() }, ); From a62aca9aecb9ff3d25026033fdfd506704557804 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 Oct 2025 14:28:14 +0800 Subject: [PATCH 12/13] Update migration guide --- docs/userguide/src/migration/prefix.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/userguide/src/migration/prefix.md b/docs/userguide/src/migration/prefix.md index fab23ceb58..63bcb03fa2 100644 --- a/docs/userguide/src/migration/prefix.md +++ b/docs/userguide/src/migration/prefix.md @@ -46,15 +46,16 @@ API changes: + `AllocationOptions`: It now has two boolean fields: * `allow_overcommit` * `at_safepoint` + * `allow_oom_call` Variants of the old `enum OnAllocationFail` should be migrated to the new API according to the following table: -| variant | `allow_overcommit` | `at_safepoint` | -|-----------------|--------------------|----------------| -| `RequestGC` | `false` | `true` | -| `ReturnFailure` | `false` | `false` | -| `OverCommit` | `true` | `false` | +| variant | `allow_overcommit` | `at_safepoint` | `allow_oom_call` | +|-----------------|--------------------|----------------|------------------| +| `RequestGC` | `false` | `true` | `true` | +| `ReturnFailure` | `false` | `false` | `false` | +| `OverCommit` | `true` | `false` | `false` | Note that MMTk now always polls before trying to get more pages from the page resource, and it may trigger GC. The old `OnAllocationFail::OverCommit` used to prevent polling, but it is no longer From 3a3243636c181ef30d4f2e9b023c4739d07f1561 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 Oct 2025 14:59:18 +0800 Subject: [PATCH 13/13] Split test cases --- ...te_no_gc_oom_on_acquire_allow_oom_call.rs} | 39 ----------------- ...locate_no_gc_oom_on_acquire_no_oom_call.rs | 42 +++++++++++++++++++ src/vm/tests/mock_tests/mod.rs | 3 +- 3 files changed, 44 insertions(+), 40 deletions(-) rename src/vm/tests/mock_tests/{mock_test_allocate_no_gc_oom_on_acquire.rs => mock_test_allocate_no_gc_oom_on_acquire_allow_oom_call.rs} (54%) create mode 100644 src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire_no_oom_call.rs diff --git a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire_allow_oom_call.rs similarity index 54% rename from src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs rename to src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire_allow_oom_call.rs index 890bf38d83..01766c19ec 100644 --- a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire.rs +++ b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire_allow_oom_call.rs @@ -7,7 +7,6 @@ use crate::AllocationSemantics; /// It will call `Collection::out_of_memory` and return null. #[test] pub fn allocate_no_gc_oom_on_acquire_allow_oom_call() { - //let mut out_of_memory_called = false; // 1MB heap with_mockvm( || -> MockVM { @@ -46,41 +45,3 @@ pub fn allocate_no_gc_oom_on_acquire_allow_oom_call() { no_cleanup, ) } - -/// This test will allocate an object that is larger than the heap size. The call will fail by -/// returning null. -#[test] -pub fn allocate_no_gc_oom_on_acquire_no_oom_call() { - // 1MB heap - with_mockvm( - default_setup, - || { - const KB: usize = 1024; - let mut fixture = MutatorFixture::create_with_heapsize(KB); - - // Attempt to allocate an object that is larger than the heap size. - let addr = memory_manager::alloc_with_options( - &mut fixture.mutator, - 1024 * 10, - 8, - 0, - AllocationSemantics::Default, - AllocationOptions { - at_safepoint: false, - allow_oom_call: false, - ..Default::default() - }, - ); - // We should get zero. - assert!(addr.is_zero()); - // block_for_gc and out_of_memory won't be called. - read_mockvm(|mock| { - assert!(!mock.block_for_gc.is_called()); - }); - read_mockvm(|mock| { - assert!(!mock.out_of_memory.is_called()); - }); - }, - no_cleanup, - ) -} diff --git a/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire_no_oom_call.rs b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire_no_oom_call.rs new file mode 100644 index 0000000000..d35635ce13 --- /dev/null +++ b/src/vm/tests/mock_tests/mock_test_allocate_no_gc_oom_on_acquire_no_oom_call.rs @@ -0,0 +1,42 @@ +use super::mock_test_prelude::*; + +use crate::util::alloc::allocator::AllocationOptions; +use crate::AllocationSemantics; + +/// This test will allocate an object that is larger than the heap size. The call will fail by +/// returning null. +#[test] +pub fn allocate_no_gc_oom_on_acquire_no_oom_call() { + // 1MB heap + with_mockvm( + default_setup, + || { + const KB: usize = 1024; + let mut fixture = MutatorFixture::create_with_heapsize(KB); + + // Attempt to allocate an object that is larger than the heap size. + let addr = memory_manager::alloc_with_options( + &mut fixture.mutator, + 1024 * 10, + 8, + 0, + AllocationSemantics::Default, + AllocationOptions { + at_safepoint: false, + allow_oom_call: false, + ..Default::default() + }, + ); + // We should get zero. + assert!(addr.is_zero()); + // block_for_gc and out_of_memory won't be called. + read_mockvm(|mock| { + assert!(!mock.block_for_gc.is_called()); + }); + read_mockvm(|mock| { + assert!(!mock.out_of_memory.is_called()); + }); + }, + no_cleanup, + ) +} diff --git a/src/vm/tests/mock_tests/mod.rs b/src/vm/tests/mock_tests/mod.rs index 0fa03b517f..9a11d03d03 100644 --- a/src/vm/tests/mock_tests/mod.rs +++ b/src/vm/tests/mock_tests/mod.rs @@ -24,7 +24,8 @@ pub(crate) mod mock_test_prelude { } mod mock_test_allocate_align_offset; -mod mock_test_allocate_no_gc_oom_on_acquire; +mod mock_test_allocate_no_gc_oom_on_acquire_allow_oom_call; +mod mock_test_allocate_no_gc_oom_on_acquire_no_oom_call; mod mock_test_allocate_no_gc_simple; mod mock_test_allocate_nonmoving; mod mock_test_allocate_overcommit;