Skip to content

Commit 1ffa5b3

Browse files
authored
Use boolean allocation options (#1400)
We remove `OnAllocationFail` and add boolean fields to `AllocationOptions`: - `allow_overcommit`: whether we allow overcommit - `at_safepoint`: whether this allocation is at a safepoint - `allow_oom_call`: whether to call `Collection::out_of_memory` Now `Space::acquire` always polls before trying to get new pages. Particularly, when `allow_overcommit == true`, polling and over-committing will happen in one allocation attempt. If we also set `at_safepoint == false`, the current mutator will be able to allocate normally in this allocation, but block for GC at the nearest safepoint. This is useful for certain VMs.
1 parent e845077 commit 1ffa5b3

File tree

10 files changed

+242
-75
lines changed

10 files changed

+242
-75
lines changed

docs/userguide/src/migration/prefix.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,39 @@ Notes for the mmtk-core developers:
3232

3333
## 0.32.0
3434

35+
### Allocation options changed
36+
37+
```admonish tldr
38+
`AllocationOptions` now has multiple boolean fields instead of one `OnAllocationFail` field. Now
39+
polling cannot be disabled. Instead we can now poll and over-commit in one allocation.
40+
```
41+
42+
API changes:
43+
44+
- module `util::alloc::allocator`
45+
+ `OnAllocationFail`: Removed.
46+
+ `AllocationOptions`: It now has two boolean fields:
47+
* `allow_overcommit`
48+
* `at_safepoint`
49+
* `allow_oom_call`
50+
51+
Variants of the old `enum OnAllocationFail` should be migrated to the new API according to the
52+
following table:
53+
54+
| variant | `allow_overcommit` | `at_safepoint` | `allow_oom_call` |
55+
|-----------------|--------------------|----------------|------------------|
56+
| `RequestGC` | `false` | `true` | `true` |
57+
| `ReturnFailure` | `false` | `false` | `false` |
58+
| `OverCommit` | `true` | `false` | `false` |
59+
60+
Note that MMTk now always polls before trying to get more pages from the page resource, and it may
61+
trigger GC. The old `OnAllocationFail::OverCommit` used to prevent polling, but it is no longer
62+
possible.
63+
64+
See also:
65+
66+
- PR: <https://github.com/mmtk/mmtk-core/pull/1400>
67+
3568
### Removed the notion of "mmap chunk"
3669

3770
```admonish tldr

src/policy/lockfreeimmortalspace.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ impl<VM: VMBinding> Space<VM> for LockFreeImmortalSpace<VM> {
151151
})
152152
.expect("update cursor failed");
153153
if start + bytes > self.limit {
154-
if alloc_options.on_fail.allow_oom_call() {
154+
if alloc_options.allow_oom_call {
155155
panic!("OutOfMemory");
156156
} else {
157157
return Address::ZERO;

src/policy/space.rs

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
8585
alloc_options: AllocationOptions,
8686
) -> bool {
8787
if self.will_oom_on_acquire(size) {
88-
if alloc_options.on_fail.allow_oom_call() {
88+
if alloc_options.allow_oom_call {
8989
VM::VMCollection::out_of_memory(
9090
tls,
9191
crate::util::alloc::AllocationError::HeapOutOfMemory,
@@ -108,35 +108,42 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
108108
"The requested pages is larger than the max heap size. Is will_go_oom_on_acquire used before acquring memory?"
109109
);
110110

111-
// Should we poll to attempt to GC?
112-
// - If tls is collector, we cannot attempt a GC.
113-
// - If gc is disabled, we cannot attempt a GC.
114-
// - If overcommit is allowed, we don't attempt a GC.
115-
// FIXME: We should allow polling while also allowing over-committing.
116-
// We should change the allocation interface.
117-
let should_poll = VM::VMActivePlan::is_mutator(tls)
118-
&& VM::VMCollection::is_collection_enabled()
119-
&& !alloc_options.on_fail.allow_overcommit();
120-
121111
trace!("Reserving pages");
122112
let pr = self.get_page_resource();
123113
let pages_reserved = pr.reserve_pages(pages);
124114
trace!("Pages reserved");
125-
trace!("Polling ..");
126115

127-
// The actual decision tree.
128-
if should_poll && self.get_gc_trigger().poll(false, Some(self.as_space())) {
129-
self.not_acquiring(tls, alloc_options, pr, pages_reserved, false);
130-
Address::ZERO
131-
} else {
132-
debug!("Collection not required");
116+
// Should we poll before acquring pages from page resources so that it can trigger a GC?
117+
// - If tls is collector, we cannot attempt a GC.
118+
// - If gc is disabled, we cannot attempt a GC.
119+
let should_poll =
120+
VM::VMActivePlan::is_mutator(tls) && VM::VMCollection::is_collection_enabled();
121+
122+
// If we should poll, do it now. Record if it has triggered a GC.
123+
// If we should not poll, GC is not triggered.
124+
let gc_triggered = should_poll && {
125+
trace!("Polling ..");
126+
self.get_gc_trigger().poll(false, Some(self.as_space()))
127+
};
128+
129+
// We can try to get pages if
130+
// - GC is not triggered, or
131+
// - GC is triggered, but we allow over-committing.
132+
let should_get_pages = !gc_triggered || alloc_options.allow_overcommit;
133133

134+
// Get new pages if we should. If we didn't get new pages from the page resource for any
135+
// reason (if we decided not to, or if we tried and failed), this function shall return a
136+
// null address.
137+
if should_get_pages {
134138
if let Some(addr) = self.get_new_pages_and_initialize(tls, pages, pr, pages_reserved) {
135139
addr
136140
} else {
137-
self.not_acquiring(tls, alloc_options, pr, pages_reserved, true);
141+
self.not_acquiring(tls, alloc_options, pr, pages_reserved, false);
138142
Address::ZERO
139143
}
144+
} else {
145+
self.not_acquiring(tls, alloc_options, pr, pages_reserved, true);
146+
Address::ZERO
140147
}
141148
}
142149

@@ -268,11 +275,17 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
268275
pages_reserved: usize,
269276
attempted_allocation_and_failed: bool,
270277
) {
278+
assert!(
279+
VM::VMActivePlan::is_mutator(tls),
280+
"A non-mutator thread failed to get pages from page resource. \
281+
Copying GC plans should compute the copying headroom carefully to prevent this."
282+
);
283+
271284
// Clear the request
272285
pr.clear_request(pages_reserved);
273286

274-
// If we do not want GC on fail, just return.
275-
if !alloc_options.on_fail.allow_gc() {
287+
// If we are not at a safepoint, return immediately.
288+
if !alloc_options.at_safepoint {
276289
return;
277290
}
278291

src/util/alloc/allocator.rs

Lines changed: 109 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::util::heap::gc_trigger::GCTrigger;
66
use crate::util::options::Options;
77
use crate::MMTK;
88

9-
use atomic::Atomic;
9+
use std::cell::RefCell;
1010
use std::sync::atomic::Ordering;
1111
use std::sync::Arc;
1212

@@ -28,39 +28,51 @@ pub enum AllocationError {
2828
MmapOutOfMemory,
2929
}
3030

31-
/// Behavior when an allocation fails, and a GC is expected.
32-
#[repr(u8)]
33-
#[derive(Copy, Clone, Default, PartialEq, bytemuck::NoUninit, Debug)]
34-
pub enum OnAllocationFail {
35-
/// Request the GC. This is the default behavior.
36-
#[default]
37-
RequestGC,
38-
/// Instead of requesting GC, the allocation request returns with a failure value.
39-
ReturnFailure,
40-
/// Instead of requesting GC, the allocation request simply overcommits the memory,
41-
/// and return a valid result at its best efforts.
42-
OverCommit,
43-
}
44-
45-
impl OnAllocationFail {
46-
pub(crate) fn allow_oom_call(&self) -> bool {
47-
*self == Self::RequestGC
48-
}
49-
pub(crate) fn allow_gc(&self) -> bool {
50-
*self == Self::RequestGC
51-
}
52-
pub(crate) fn allow_overcommit(&self) -> bool {
53-
*self == Self::OverCommit
54-
}
55-
}
56-
5731
/// Allow specifying different behaviors with [`Allocator::alloc_with_options`].
5832
#[repr(C)]
59-
#[derive(Copy, Clone, Default, PartialEq, bytemuck::NoUninit, Debug)]
33+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
6034
pub struct AllocationOptions {
61-
/// When the allocation fails and a GC is originally expected, on_fail
62-
/// allows a different behavior to avoid the GC.
63-
pub on_fail: OnAllocationFail,
35+
/// Whether over-committing is allowed at this allocation site. Over-committing means the
36+
/// allocation is allowed to go beyond the current heap size. But it is not guaranteed to
37+
/// succeed.
38+
///
39+
/// **The default is `false`**.
40+
///
41+
/// Note that regardless of the value of `allow_overcommit`, the allocation may trigger GC if
42+
/// the GC trigger considers it needed.
43+
pub allow_overcommit: bool,
44+
45+
/// Whether the allocation is at a safepoint.
46+
///
47+
/// **The default is `true`**.
48+
///
49+
/// If `true`, the allocation is allowed to block for GC.
50+
///
51+
/// If `false`, the allocation will immediately return a null address if the allocation cannot
52+
/// be satisfied without a GC.
53+
pub at_safepoint: bool,
54+
55+
/// Whether the allocation is allowed to call [`Collection::out_of_memory`].
56+
///
57+
/// **The default is `true`**.
58+
///
59+
/// If `true`, the allocation will call [`Collection::out_of_memory`] when out of memory and
60+
/// return null.
61+
///
62+
/// If `fasle`, the allocation will return null immediately when out of memory.
63+
pub allow_oom_call: bool,
64+
}
65+
66+
/// The default value for `AllocationOptions` has the same semantics as calling [`Allocator::alloc`]
67+
/// directly.
68+
impl Default for AllocationOptions {
69+
fn default() -> Self {
70+
Self {
71+
allow_overcommit: false,
72+
at_safepoint: true,
73+
allow_oom_call: true,
74+
}
75+
}
6476
}
6577

6678
impl AllocationOptions {
@@ -69,6 +81,56 @@ impl AllocationOptions {
6981
}
7082
}
7183

84+
/// A wrapper for [`AllocatorContext`] to hold a [`AllocationOptions`] that can be modified by the
85+
/// same mutator thread.
86+
///
87+
/// All [`Allocator`] instances in `Allocators` share one `AllocationOptions` instance, and it will
88+
/// only be accessed by the mutator (via `Mutator::allocators`) or the GC worker (via
89+
/// `GCWorker::copy`) that owns it. Rust doesn't like multiple mutable references pointing to a
90+
/// shared data structure. We cannot use [`atomic::Atomic`] because `AllocationOptions` has
91+
/// multiple fields. We wrap it in a `RefCell` to make it internally mutable.
92+
///
93+
/// Note: The allocation option is called every time [`Allocator::alloc_with_options`] is called.
94+
/// Because API functions should only be called on allocation slow paths, we believe that `RefCell`
95+
/// should be good enough for performance. If this is too slow, we may consider `UnsafeCell`. If
96+
/// that's still too slow, we should consider changing the API to make the allocation options a
97+
/// persistent per-mutator value, and allow the VM binding set its value via a new API function.
98+
struct AllocationOptionsHolder {
99+
alloc_options: RefCell<AllocationOptions>,
100+
}
101+
102+
/// Strictly speaking, `AllocationOptionsHolder` isn't `Sync`. Two threads cannot set or clear the
103+
/// same `AllocationOptionsHolder` at the same time. However, both `Mutator` and `GCWorker` are
104+
/// `Send`, and both of which own `Allocators` and require its field `Arc<AllocationContext>` to be
105+
/// `Send`, which requires `AllocationContext` to be `Sync`, which requires
106+
/// `AllocationOptionsHolder` to be `Sync`. (Note that `Arc<T>` can be cloned and given to another
107+
/// thread, and Rust expects `T` to be `Sync`, too. But we never share `AllocationContext` between
108+
/// threads, but only between multiple `Allocator` instances within the same `Allocators` instance.
109+
/// Rust can't figure this out.)
110+
unsafe impl Sync for AllocationOptionsHolder {}
111+
112+
impl AllocationOptionsHolder {
113+
pub fn new(alloc_options: AllocationOptions) -> Self {
114+
Self {
115+
alloc_options: RefCell::new(alloc_options),
116+
}
117+
}
118+
pub fn set_alloc_options(&self, options: AllocationOptions) {
119+
let mut alloc_options = self.alloc_options.borrow_mut();
120+
*alloc_options = options;
121+
}
122+
123+
pub fn clear_alloc_options(&self) {
124+
let mut alloc_options = self.alloc_options.borrow_mut();
125+
*alloc_options = AllocationOptions::default();
126+
}
127+
128+
pub fn get_alloc_options(&self) -> AllocationOptions {
129+
let alloc_options = self.alloc_options.borrow();
130+
*alloc_options
131+
}
132+
}
133+
72134
pub fn align_allocation_no_fill<VM: VMBinding>(
73135
region: Address,
74136
alignment: usize,
@@ -180,7 +242,7 @@ pub(crate) fn assert_allocation_args<VM: VMBinding>(size: usize, align: usize, o
180242

181243
/// The context an allocator needs to access in order to perform allocation.
182244
pub struct AllocatorContext<VM: VMBinding> {
183-
pub alloc_options: Atomic<AllocationOptions>,
245+
alloc_options: AllocationOptionsHolder,
184246
pub state: Arc<GlobalState>,
185247
pub options: Arc<Options>,
186248
pub gc_trigger: Arc<GCTrigger<VM>>,
@@ -191,7 +253,7 @@ pub struct AllocatorContext<VM: VMBinding> {
191253
impl<VM: VMBinding> AllocatorContext<VM> {
192254
pub fn new(mmtk: &MMTK<VM>) -> Self {
193255
Self {
194-
alloc_options: Atomic::new(AllocationOptions::default()),
256+
alloc_options: AllocationOptionsHolder::new(AllocationOptions::default()),
195257
state: mmtk.state.clone(),
196258
options: mmtk.options.clone(),
197259
gc_trigger: mmtk.gc_trigger.clone(),
@@ -201,16 +263,15 @@ impl<VM: VMBinding> AllocatorContext<VM> {
201263
}
202264

203265
pub fn set_alloc_options(&self, options: AllocationOptions) {
204-
self.alloc_options.store(options, Ordering::Relaxed);
266+
self.alloc_options.set_alloc_options(options);
205267
}
206268

207269
pub fn clear_alloc_options(&self) {
208-
self.alloc_options
209-
.store(AllocationOptions::default(), Ordering::Relaxed);
270+
self.alloc_options.clear_alloc_options();
210271
}
211272

212273
pub fn get_alloc_options(&self) -> AllocationOptions {
213-
self.alloc_options.load(Ordering::Relaxed)
274+
self.alloc_options.get_alloc_options()
214275
}
215276
}
216277

@@ -367,12 +428,6 @@ pub trait Allocator<VM: VMBinding>: Downcast {
367428
return result;
368429
}
369430

370-
if result.is_zero()
371-
&& self.get_context().get_alloc_options().on_fail == OnAllocationFail::ReturnFailure
372-
{
373-
return result;
374-
}
375-
376431
if !result.is_zero() {
377432
// Report allocation success to assist OutOfMemory handling.
378433
if !self
@@ -427,6 +482,17 @@ pub trait Allocator<VM: VMBinding>: Downcast {
427482
return result;
428483
}
429484

485+
// From here on, we handle the case that alloc_once failed.
486+
assert!(result.is_zero());
487+
488+
if !self.get_context().get_alloc_options().at_safepoint {
489+
// If the allocation is not at safepoint, it will not be able to block for GC. But
490+
// the code beyond this point tests OOM conditions and, if not OOM, try to allocate
491+
// again. Since we didn't block for GC, the allocation will fail again if we try
492+
// again. So we return null immediately.
493+
return Address::ZERO;
494+
}
495+
430496
// It is possible to have cases where a thread is blocked for another GC (non emergency)
431497
// immediately after being blocked for a GC (emergency) (e.g. in stress test), that is saying
432498
// the thread does not leave this loop between the two GCs. The local var 'emergency_collection'

src/util/alloc/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ pub use allocator::fill_alignment_gap;
66
pub use allocator::AllocationError;
77
pub use allocator::AllocationOptions;
88
pub use allocator::Allocator;
9-
pub use allocator::OnAllocationFail;
109

1110
/// A list of all the allocators, embedded in Mutator
1211
pub(crate) mod allocators;

0 commit comments

Comments
 (0)