Add API disable_collection() (#474)

* Rename current enable_collection() to initialize_collection()
* Implement enable/disable_collection()
* Add tests for enable/disable_collection

Author: qinsoon

src/memory_manager.rs

@@ -38,6 +38,8 @@ pub fn start_control_collector<VM: VMBinding>(mmtk: &MMTK<VM>, tls: VMWorkerThre
 /// Initialize an MMTk instance. A VM should call this method after creating an [MMTK](../mmtk/struct.MMTK.html)
 /// instance but before using any of the methods provided in MMTk. This method will attempt to initialize a
 /// logger. If the VM would like to use its own logger, it should initialize the logger before calling this method.
+/// Note that, to allow MMTk to do GC properly, `initialize_collection()` needs to be called after this call when
+/// the VM's thread system is ready to spawn GC workers.
 ///
 /// Arguments:
 /// * `mmtk`: A reference to an MMTk instance to initialize.
@@ -177,20 +179,63 @@ pub fn start_worker<VM: VMBinding>(
     worker.run(mmtk);
 }
 
-/// Allow MMTk to trigger garbage collection. A VM should only call this method when it is ready for the mechanisms required for
-/// collection during the boot process. MMTk will invoke Collection::spawn_worker_thread() to create GC threads during
-/// this funciton call.
+/// Initialize the scheduler and GC workers that are required for doing garbage collections.
+/// This is a mandatory call for a VM during its boot process once its thread system
+/// is ready. This should only be called once. This call will invoke Collection::spawn_worker_thread()
+/// to create GC threads.
 ///
 /// Arguments:
 /// * `mmtk`: A reference to an MMTk instance.
 /// * `tls`: The thread that wants to enable the collection. This value will be passed back to the VM in
 ///   Collection::spawn_worker_thread() so that the VM knows the context.
-pub fn enable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThread) {
+pub fn initialize_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThread) {
+    assert!(
+        !mmtk.plan.is_initialized(),
+        "MMTk collection has been initialized (was initialize_collection() already called before?)"
+    );
     mmtk.scheduler.initialize(mmtk.options.threads, mmtk, tls);
     VM::VMCollection::spawn_worker_thread(tls, None); // spawn controller thread
     mmtk.plan.base().initialized.store(true, Ordering::SeqCst);
 }
 
+/// Allow MMTk to trigger garbage collection when heap is full. This should only be used in pair with disable_collection().
+/// See the comments on disable_collection(). If disable_collection() is not used, there is no need to call this function at all.
+/// Note this call is not thread safe, only one VM thread should call this.
+///
+/// Arguments:
+/// * `mmtk`: A reference to an MMTk instance.
+pub fn enable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
+    debug_assert!(
+        !mmtk.plan.should_trigger_gc_when_heap_is_full(),
+        "enable_collection() is called when GC is already enabled."
+    );
+    mmtk.plan
+        .base()
+        .trigger_gc_when_heap_is_full
+        .store(true, Ordering::SeqCst);
+}
+
+/// Disallow MMTk to trigger garbage collection. When collection is disabled, you can still allocate through MMTk. But MMTk will
+/// not trigger a GC even if the heap is full. In such a case, the allocation will exceed the MMTk's heap size (the soft heap limit).
+/// However, there is no guarantee that the physical allocation will succeed, and if it succeeds, there is no guarantee that further allocation
+/// will keep succeeding. So if a VM disables collection, it needs to allocate with careful consideration to make sure that the physical memory
+/// allows the amount of allocation. We highly recommend not using this method. However, we support this to accomodate some VMs that require this
+/// behavior. This call does not disable explicit GCs (through handle_user_collection_request()).
+/// Note this call is not thread safe, only one VM thread should call this.
+///
+/// Arguments:
+/// * `mmtk`: A reference to an MMTk instance.
+pub fn disable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
+    debug_assert!(
+        mmtk.plan.should_trigger_gc_when_heap_is_full(),
+        "disable_collection() is called when GC is not enabled."
+    );
+    mmtk.plan
+        .base()
+        .trigger_gc_when_heap_is_full
+        .store(false, Ordering::SeqCst);
+}
+
 /// Process MMTk run-time options.
 ///
 /// Arguments:

