Add missing docs for the rest of the util module (#1026)

This PR is a step towards #309.
* Deny `missing_docs` for the `util` module and the `vm` module.
* Change some items from `pub` to `pub(crate)`.
* Remove some unused constants.

Author: qinsoon

src/lib.rs

@@ -61,7 +61,9 @@ pub mod build_info;
 pub mod memory_manager;
 pub mod plan;
 pub mod scheduler;
+#[deny(missing_docs)]
 pub mod util;
+#[deny(missing_docs)]
 pub mod vm;
 
 pub use crate::plan::{

src/plan/immix/global.rs

@@ -40,7 +40,7 @@ pub const IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints {
     gc_header_bits: 2,
     gc_header_words: 0,
     num_specialized_scans: 1,
-    /// Max immix object size is half of a block.
+    // Max immix object size is half of a block.
     max_non_los_default_alloc_bytes: crate::policy::immix::MAX_IMMIX_OBJECT_SIZE,
     needs_prepare_mutator: false,
     ..PlanConstraints::default()

src/plan/plan_constraints.rs

@@ -51,7 +51,10 @@ impl PlanConstraints {
             num_specialized_scans: 0,
             max_non_los_default_alloc_bytes: MAX_INT,
             max_non_los_copy_bytes: MAX_INT,
-            needs_linear_scan: SUPPORT_CARD_SCANNING || LAZY_SWEEP,
+            // As `LAZY_SWEEP` is true, needs_linear_scan is true for all the plans. This is strange.
+            // https://github.com/mmtk/mmtk-core/issues/1027 trackes the issue.
+            needs_linear_scan: crate::util::constants::SUPPORT_CARD_SCANNING
+                || crate::util::constants::LAZY_SWEEP,
             needs_concurrent_workers: false,
             generate_gc_trace: false,
             may_trace_duplicate_edges: false,

src/policy/sft_map.rs

@@ -410,7 +410,7 @@ mod dense_chunk_map {
 
         pub fn new() -> Self {
             Self {
-                /// Empty space is at index 0
+                // Empty space is at index 0
                 sft: vec![SFTRefStorage::default()],
                 index_map: HashMap::new(),
             }

src/util/address.rs

@@ -129,14 +129,17 @@ impl Shl<usize> for Address {
 }
 
 impl Address {
+    /// The lowest possible address.
     pub const ZERO: Self = Address(0);
+    /// The highest possible address.
     pub const MAX: Self = Address(usize::max_value());
 
     /// creates Address from a pointer
     pub fn from_ptr<T>(ptr: *const T) -> Address {
         Address(ptr as usize)
     }
 
+    /// creates Address from a Rust reference
     pub fn from_ref<T>(r: &T) -> Address {
         Address(r as *const T as usize)
     }
@@ -180,10 +183,12 @@ impl Address {
     // These const functions are duplicated with the operator traits. But we need them,
     // as we need them to declare constants.
 
+    /// Get the number of bytes between two addresses. The current address needs to be higher than the other address.
     pub const fn get_extent(self, other: Address) -> ByteSize {
         self.0 - other.0
     }
 
+    /// Get the offset from `other` to `self`. The result is negative is `self` is lower than `other`.
     pub const fn get_offset(self, other: Address) -> ByteOffset {
         self.0 as isize - other.0 as isize
     }
@@ -192,6 +197,7 @@ impl Address {
     // The add() function is const fn, and we can use it to declare Address constants.
     // The Add trait function cannot be const.
     #[allow(clippy::should_implement_trait)]
+    /// Add an offset to the address.
     pub const fn add(self, size: usize) -> Address {
         Address(self.0 + size)
     }
@@ -200,15 +206,17 @@ impl Address {
     // The sub() function is const fn, and we can use it to declare Address constants.
     // The Sub trait function cannot be const.
     #[allow(clippy::should_implement_trait)]
+    /// Subtract an offset from the address.
     pub const fn sub(self, size: usize) -> Address {
         Address(self.0 - size)
     }
 
+    /// Bitwise 'and' with a mask.
     pub const fn and(self, mask: usize) -> usize {
         self.0 & mask
     }
 
-    // Perform a saturating subtract on the Address
+    /// Perform a saturating subtract on the Address
     pub const fn saturating_sub(self, size: usize) -> Address {
         Address(self.0.saturating_sub(size))
     }
@@ -473,6 +481,7 @@ use crate::vm::VMBinding;
 pub struct ObjectReference(usize);
 
 impl ObjectReference {
+    /// The null object reference, represented as zero.
     pub const NULL: ObjectReference = ObjectReference(0);
 
     /// Cast the object reference to its raw address. This method is mostly for the convinience of a binding.
@@ -511,6 +520,9 @@ impl ObjectReference {
         VM::VMObjectModel::ref_to_header(self)
     }
 
+    /// Get the start of the allocation address for the object. This method is used by MMTk to get the start of the allocation
+    /// address originally returned from [`crate::memory_manager::alloc`] for the object.
+    /// This method is syntactic sugar for [`crate::vm::ObjectModel::ref_to_object_start`]. See comments on [`crate::vm::ObjectModel::ref_to_object_start`].
     pub fn to_object_start<VM: VMBinding>(self) -> Address {
         use crate::vm::ObjectModel;
         let object_start = VM::VMObjectModel::ref_to_object_start(self);
@@ -557,6 +569,7 @@ impl ObjectReference {
         }
     }
 
+    /// Can the object be moved?
     pub fn is_movable(self) -> bool {
         unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_movable()
     }
@@ -566,10 +579,12 @@ impl ObjectReference {
         unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.get_forwarded_object(self)
     }
 
+    /// Is the object in any MMTk spaces?
     pub fn is_in_any_space(self) -> bool {
         unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_in_space(self)
     }
 
+    /// Is the object sane?
     #[cfg(feature = "sanity")]
     pub fn is_sane(self) -> bool {
         unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_sane()

src/util/alloc/allocator.rs

@@ -83,6 +83,7 @@ pub fn align_allocation_inner<VM: VMBinding>(
     region + delta
 }
 
+/// Fill the specified region with the alignment value.
 pub fn fill_alignment_gap<VM: VMBinding>(immut_start: Address, end: Address) {
     let mut start = immut_start;
 

src/util/alloc/allocators.rs

@@ -167,27 +167,35 @@ impl<VM: VMBinding> Allocators<VM> {
     }
 }
 
-// This type describe which allocator in the allocators set.
-// For VM binding implementors, this type is equivalent to the following native types:
-// #[repr(C)]
-// struct AllocatorSelector {
-//   tag: AllocatorSelectorTag,
-//   payload: u8,
-// }
-// #[repr(u8)]
-// enum AllocatorSelectorTag {
-//   BumpPointer,
-//   LargeObject,
-// }
+/// This type describe an allocator in the [`crate::Mutator`].
+/// For some VM bindings, they may need to access this type from native code. This type is equivalent to the following native types:
+/// #[repr(C)]
+/// struct AllocatorSelector {
+///   tag: AllocatorSelectorTag,
+///   payload: u8,
+/// }
+/// #[repr(u8)]
+/// enum AllocatorSelectorTag {
+///   BumpPointer,
+///   LargeObject,
+///   ...
+/// }
 #[repr(C, u8)]
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Default)]
 pub enum AllocatorSelector {
+    /// Represents a [`crate::util::alloc::bumpallocator::BumpAllocator`].
     BumpPointer(u8),
+    /// Represents a [`crate::util::alloc::large_object_allocator::LargeObjectAllocator`].
     LargeObject(u8),
+    /// Represents a [`crate::util::alloc::malloc_allocator::MallocAllocator`].
     Malloc(u8),
+    /// Represents a [`crate::util::alloc::immix_allocator::ImmixAllocator`].
     Immix(u8),
+    /// Represents a [`crate::util::alloc::markcompact_allocator::MarkCompactAllocator`].
     MarkCompact(u8),
+    /// Represents a [`crate::util::alloc::free_list_allocator::FreeListAllocator`].
     FreeList(u8),
+    /// No allocator found.
     #[default]
     None,
 }
@@ -197,11 +205,15 @@ pub enum AllocatorSelector {
 #[repr(C, u8)]
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Default)]
 pub enum AllocatorInfo {
+    /// This allocator uses a [`crate::util::alloc::bumpallocator::BumpPointer`] as its fastpath.
     BumpPointer {
+        /// The byte offset from the mutator's pointer to the [`crate::util::alloc::bumpallocator::BumpPointer`].
         bump_pointer_offset: usize,
     },
+    /// This allocator uses a fastpath, but we haven't implemented it yet.
     // FIXME: Add free-list fast-path
     Unimplemented,
+    /// This allocator does not have a fastpath.
     #[default]
     None,
 }

src/util/alloc/bumpallocator.rs

@@ -5,14 +5,16 @@ use crate::util::Address;
 use crate::util::alloc::Allocator;
 
 use crate::policy::space::Space;
-use crate::util::conversions::bytes_to_pages;
+use crate::util::conversions::bytes_to_pages_up;
 use crate::util::opaque_pointer::*;
 use crate::vm::VMBinding;
 
 const BYTES_IN_PAGE: usize = 1 << 12;
 const BLOCK_SIZE: usize = 8 * BYTES_IN_PAGE;
 const BLOCK_MASK: usize = BLOCK_SIZE - 1;
 
+/// A bump pointer allocator. It keeps a thread local allocation buffer,
+/// and bumps a cursor to allocate from the buffer.
 #[repr(C)]
 pub struct BumpAllocator<VM: VMBinding> {
     /// [`VMThread`] associated with this allocator instance
@@ -32,11 +34,14 @@ pub struct BumpAllocator<VM: VMBinding> {
 #[repr(C)]
 #[derive(Copy, Clone)]
 pub struct BumpPointer {
+    /// The cursor inside the allocation buffer where the next object will be allocated.
     pub cursor: Address,
+    /// The upperbound of the allocation buffer.
     pub limit: Address,
 }
 
 impl BumpPointer {
+    /// Reset the cursor and limit to the given values.
     pub fn reset(&mut self, start: Address, end: Address) {
         self.cursor = start;
         self.limit = end;
@@ -46,7 +51,7 @@ 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.    
+    /// `cursor + size < limit`, and go to the slowpath.
     fn default() -> Self {
         BumpPointer {
             cursor: Address::ZERO,
@@ -56,16 +61,16 @@ impl std::default::Default for BumpPointer {
 }
 
 impl<VM: VMBinding> BumpAllocator<VM> {
-    pub fn set_limit(&mut self, start: Address, limit: Address) {
+    pub(crate) fn set_limit(&mut self, start: Address, limit: Address) {
         self.bump_pointer.reset(start, limit);
     }
 
-    pub fn reset(&mut self) {
+    pub(crate) fn reset(&mut self) {
         let zero = unsafe { Address::zero() };
         self.bump_pointer.reset(zero, zero);
     }
 
-    pub fn rebind(&mut self, space: &'static dyn Space<VM>) {
+    pub(crate) fn rebind(&mut self, space: &'static dyn Space<VM>) {
         self.reset();
         self.space = space;
     }
@@ -194,7 +199,7 @@ impl<VM: VMBinding> BumpAllocator<VM> {
         }
 
         let block_size = (size + BLOCK_MASK) & (!BLOCK_MASK);
-        let acquired_start = self.space.acquire(self.tls, bytes_to_pages(block_size));
+        let acquired_start = self.space.acquire(self.tls, bytes_to_pages_up(block_size));
         if acquired_start.is_zero() {
             trace!("Failed to acquire a new block");
             acquired_start

src/util/alloc/free_list_allocator.rs

@@ -15,6 +15,7 @@ use super::allocator::AllocatorContext;
 /// A MiMalloc free list allocator
 #[repr(C)]
 pub struct FreeListAllocator<VM: VMBinding> {
+    /// [`VMThread`] associated with this allocator instance
     pub tls: VMThread,
     space: &'static MarkSweepSpace<VM>,
     context: Arc<AllocatorContext<VM>>,

src/util/alloc/immix_allocator.rs

@@ -16,7 +16,9 @@ use crate::vm::*;
 /// Immix allocator
 #[repr(C)]
 pub struct ImmixAllocator<VM: VMBinding> {
+    /// [`VMThread`] associated with this allocator instance
     pub tls: VMThread,
+    /// The fastpath bump pointer.
     pub bump_pointer: BumpPointer,
     /// [`Space`](src/policy/space/Space) instance associated with this allocator instance.
     space: &'static ImmixSpace<VM>,
@@ -34,7 +36,7 @@ pub struct ImmixAllocator<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> ImmixAllocator<VM> {
-    pub fn reset(&mut self) {
+    pub(crate) fn reset(&mut self) {
         self.bump_pointer.reset(Address::ZERO, Address::ZERO);
         self.large_bump_pointer.reset(Address::ZERO, Address::ZERO);
         self.request_for_large = false;
@@ -183,7 +185,7 @@ impl<VM: VMBinding> ImmixAllocator<VM> {
         }
     }
 
-    pub fn immix_space(&self) -> &'static ImmixSpace<VM> {
+    pub(crate) fn immix_space(&self) -> &'static ImmixSpace<VM> {
         self.space
     }