From abd38ec31064e7d52ae2b492a75b4bcc7f7cc554 Mon Sep 17 00:00:00 2001
From: Yi Lin <qinsoon@gmail.com>
Date: Wed, 25 Oct 2023 18:05:09 +1300
Subject: [PATCH] Use BumpPointer::default() (#993)

This PR changes the code snippet in our doc to use
`BumpPointer::default()` to create a new zero-initialized bump pointer
struct. It also changes our internal implementation to use
`BumpPointer::default()` instead of `BumpPointer::new(zero, zero)`.
---
 src/util/alloc/bumpallocator.rs                | 18 +++++++-----------
 src/util/alloc/immix_allocator.rs              |  4 ++--
 .../dummyvm/src/tests/doc_mutator_storage.rs   | 18 ++++--------------
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/src/util/alloc/bumpallocator.rs b/src/util/alloc/bumpallocator.rs
index 8b993ce5e3..98c33fc56d 100644
--- a/src/util/alloc/bumpallocator.rs
+++ b/src/util/alloc/bumpallocator.rs
@@ -26,6 +26,9 @@ pub struct BumpAllocator<VM: VMBinding> {
 
 /// A common fast-path bump-pointer allocator shared across different allocator implementations
 /// that use bump-pointer allocation.
+/// A `BumpPointer` is always initialized with cursor = 0, limit = 0, so the first allocation
+/// always fails the check of `cursor + size < limit` and goes to the slowpath. A binding
+/// can also take advantage of this design to zero-initialize the a bump pointer.
 #[repr(C)]
 #[derive(Copy, Clone)]
 pub struct BumpPointer {
@@ -34,13 +37,6 @@ pub struct BumpPointer {
 }
 
 impl BumpPointer {
-    pub const fn new(start: Address, end: Address) -> Self {
-        BumpPointer {
-            cursor: start,
-            limit: end,
-        }
-    }
-
     pub fn reset(&mut self, start: Address, end: Address) {
         self.cursor = start;
         self.limit = end;
@@ -48,10 +44,10 @@ impl BumpPointer {
 }
 
 impl std::default::Default for BumpPointer {
+    /// Defaults to 0,0. In this case, the first
+    /// allocation would naturally fail the check
+    /// `cursor + size < limit`, and go to the slowpath.    
     fn default() -> Self {
-        // Defaults to 0,0. In this case, the first
-        // allocation would naturally fail the check
-        // `cursor + size < limit`, and go to the slowpath.
         BumpPointer {
             cursor: Address::ZERO,
             limit: Address::ZERO,
@@ -180,7 +176,7 @@ impl<VM: VMBinding> BumpAllocator<VM> {
     ) -> Self {
         BumpAllocator {
             tls,
-            bump_pointer: unsafe { BumpPointer::new(Address::zero(), Address::zero()) },
+            bump_pointer: BumpPointer::default(),
             space,
             context,
         }
diff --git a/src/util/alloc/immix_allocator.rs b/src/util/alloc/immix_allocator.rs
index 994b5929ba..077aaefbd9 100644
--- a/src/util/alloc/immix_allocator.rs
+++ b/src/util/alloc/immix_allocator.rs
@@ -174,10 +174,10 @@ impl<VM: VMBinding> ImmixAllocator<VM> {
             tls,
             space: space.unwrap().downcast_ref::<ImmixSpace<VM>>().unwrap(),
             context,
-            bump_pointer: BumpPointer::new(Address::ZERO, Address::ZERO),
+            bump_pointer: BumpPointer::default(),
             hot: false,
             copy,
-            large_bump_pointer: BumpPointer::new(Address::ZERO, Address::ZERO),
+            large_bump_pointer: BumpPointer::default(),
             request_for_large: false,
             line: None,
         }
diff --git a/vmbindings/dummyvm/src/tests/doc_mutator_storage.rs b/vmbindings/dummyvm/src/tests/doc_mutator_storage.rs
index b48dcbdea6..48734626bf 100644
--- a/vmbindings/dummyvm/src/tests/doc_mutator_storage.rs
+++ b/vmbindings/dummyvm/src/tests/doc_mutator_storage.rs
@@ -76,19 +76,9 @@ pub fn embed_fastpath_struct() {
 
         // Bind an MMTk mutator
         let mutator = mmtk::memory_manager::bind_mutator(&fixture.mmtk, tls_opaque_pointer);
-        // Create a fastpath BumpPointer
-        let default_bump_pointer = {
-            // First find the allocator
-            let selector = mmtk::memory_manager::get_allocator_mapping(
-                &fixture.mmtk,
-                AllocationSemantics::Default,
-            );
-            let default_allocator = unsafe {
-                mutator.allocator_impl::<mmtk::util::alloc::BumpAllocator<DummyVM>>(selector)
-            };
-            // Copy the bump pointer struct
-            default_allocator.bump_pointer
-        };
+        // Create a fastpath BumpPointer with default(). The BumpPointer from default() will guarantee to fail on the first allocation
+        // so the allocation goes to the slowpath and we will get an allocation buffer from MMTk.
+        let default_bump_pointer = BumpPointer::default();
         // Store the fastpath BumpPointer along with the mutator
         let mut storage = MutatorInTLS {
             default_bump_pointer,
@@ -119,7 +109,7 @@ pub fn embed_fastpath_struct() {
                 default_allocator.bump_pointer = storage.default_bump_pointer;
                 // Do slow path allocation with MMTk
                 let addr = default_allocator.alloc_slow(size, 8, 0);
-                // Copy bump pointer values to the fastpath BumpPointer
+                // Copy bump pointer values to the fastpath BumpPointer so we will have an allocation buffer.
                 storage.default_bump_pointer = default_allocator.bump_pointer;
                 addr
             }