src/plan/global.rs

@@ -252,6 +252,12 @@ pub trait Plan: 'static + Sync + Downcast {
         self.base().initialized.load(Ordering::SeqCst)
     }
 
+    fn should_trigger_gc_when_heap_is_full(&self) -> bool {
+        self.base()
+            .trigger_gc_when_heap_is_full
+            .load(Ordering::SeqCst)
+    }
+
     fn prepare(&mut self, tls: VMWorkerThread);
     fn release(&mut self, tls: VMWorkerThread);
 
@@ -370,8 +376,11 @@ pub enum GcStatus {
 BasePlan should contain all plan-related state and functions that are _fundamental_ to _all_ plans.  These include VM-specific (but not plan-specific) features such as a code space or vm space, which are fundamental to all plans for a given VM.  Features that are common to _many_ (but not intrinsically _all_) plans should instead be included in CommonPlan.
 */
 pub struct BasePlan<VM: VMBinding> {
-    // Whether MMTk is now ready for collection. This is set to true when enable_collection() is called.
+    /// Whether MMTk is now ready for collection. This is set to true when initialize_collection() is called.
     pub initialized: AtomicBool,
+    /// Should we trigger a GC when the heap is full? It seems this should always be true. However, we allow
+    /// bindings to temporarily disable GC, at which point, we do not trigger GC even if the heap is full.
+    pub trigger_gc_when_heap_is_full: AtomicBool,
     pub gc_status: Mutex<GcStatus>,
     pub last_stress_pages: AtomicUsize,
     pub stacks_prepared: AtomicBool,
@@ -503,6 +512,7 @@ impl<VM: VMBinding> BasePlan<VM> {
             ),
 
             initialized: AtomicBool::new(false),
+            trigger_gc_when_heap_is_full: AtomicBool::new(true),
             gc_status: Mutex::new(GcStatus::NotInGC),
             last_stress_pages: AtomicUsize::new(0),
             stacks_prepared: AtomicBool::new(false),

src/policy/space.rs

@@ -275,10 +275,14 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
 
     fn acquire(&self, tls: VMThread, pages: usize) -> Address {
         trace!("Space.acquire, tls={:?}", tls);
-        // Should we poll to attempt to GC? If tls is collector, we cant attempt a GC.
-        let should_poll = VM::VMActivePlan::is_mutator(tls);
-        // Is a GC allowed here? enable_collection() has to be called so we know GC is initialized.
-        let allow_poll = should_poll && VM::VMActivePlan::global().is_initialized();
+        // Should we poll to attempt to GC?
+        // - If tls is collector, we cannot attempt a GC.
+        // - If gc is disabled, we cannot attempt a GC.
+        let should_poll = VM::VMActivePlan::is_mutator(tls)
+            && VM::VMActivePlan::global().should_trigger_gc_when_heap_is_full();
+        // Is a GC allowed here? If we should poll but are not allowed to poll, we will panic.
+        // initialize_collection() has to be called so we know GC is initialized.
+        let allow_gc = should_poll && VM::VMActivePlan::global().is_initialized();
 
         trace!("Reserving pages");
         let pr = self.get_page_resource();
@@ -288,8 +292,8 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
 
         if should_poll && VM::VMActivePlan::global().poll(false, self.as_space()) {
             debug!("Collection required");
-            if !allow_poll {
-                panic!("Collection is not enabled.");
+            if !allow_gc {
+                panic!("GC is not allowed here: collection is not initialized (did you call initialize_collection()?).");
             }
             pr.clear_request(pages_reserved);
             VM::VMCollection::block_for_gc(VMMutatorThread(tls)); // We have checked that this is mutator
@@ -330,8 +334,8 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
                 }
                 Err(_) => {
                     // We thought we had memory to allocate, but somehow failed the allocation. Will force a GC.
-                    if !allow_poll {
-                        panic!("Physical allocation failed when polling not allowed!");
+                    if !allow_gc {
+                        panic!("Physical allocation failed when GC is not allowed!");
                     }
 
                     let gc_performed = VM::VMActivePlan::global().poll(true, self.as_space());

src/vm/collection.rs

@@ -42,11 +42,11 @@ pub trait Collection<VM: VMBinding> {
     /// Ask the VM to spawn a GC thread for MMTk. A GC thread may later call into the VM through these VM traits. Some VMs
     /// have assumptions that those calls needs to be within VM internal threads.
     /// As a result, MMTk does not spawn GC threads itself to avoid breaking this kind of assumptions.
-    /// MMTk calls this method to spawn GC threads during [`enable_collection()`](../memory_manager/fn.enable_collection.html).
+    /// MMTk calls this method to spawn GC threads during [`initialize_collection()`](../memory_manager/fn.initialize_collection.html).
     ///
     /// Arguments:
     /// * `tls`: The thread pointer for the parent thread that we spawn new threads from. This is the same `tls` when the VM
-    ///   calls `enable_collection()` and passes as an argument.
+    ///   calls `initialize_collection()` and passes as an argument.
     /// * `ctx`: The GC worker context for the GC thread. If `None` is passed, it means spawning a GC thread for the GC controller,
     ///   which does not have a worker context.
     fn spawn_worker_thread(tls: VMThread, ctx: Option<&GCWorker<VM>>);

vmbindings/dummyvm/src/api.rs

@@ -66,8 +66,18 @@ pub extern "C" fn start_worker(tls: VMWorkerThread, worker: &'static mut GCWorke
 }
 
 #[no_mangle]
-pub extern "C" fn enable_collection(tls: VMThread) {
-    memory_manager::enable_collection(&SINGLETON, tls)
+pub extern "C" fn initialize_collection(tls: VMThread) {
+    memory_manager::initialize_collection(&SINGLETON, tls)
+}
+
+#[no_mangle]
+pub extern "C" fn disable_collection() {
+    memory_manager::disable_collection(&SINGLETON)
+}
+
+#[no_mangle]
+pub extern "C" fn enable_collection() {
+    memory_manager::enable_collection(&SINGLETON)
 }
 
 #[no_mangle]

vmbindings/dummyvm/src/collection.rs

@@ -16,11 +16,11 @@ impl Collection<DummyVM> for VMCollection {
     }
 
     fn block_for_gc(_tls: VMMutatorThread) {
-        unimplemented!();
+        panic!("block_for_gc is not implemented")
     }
 
     fn spawn_worker_thread(_tls: VMThread, _ctx: Option<&GCWorker<DummyVM>>) {
-        unimplemented!();
+
     }
 
     fn prepare_mutator<T: MutatorContext<DummyVM>>(_tls_w: VMWorkerThread, _tls_m: VMMutatorThread, _mutator: &T) {

vmbindings/dummyvm/src/tests/allcoate_with_disable_collection.rs

@@ -0,0 +1,22 @@
+use crate::api::*;
+use mmtk::util::opaque_pointer::*;
+use mmtk::AllocationSemantics;
+
+/// This test allocates after calling disable_collection(). When we exceed the heap limit, MMTk will NOT trigger a GC.
+/// And the allocation will succeed.
+#[test]
+pub fn allocate_with_disable_collection() {
+    const MB: usize = 1024 * 1024;
+    // 1MB heap
+    gc_init(MB);
+    initialize_collection(VMThread::UNINITIALIZED);
+    let handle = bind_mutator(VMMutatorThread(VMThread::UNINITIALIZED));
+    // Allocate 1MB. It should be fine.
+    let addr = alloc(handle, MB, 8, 0, AllocationSemantics::Default);
+    assert!(!addr.is_zero());
+    // Disable GC
+    disable_collection();
+    // Allocate another MB. This exceeds the heap size. But as we have disabled GC, MMTk will not trigger a GC, and allow this allocation.
+    let addr = alloc(handle, MB, 8, 0, AllocationSemantics::Default);
+    assert!(!addr.is_zero());
+}

vmbindings/dummyvm/src/tests/allocate_with_initialize_collection.rs

@@ -0,0 +1,18 @@
+use crate::api::*;
+use mmtk::util::opaque_pointer::*;
+use mmtk::AllocationSemantics;
+
+/// This test allocates after calling initialize_collection(). When we exceed the heap limit, MMTk will trigger a GC. And block_for_gc will be called.
+/// We havent implemented block_for_gc so it will panic.
+#[test]
+#[should_panic(expected = "block_for_gc is not implemented")]
+pub fn allocate_with_initialize_collection() {
+    const MB: usize = 1024 * 1024;
+    // 1MB heap
+    gc_init(MB);
+    initialize_collection(VMThread::UNINITIALIZED);
+    let handle = bind_mutator(VMMutatorThread(VMThread::UNINITIALIZED));
+    // Attempt to allocate 2MB. This will trigger GC.
+    let addr = alloc(handle, 2 * MB, 8, 0, AllocationSemantics::Default);
+    assert!(!addr.is_zero());
+}

vmbindings/dummyvm/src/tests/allocate_with_re_enable_collection.rs

@@ -0,0 +1,26 @@
+use crate::api::*;
+use mmtk::util::opaque_pointer::*;
+use mmtk::AllocationSemantics;
+
+/// This test allocates after calling initialize_collection(). When we exceed the heap limit, MMTk will trigger a GC. And block_for_gc will be called.
+/// We havent implemented block_for_gc so it will panic. This test is similar to allocate_with_initialize_collection, except that we once disabled GC in the test.
+#[test]
+#[should_panic(expected = "block_for_gc is not implemented")]
+pub fn allocate_with_re_enable_collection() {
+    const MB: usize = 1024 * 1024;
+    // 1MB heap
+    gc_init(MB);
+    initialize_collection(VMThread::UNINITIALIZED);
+    let handle = bind_mutator(VMMutatorThread(VMThread::UNINITIALIZED));
+    // Allocate 1MB. It should be fine.
+    let addr = alloc(handle, MB, 8, 0, AllocationSemantics::Default);
+    assert!(!addr.is_zero());
+    // Disable GC. So we can keep allocate without triggering a GC.
+    disable_collection();
+    let addr = alloc(handle, MB, 8, 0, AllocationSemantics::Default);
+    assert!(!addr.is_zero());
+    // Enable GC again. When we allocate, we should see a GC triggered immediately.
+    enable_collection();
+    let addr = alloc(handle, MB, 8, 0, AllocationSemantics::Default);
+    assert!(!addr.is_zero());
+}

vmbindings/dummyvm/src/tests/allocate_without_initialize_collection.rs

@@ -0,0 +1,17 @@
+use crate::api::*;
+use mmtk::util::opaque_pointer::*;
+use mmtk::AllocationSemantics;
+
+/// This test allocates without calling initialize_collection(). When we exceed the heap limit, a GC should be triggered by MMTk.
+/// But as we haven't enabled collection, GC is not initialized, so MMTk will panic.
+#[test]
+#[should_panic(expected = "GC is not allowed here")]
+pub fn allocate_without_initialize_collection() {
+    const MB: usize = 1024 * 1024;
+    // 1MB heap
+    gc_init(MB);
+    let handle = bind_mutator(VMMutatorThread(VMThread::UNINITIALIZED));
+    // Attempt to allocate 2MB memory. This should trigger a GC, but as we never call initialize_collection(), we cannot do GC.
+    let addr = alloc(handle, 2 * MB, 8, 0, AllocationSemantics::Default);
+    assert!(!addr.is_zero());
+}

vmbindings/dummyvm/src/tests/mod.rs

@@ -4,4 +4,8 @@
 mod issue139;
 mod handle_mmap_oom;
 mod handle_mmap_conflict;
+mod allocate_without_initialize_collection;
+mod allocate_with_initialize_collection;
+mod allcoate_with_disable_collection;
+mod allocate_with_re_enable_collection;
 mod malloc;