Reduce public visibility (#312)

* Turns public modules to private or pub crate if possible
* Removes some unused code/macros

Author: qinsoon

.github/scripts/ci-doc.sh

@@ -1,7 +1,11 @@
 . $(dirname "$0")/ci-common.sh
 
 # Check cargo doc
-cargo doc --features $non_exclusive_features --no-deps
+# We generate docs including private items so it would be easier for MMTk developers (GC implementers). However,
+# this could be confusing to MMTk users (binding implementers), as they may find items in the doc which
+# are not visible to a binding. If we exclude private items, the doc would be easier for the users, but would hide
+# implementation details for developers.
+cargo doc --features $non_exclusive_features --no-deps --document-private-items
 
 # Check tutorial code
 tutorial_code_dir=$project_root/docs/tutorial/code/mygc_semispace

src/lib.rs

@@ -2,6 +2,7 @@
 #![feature(asm)]
 #![feature(const_fn)]
 #![feature(integer_atomics)]
+#![feature(is_sorted)]
 #![feature(drain_filter)]
 #![feature(nll)]
 #![feature(box_syntax)]
@@ -12,6 +13,8 @@
 #![feature(associated_type_defaults)]
 #![feature(specialization)]
 #![feature(trait_alias)]
+// TODO: We should fix missing docs for public items and turn this on (Issue #309).
+// #![deny(missing_docs)]
 
 //! Memory Management ToolKit (MMTk) is a portable and high performance memory manager
 //! that includes various garbage collection algorithms and provides clean and efficient
@@ -50,17 +53,19 @@ extern crate num_cpus;
 #[macro_use]
 extern crate downcast_rs;
 
-#[macro_use]
-pub mod util;
-mod mm;
 mod mmtk;
+pub use mmtk::MMTK;
+pub(crate) use mmtk::VM_MAP;
+
+mod policy;
+
+pub mod memory_manager;
 pub mod plan;
-pub mod policy;
 pub mod scheduler;
+pub mod util;
 pub mod vm;
 
-pub use crate::mm::memory_manager;
-pub use crate::mmtk::MMTK;
 pub use crate::plan::{
-    AllocationSemantics, CopyContext, Mutator, MutatorContext, Plan, TraceLocal, TransitiveClosure,
+    AllocationSemantics, BarrierSelector, CopyContext, Mutator, MutatorContext, Plan, TraceLocal,
+    TransitiveClosure,
 };

src/mm/memory_manager.rs renamed to src/memory_manager.rs

@@ -12,9 +12,11 @@
 //! pointer. Either way, the VM binding code needs to guarantee the safety.
 
 use crate::mmtk::MMTK;
-use crate::plan::mutator_context::{Mutator, MutatorContext};
 use crate::plan::AllocationSemantics;
+use crate::plan::{Mutator, MutatorContext};
 use crate::scheduler::GCWorker;
+use crate::scheduler::Work;
+use crate::scheduler::WorkBucketStage;
 use crate::util::alloc::allocators::AllocatorSelector;
 use crate::util::constants::LOG_BYTES_IN_PAGE;
 use crate::util::heap::layout::vm_layout_constants::HEAP_END;
@@ -49,7 +51,8 @@ pub fn gc_init<VM: VMBinding>(mmtk: &'static mut MMTK<VM>, heap_size: usize) {
         ),
     }
     assert!(heap_size > 0, "Invalid heap size");
-    mmtk.plan.gc_init(heap_size, &mmtk.vm_map, &mmtk.scheduler);
+    mmtk.plan
+        .gc_init(heap_size, &crate::VM_MAP, &mmtk.scheduler);
     info!("Initialized MMTk with {:?}", mmtk.options.plan);
 }
 
@@ -63,7 +66,7 @@ pub fn bind_mutator<VM: VMBinding>(
     mmtk: &'static MMTK<VM>,
     tls: VMMutatorThread,
 ) -> Box<Mutator<VM>> {
-    crate::plan::global::create_mutator(tls, mmtk)
+    crate::plan::create_mutator(tls, mmtk)
 }
 
 /// Reclaim a mutator that is no longer needed.
@@ -372,3 +375,43 @@ pub fn get_finalized_object<VM: VMBinding>(mmtk: &'static MMTK<VM>) -> Option<Ob
         .unwrap()
         .get_ready_object()
 }
