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.

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
  }
 

src/util/alloc/large_object_allocator.rs

@@ -9,6 +9,8 @@ use crate::vm::VMBinding;
 
 use super::allocator::AllocatorContext;
 
+/// An allocator that only allocates at page granularity.
+/// This is intended for large objects.
 #[repr(C)]
 pub struct LargeObjectAllocator<VM: VMBinding> {
  /// [`VMThread`] associated with this allocator instance

src/util/alloc/malloc_allocator.rs

@@ -9,6 +9,8 @@ use crate::vm::VMBinding;
 
 use super::allocator::AllocatorContext;
 
+/// The allocator that internally uses malloc for all the allocation requests.
+/// This allocator is only intended for experimental uses.
 #[repr(C)]
 pub struct MallocAllocator<VM: VMBinding> {
  /// [`VMThread`] associated with this allocator instance

src/util/alloc/markcompact_allocator.rs

@@ -16,15 +16,15 @@ pub struct MarkCompactAllocator<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> MarkCompactAllocator<VM> {
- pub fn set_limit(&mut self, cursor: Address, limit: Address) {
+ pub(crate) fn set_limit(&mut self, cursor: Address, limit: Address) {
  self.bump_allocator.set_limit(cursor, limit);
  }
 
- pub fn reset(&mut self) {
+ pub(crate) fn reset(&mut self) {
  self.bump_allocator.reset();
  }
 
- pub fn rebind(&mut self, space: &'static dyn Space<VM>) {
+ pub(crate) fn rebind(&mut self, space: &'static dyn Space<VM>) {
  self.bump_allocator.rebind(space);
  }
 }
@@ -89,6 +89,7 @@ impl<VM: VMBinding> Allocator<VM> for MarkCompactAllocator<VM> {
 }
 
 impl<VM: VMBinding> MarkCompactAllocator<VM> {
+ /// The number of bytes that the allocator reserves for its own header.
  pub const HEADER_RESERVED_IN_BYTES: usize =
  crate::policy::markcompactspace::MarkCompactSpace::<VM>::HEADER_RESERVED_IN_BYTES;
  pub(crate) fn new(

src/util/alloc/mod.rs

@@ -27,7 +27,7 @@ pub use malloc_allocator::MallocAllocator;
 pub mod immix_allocator;
 pub use self::immix_allocator::ImmixAllocator;
 
-// Free list allocator based on Mimalloc
+/// Free list allocator based on Mimalloc
 pub mod free_list_allocator;
 pub use free_list_allocator::FreeListAllocator;