+
+/// Get the number of workers. MMTk spawns worker threads for the 'threads' defined in the options.
+/// So the number of workers is derived from the threads option. Note the feature single_worker overwrites
+/// the threads option, and force one worker thread.
+///
+/// Arguments:
+/// * `mmtk`: A reference to an MMTk instance.
+pub fn num_of_workers<VM: VMBinding>(mmtk: &'static MMTK<VM>) -> usize {
+    mmtk.scheduler.num_workers()
+}
+
+/// Add a work packet to the given work bucket. Note that this simply adds the work packet to the given
+/// work bucket, and the scheduler will decide when to execute the work packet.
+///
+/// Arguments:
+/// * `mmtk`: A reference to an MMTk instance.
+/// * `bucket`: Which work bucket to add this packet to.
+/// * `packet`: The work packet to be added.
+pub fn add_work_packet<VM: VMBinding, W: Work<MMTK<VM>>>(
+    mmtk: &'static MMTK<VM>,
+    bucket: WorkBucketStage,
+    packet: W,
+) {
+    mmtk.scheduler.work_buckets[bucket].add(packet)
+}
+
+/// Bulk add a number of work packets to the given work bucket. Note that this simply adds the work packets
+/// to the given work bucket, and the scheduler will decide when to execute the work packets.
+///
+/// Arguments:
+/// * `mmtk`: A reference to an MMTk instance.
+/// * `bucket`: Which work bucket to add these packets to.
+/// * `packet`: The work packets to be added.
+pub fn add_work_packets<VM: VMBinding>(
+    mmtk: &'static MMTK<VM>,
+    bucket: WorkBucketStage,
+    packets: Vec<Box<dyn Work<MMTK<VM>>>>,
+) {
+    mmtk.scheduler.work_buckets[bucket].bulk_add(packets)
+}

src/mm/mod.rs

@@ -1,6 +1,7 @@
+///! MMTk instance.
 use crate::plan::Plan;
 use crate::policy::space::SFTMap;
-use crate::scheduler::Scheduler;
+use crate::scheduler::MMTkScheduler;
 use crate::util::finalizable_processor::FinalizableProcessor;
 use crate::util::heap::layout::heap_layout::Mmapper;
 use crate::util::heap::layout::heap_layout::VMMap;
@@ -24,38 +25,37 @@ lazy_static! {
     // 2. These mmappers are possibly global across multiple MMTk instances, as they manage the
     //    entire address space.
     // TODO: We should refactor this when we know more about how multiple MMTK instances work.
+
+    /// A global VMMap that manages the mapping of spaces to virtual memory ranges.
     pub static ref VM_MAP: VMMap = VMMap::new();
+
+    /// A global Mmapper for mmaping and protection of virtual memory.
     pub static ref MMAPPER: Mmapper = Mmapper::new();
+
+    // A global space function table that allows efficient dispatch space specific code for addresses in our heap.
     pub static ref SFT_MAP: SFTMap<'static> = SFTMap::new();
 }
 
-/// An MMTk instance. MMTk allows mutiple instances to run independently, and each instance gives users a separate heap.
+/// An MMTk instance. MMTk allows multiple instances to run independently, and each instance gives users a separate heap.
 /// *Note that multi-instances is not fully supported yet*
 pub struct MMTK<VM: VMBinding> {
-    pub plan: Box<dyn Plan<VM = VM>>,
-    pub vm_map: &'static VMMap,
-    pub mmapper: &'static Mmapper,
-    pub sftmap: &'static SFTMap<'static>,
-    pub reference_processors: ReferenceProcessors,
-    pub finalizable_processor: Mutex<FinalizableProcessor>,
-    pub options: Arc<UnsafeOptionsWrapper>,
-    pub scheduler: Arc<Scheduler<Self>>,
+    pub(crate) plan: Box<dyn Plan<VM = VM>>,
+    pub(crate) reference_processors: ReferenceProcessors,
+    pub(crate) finalizable_processor: Mutex<FinalizableProcessor>,
+    pub(crate) options: Arc<UnsafeOptionsWrapper>,
+    pub(crate) scheduler: Arc<MMTkScheduler<VM>>,
     #[cfg(feature = "sanity")]
-    pub sanity_checker: Mutex<SanityChecker>,
+    pub(crate) sanity_checker: Mutex<SanityChecker>,
     inside_harness: AtomicBool,
 }
 
 impl<VM: VMBinding> MMTK<VM> {
     pub fn new() -> Self {
-        let scheduler = Scheduler::new();
+        let scheduler = MMTkScheduler::new();
         let options = Arc::new(UnsafeOptionsWrapper::new(Options::default()));
-        let plan =
-            crate::plan::global::create_plan(options.plan, &VM_MAP, &MMAPPER, options.clone());
+        let plan = crate::plan::create_plan(options.plan, &VM_MAP, &MMAPPER, options.clone());
         MMTK {
             plan,
-            vm_map: &VM_MAP,
-            mmapper: &MMAPPER,
-            sftmap: &SFT_MAP,
             reference_processors: ReferenceProcessors::new(),
             finalizable_processor: Mutex::new(FinalizableProcessor::new()),
             options,
@@ -78,6 +78,10 @@ impl<VM: VMBinding> MMTK<VM> {
         self.plan.base().stats.stop_all(self);
         self.inside_harness.store(false, Ordering::SeqCst);
     }
+
+    pub fn get_plan(&self) -> &dyn Plan<VM = VM> {
+        self.plan.as_ref()
+    }
 }
 
 impl<VM: VMBinding> Default for MMTK<VM> {

src/mmtk.rs

@@ -6,6 +6,7 @@ use crate::util::side_metadata::*;
 use crate::util::*;
 use crate::MMTK;
 
+/// BarrierSelector describes which barrier to use.
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub enum BarrierSelector {
     NoBarrier,

src/plan/barriers.rs

@@ -1,3 +1,5 @@
+//! Plan: marksweep (currently using malloc as its freelist allocator)
+
 mod gc_work;
 mod global;
 pub mod mutator;

src/plan/marksweep/mod.rs

@@ -2,35 +2,53 @@
 //!
 //! This module provides various GC plans, each of which implements a GC algorithm.
 //! Generally a plan consists of a few parts:
-//! * A plan type that implements the [`Plan`](crate::plan::global::Plan) trait, which defines
+//! * A plan type that implements the [`Plan`](crate::plan::Plan) trait, which defines
 //!   spaces used in the plan, and their behaviors in GC and page accounting.
 //! * A mutator definition, which describes the mapping between allocators and allocation semantics,
 //!   and the mapping between allocators and spaces. If the plan needs barrier, the barrier definition is
 //!   also included here.
-//! * A constant for [`PlanConstraints`](crate::plan::plan_constraints::PlanConstraints), which defines
+//! * A constant for [`PlanConstraints`](crate::plan::PlanConstraints), which defines
 //!   plan-specific constants.
 //! * Plan-specific [`GCWork`](crate::scheduler::GCWork), which is scheduled during GC. If the plan
-//!   implements a copying GC, a [`CopyContext`](crate::plan::global::CopyContext) also needs to be provided.
+//!   implements a copying GC, a [`CopyContext`](crate::plan::CopyContext) also needs to be provided.
 //!
 //! For more about implementing a plan, it is recommended to read the [MMTk tutorial](/docs/tutorial/Tutorial.md).
 
-pub mod barriers;
-pub mod controller_collector_context;
-pub mod global;
-pub mod mutator_context;
-pub mod plan_constraints;
-pub mod tracelocal;
-pub mod transitive_closure;
-pub use self::global::AllocationSemantics;
-pub use self::global::CopyContext;
-pub use self::global::Plan;
-pub use self::mutator_context::Mutator;
-pub use self::mutator_context::MutatorContext;
-pub use self::plan_constraints::PlanConstraints;
-pub use self::tracelocal::TraceLocal;
-pub use self::transitive_closure::TransitiveClosure;
-
-pub mod gencopy;
-pub mod marksweep;
-pub mod nogc;
-pub mod semispace;
+mod barriers;
+pub use barriers::BarrierSelector;
+
+mod controller_collector_context;
+
+mod global;
+pub(crate) use global::create_mutator;
+pub(crate) use global::create_plan;
+pub use global::AllocationSemantics;
+pub use global::CopyContext;
+pub(crate) use global::GcStatus;
+pub use global::Plan;
+
+mod mutator_context;
+pub use mutator_context::Mutator;
+pub use mutator_context::MutatorContext;
+
+mod plan_constraints;
+pub use plan_constraints::PlanConstraints;
+
+mod tracelocal;
+pub use tracelocal::TraceLocal;
+
+mod transitive_closure;
+pub use transitive_closure::TransitiveClosure;
+
+mod gencopy;
+mod marksweep;
+mod nogc;
+mod semispace;
+
+// Expose plan constraints as public. Though a binding can get them from plan.constraints(),
+// it is possible for performance reasons that they want the constraints as constants.
+
+pub use gencopy::GENCOPY_CONSTRAINTS;
+pub use marksweep::MS_CONSTRAINTS;
+pub use nogc::NOGC_CONSTRAINTS;
+pub use semispace::SS_CONSTRAINTS;

src/plan/mod.rs

@@ -16,6 +16,7 @@ use std::sync::atomic::{AtomicBool, Ordering};
 
 const META_DATA_PAGES_PER_REGION: usize = CARD_META_PAGES_PER_REGION;
 
+/// This type implements a simple copying space.
 pub struct CopySpace<VM: VMBinding> {
     common: CommonSpace<VM>,
     pr: MonotonePageResource<VM>,
@@ -153,6 +154,7 @@ impl<VM: VMBinding> CopySpace<VM> {
         }
     }
 
+    #[allow(dead_code)] // Only used with certain features (such as sanity)
     pub fn protect(&self) {
         if !self.common().contiguous {
             panic!(
@@ -167,6 +169,7 @@ impl<VM: VMBinding> CopySpace<VM> {
         trace!("Protect {:x} {:x}", start, start + extent);
     }
 
+    #[allow(dead_code)] // Only used with certain features (such as sanity)
     pub fn unprotect(&self) {
         if !self.common().contiguous {
             panic!(

src/policy/copyspace.rs

@@ -16,6 +16,10 @@ use crate::util::heap::HeapMeta;
 use crate::util::side_metadata::{SideMetadataContext, SideMetadataSpec};
 use crate::vm::VMBinding;
 
+/// This type implements a simple immortal collection
+/// policy. Under this policy all that is required is for the
+/// "collector" to propagate marks in a liveness trace.  It does not
+/// actually collect.
 pub struct ImmortalSpace<VM: VMBinding> {
     mark_state: u8,
     common: CommonSpace<VM>,

src/policy/immortalspace.rs

@@ -25,6 +25,8 @@ const USE_PRECEEDING_GC_HEADER: bool = true;
 const PRECEEDING_GC_HEADER_WORDS: usize = 1;
 const PRECEEDING_GC_HEADER_BYTES: usize = PRECEEDING_GC_HEADER_WORDS << LOG_BYTES_IN_WORD;
 
+/// This type implements a policy for large objects. Each instance corresponds
+/// to one Treadmill space.
 pub struct LargeObjectSpace<VM: VMBinding> {
     common: CommonSpace<VM>,
     pr: FreeListPageResource<VM>,

src/policy/largeobjectspace.rs

@@ -18,6 +18,12 @@ use crate::vm::*;
 use std::marker::PhantomData;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
+/// This type implements a lock free version of the immortal collection
+/// policy. This is close to the OpenJDK's epsilon GC.
+/// Different from the normal ImmortalSpace, this version should only
+/// be used by NoGC plan, and it now uses the whole heap range.
+// FIXME: It is wrong that the space uses the whole heap range. It has to reserve its own
+// range from HeapMeta, and not clash with other spaces.
 pub struct LockFreeImmortalSpace<VM: VMBinding> {
     #[allow(unused)]
     name: &'static str,
@@ -122,6 +128,7 @@ impl<VM: VMBinding> Space<VM> for LockFreeImmortalSpace<VM> {
 }
 
 impl<VM: VMBinding> LockFreeImmortalSpace<VM> {
+    #[allow(dead_code)] // Only used with certain features.
     pub fn new(
         name: &'static str,
         slow_path_zeroing: bool,

src/policy/lockfreeimmortalspace.rs

@@ -1,3 +1,4 @@
+///! A marksweep space that allocates from malloc.
 mod global;
 pub mod metadata;
 

src/policy/mallocspace/mod.rs

@@ -18,5 +18,3 @@ pub mod immortalspace;
 pub mod largeobjectspace;
 pub mod lockfreeimmortalspace;
 pub mod mallocspace;
-
-pub const NUMBER_OF_POLICIES_REQUIRING_SIDE_METADATA: usize = 0;

src/policy/mod.rs

@@ -186,6 +186,8 @@ impl<'a> SFTMap<'a> {
         }
     }
 
+    // TODO: We should clear a SFT entry when a space releases a chunk.
+    #[allow(dead_code)]
     pub fn clear(&self, chunk_idx: usize) {
         self.set(chunk_idx, &EMPTY_SPACE_SFT);
     }