From 916eb61370abfab12442c7c9550932837b54e520 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Thu, 14 Apr 2022 12:10:08 +1000 Subject: [PATCH 1/6] Add a flag is_gc_space to MallocSpace. A malloc space could be manual malloc space. --- src/plan/marksweep/global.rs | 2 +- src/policy/mallocspace/global.rs | 62 ++++++++++++++++++++++++++++-- src/util/alloc/malloc_allocator.rs | 2 +- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index 1bf8ef0611..506a0f7a30 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -109,7 +109,7 @@ impl MarkSweep { ]); let res = MarkSweep { - ms: MallocSpace::new(global_metadata_specs.clone()), + ms: MallocSpace::new(true, global_metadata_specs.clone()), common: CommonPlan::new( vm_map, mmapper, diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index 136a212ed8..12e478bfbc 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -20,7 +20,7 @@ use crate::{policy::space::Space, util::heap::layout::vm_layout_constants::BYTES use std::marker::PhantomData; #[cfg(debug_assertions)] use std::sync::atomic::AtomicU32; -use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; // only used for debugging use crate::policy::space::*; #[cfg(debug_assertions)] @@ -33,12 +33,32 @@ use std::sync::Mutex; #[cfg(debug_assertions)] const ASSERT_ALLOCATION: bool = false; +/// A malloc space. This space is special in a number of ways: +/// 1. This space does not use a page resource. Instead, this space use malloc to back up its +/// allocation. So MMTk does not manage the mmapping for its data, but MMTk still manages +/// the mmapping for the necessary metadata it needs. +/// 2. This space can be a GC space, for which we use mark sweep to do GC on the space. And +/// it can be a non-GC space, the user will manage its allocation and free through a +/// malloc/free API. In this case, we do not perform any GC work on the space, and it is +/// purely the user's responsibility to manage objects in the space. pub struct MallocSpace { phantom: PhantomData, active_bytes: AtomicUsize, pub chunk_addr_min: AtomicUsize, // XXX: have to use AtomicUsize to represent an Address pub chunk_addr_max: AtomicUsize, metadata: SideMetadataContext, + + /// Do we perform GC on this space? If true, we perform mark sweep on the space. Otherwise, + /// objects are freed manually. + /// To be specific, the differences are: + /// GC | non-GC/manual + /// mark/trace_object yes | no + /// sweep/release yes | no + /// object reference normal | no object reference, any method related with object reference cannot be used + /// set alloc bit in post_alloc | in alloc (post_alloc should not be called) + /// clear alloc bit in sweep | when freed manually + is_gc_space: AtomicBool, + // Mapping between allocated address and its size - this is used to check correctness. // Size will be set to zero when the memory is freed. #[cfg(debug_assertions)] @@ -54,12 +74,16 @@ pub struct MallocSpace { pub work_live_bytes: AtomicUsize, } +const NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE: &str = "Manual malloc space cannot use methods with an ObjectReference argument (there is no object in manual malloc space"; + impl SFT for MallocSpace { fn name(&self) -> &str { self.get_name() } fn is_live(&self, object: ObjectReference) -> bool { + debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + // If this is a GC space, we use mark bit to tell whether it is alive is_marked::(object, Some(Ordering::SeqCst)) } @@ -74,6 +98,7 @@ impl SFT for MallocSpace { // For malloc space, we need to further check the alloc bit. fn is_in_space(&self, object: ObjectReference) -> bool { + debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); is_alloced_by_malloc(object) } @@ -81,6 +106,7 @@ impl SFT for MallocSpace { #[cfg(feature = "is_mmtk_object")] #[inline(always)] fn is_mmtk_object(&self, addr: Address) -> bool { + debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); debug_assert!(!addr.is_zero()); // `addr` cannot be mapped by us. It should be mapped by the malloc library. debug_assert!(!addr.is_mapped()); @@ -88,6 +114,7 @@ impl SFT for MallocSpace { } fn initialize_object_metadata(&self, object: ObjectReference, _alloc: bool) { + debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); trace!("initialize_object_metadata for object {}", object); let page_addr = conversions::page_align_down(object.to_address()); set_page_mark(page_addr); @@ -101,6 +128,7 @@ impl SFT for MallocSpace { object: ObjectReference, _worker: GCWorkerMutRef, ) -> ObjectReference { + debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); let trace = trace.into_mut::(); self.trace_object(trace, object) } @@ -134,6 +162,8 @@ impl Space for MallocSpace { // We have assertions in a debug build. We allow this pattern for the release build. #[allow(clippy::let_and_return)] fn in_space(&self, object: ObjectReference) -> bool { + debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + let ret = is_alloced_by_malloc(object); #[cfg(debug_assertions)] @@ -190,7 +220,7 @@ impl Space for MallocSpace { } impl MallocSpace { - pub fn new(global_side_metadata_specs: Vec) -> Self { + pub fn new(gc: bool, global_side_metadata_specs: Vec) -> Self { MallocSpace { phantom: PhantomData, active_bytes: AtomicUsize::new(0), @@ -204,6 +234,7 @@ impl MallocSpace { *VM::VMObjectModel::LOCAL_MARK_BIT_SPEC, ]), }, + is_gc_space: AtomicBool::new(gc), #[cfg(debug_assertions)] active_mem: Mutex::new(HashMap::new()), #[cfg(debug_assertions)] @@ -215,6 +246,10 @@ impl MallocSpace { } } + fn is_gc_space(&self) -> bool { + self.is_gc_space.load(Ordering::Relaxed) + } + pub fn alloc(&self, tls: VMThread, size: usize, align: usize, offset: isize) -> Address { // TODO: Should refactor this and Space.acquire() if VM::VMActivePlan::global().poll(false, self) { @@ -240,6 +275,11 @@ impl MallocSpace { set_offset_malloc_bit(address); } + if !self.is_gc_space() { + // if this is a non-GC space, we directly set alloc bit for the address + set_alloc_bit(unsafe { address.to_object_reference() }); + } + #[cfg(debug_assertions)] if ASSERT_ALLOCATION { debug_assert!(actual_size != 0); @@ -250,7 +290,19 @@ impl MallocSpace { address } - pub fn free(&self, addr: Address) { + pub fn free_manually(&self, address: Address) { + debug_assert!(!self.is_gc_space(), "we can only manually free objects if it is non-GC malloc space"); + // Free the result + self.free_malloc_result(address); + // This is non-GC malloc space. We assume address === object reference. + unset_alloc_bit(unsafe { address.to_object_reference() }); + } + + /// Frees an address result that we get from maloc, but have ot yet returned to the user. + /// This is used for retrying malloc in the case that malloc may give us some undesired addresses. + /// This will clear offset bit if appriopriate, but this does not clear alloc bit + /// (as the alloc bit should not be set for it yet) + pub fn free_malloc_result(&self, addr: Address) { let offset_malloc_bit = is_offset_malloc(addr); let bytes = get_malloc_usable_size(addr, offset_malloc_bit); self.free_internal(addr, bytes, offset_malloc_bit); @@ -285,6 +337,8 @@ impl MallocSpace { trace: &mut T, object: ObjectReference, ) -> ObjectReference { + debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + if object.is_null() { return object; } @@ -351,6 +405,7 @@ impl MallocSpace { } pub fn sweep_chunk(&self, chunk_start: Address) { + debug_assert!(self.is_gc_space(), "sweep_chunk() should only be called in a GC malloc space"); // Call the relevant sweep function depending on the location of the mark bits match *VM::VMObjectModel::LOCAL_MARK_BIT_SPEC { MetadataSpec::OnSide(local_mark_bit_side_spec) => { @@ -365,6 +420,7 @@ impl MallocSpace { /// Given an object in MallocSpace, return its malloc address, whether it is an offset malloc, and malloc size #[inline(always)] fn get_malloc_addr_size(object: ObjectReference) -> (Address, bool, usize) { + // This method is only used for GC. So we assume we can use object model normally. let obj_start = VM::VMObjectModel::object_start_ref(object); let offset_malloc_bit = is_offset_malloc(obj_start); let bytes = get_malloc_usable_size(obj_start, offset_malloc_bit); diff --git a/src/util/alloc/malloc_allocator.rs b/src/util/alloc/malloc_allocator.rs index 41200889b7..3eb409f880 100644 --- a/src/util/alloc/malloc_allocator.rs +++ b/src/util/alloc/malloc_allocator.rs @@ -53,7 +53,7 @@ impl Allocator for MallocAllocator { } else { // The result passes the check. We free all the cached results, and return the new result. for addr in to_free.iter() { - self.space.free(*addr); + self.space.free_malloc_result(*addr); } return ret; } From c783cf835c45a6c396cf88dd5030a254dcd1abf3 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 19 Apr 2022 09:28:40 +1000 Subject: [PATCH 2/6] WIP: add malloc_space --- Cargo.toml | 3 +- src/memory_manager.rs | 10 +++++ src/plan/global.rs | 39 +++++++++++++++++++ src/plan/marksweep/global.rs | 3 ++ src/plan/mutator_context.rs | 15 +++++++ src/policy/mallocspace/global.rs | 7 ++++ vmbindings/dummyvm/Cargo.toml | 1 + vmbindings/dummyvm/src/api.rs | 6 +++ vmbindings/dummyvm/src/tests/fixtures/mod.rs | 18 +++++++++ .../src/tests/{malloc.rs => malloc_impl.rs} | 0 vmbindings/dummyvm/src/tests/malloc_space.rs | 21 ++++++++++ vmbindings/dummyvm/src/tests/mod.rs | 4 +- 12 files changed, 125 insertions(+), 2 deletions(-) rename vmbindings/dummyvm/src/tests/{malloc.rs => malloc_impl.rs} (100%) create mode 100644 vmbindings/dummyvm/src/tests/malloc_space.rs diff --git a/Cargo.toml b/Cargo.toml index 51cb2cc69a..b3c3f97949 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,7 +58,8 @@ perf_counter = ["pfm"] # spaces vm_space = [] ro_space = [] -code_space = [] +code_space = [] +malloc_space = [] # metadata global_alloc_bit = [] diff --git a/src/memory_manager.rs b/src/memory_manager.rs index 907042828e..c40807465a 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -139,6 +139,16 @@ pub fn post_alloc( mutator.post_alloc(refer, bytes, semantics); } +#[cfg(feature = "malloc_space")] +pub fn free(mmtk: &MMTK, address: Address) { + mmtk.plan.base().malloc_space.free_manually(address) +} + +#[cfg(feature = "malloc_space")] +pub fn malloc_usable_size(mmtk: &MMTK, address: Address) -> usize { + mmtk.plan.base().malloc_space.malloc_usable_size(address) +} + /// Return an AllocatorSelector for the given allocation semantic. This method is provided /// so that VM compilers may call it to help generate allocation fast-path. /// diff --git a/src/plan/global.rs b/src/plan/global.rs index 2c990cd115..2dc8a127a1 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -28,6 +28,8 @@ use crate::util::statistics::stats::Stats; use crate::util::ObjectReference; use crate::util::{VMMutatorThread, VMWorkerThread}; use crate::vm::*; +#[cfg(feature = "malloc_space")] +use crate::policy::mallocspace::MallocSpace; use downcast_rs::Downcast; use enum_map::EnumMap; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -376,13 +378,24 @@ pub struct BasePlan { pub analysis_manager: AnalysisManager, // Spaces in base plan + + /// Code space allows execution. + // TODO: a proper CodeSpace. #[cfg(feature = "code_space")] pub code_space: ImmortalSpace, #[cfg(feature = "code_space")] pub code_lo_space: ImmortalSpace, + + /// Read-only space does not allow write once read-only is enabled. + // TODO: a proper ReadOnlySpace #[cfg(feature = "ro_space")] pub ro_space: ImmortalSpace, + /// Malloc space allows manual malloc/free-alike API. + /// A user allocate into a malloc space with [`AllocationSemantics::Malloc`](crate::plan::AllocationSemantics) + #[cfg(feature = "malloc_space")] + pub malloc_space: MallocSpace, + /// A VM space is a space allocated and populated by the VM. Currently it is used by JikesRVM /// for boot image. /// @@ -478,6 +491,8 @@ impl BasePlan { &mut heap, constraints, ), + #[cfg(feature = "malloc_space")] + malloc_space: MallocSpace::new(false, global_side_metadata_specs.clone()), #[cfg(feature = "vm_space")] vm_space: create_vm_space( vm_map, @@ -532,6 +547,8 @@ impl BasePlan { self.code_lo_space.init(vm_map); #[cfg(feature = "ro_space")] self.ro_space.init(vm_map); + #[cfg(feature = "malloc_space")] + self.malloc_space.init(vm_map); #[cfg(feature = "vm_space")] { self.vm_space.init(vm_map); @@ -588,6 +605,10 @@ impl BasePlan { { pages += self.ro_space.reserved_pages(); } + #[cfg(feature = "malloc_space")] + { + pages += self.malloc_space.reserved_pages(); + } // The VM space may be used as an immutable boot image, in which case, we should not count // it as part of the heap size. @@ -617,6 +638,11 @@ impl BasePlan { return self.ro_space.trace_object(_trace, _object); } + #[cfg(feature = "malloc_space")] + if self.malloc_space.in_space(_object) { + panic!("We cannot trace object in malloc_space"); + } + #[cfg(feature = "vm_space")] if self.vm_space.in_space(_object) { trace!("trace_object: object in boot space"); @@ -632,6 +658,10 @@ impl BasePlan { self.code_lo_space.prepare(); #[cfg(feature = "ro_space")] self.ro_space.prepare(); + #[cfg(feature = "malloc_space")] + { + // We do not need to prepare for malloc space + } #[cfg(feature = "vm_space")] self.vm_space.prepare(); } @@ -643,6 +673,10 @@ impl BasePlan { self.code_lo_space.release(); #[cfg(feature = "ro_space")] self.ro_space.release(); + #[cfg(feature = "malloc_space")] + { + // We do not need to release for malloc space + } #[cfg(feature = "vm_space")] self.vm_space.release(); } @@ -825,6 +859,9 @@ impl BasePlan { #[cfg(feature = "ro_space")] self.ro_space .verify_side_metadata_sanity(side_metadata_sanity_checker); + #[cfg(feature = "malloc_space")] + self.malloc_space + .verify_side_metadata_sanity(side_metadata_sanity_checker); #[cfg(feature = "vm_space")] self.vm_space .verify_side_metadata_sanity(side_metadata_sanity_checker); @@ -957,4 +994,6 @@ pub enum AllocationSemantics { Code = 3, ReadOnly = 4, LargeCode = 5, + /// Malloc and free through MMTk. + Malloc = 6, } diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index 506a0f7a30..c1f61b3af9 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -108,6 +108,9 @@ impl MarkSweep { ACTIVE_CHUNK_METADATA_SPEC, ]); + if cfg!(feature = "malloc_space") { + panic!("We only allow one malloc space in use (marksweep currently uses malloc space and performs marksweep on it"); + } let res = MarkSweep { ms: MallocSpace::new(true, global_metadata_specs.clone()), common: CommonPlan::new( diff --git a/src/plan/mutator_context.rs b/src/plan/mutator_context.rs index 0f7349dc3a..32b4a8519d 100644 --- a/src/plan/mutator_context.rs +++ b/src/plan/mutator_context.rs @@ -225,6 +225,12 @@ pub(crate) fn create_allocator_mapping( reserved.n_bump_pointer += 1; } + #[cfg(feature = "malloc_space")] + { + map[AllocationSemantics::Malloc] = AllocatorSelector::Malloc(reserved.n_malloc); + reserved.n_malloc += 1; + } + // spaces in common plan if include_common_plan { @@ -282,6 +288,15 @@ pub(crate) fn create_space_mapping( reserved.n_bump_pointer += 1; } + #[cfg(feature = "malloc_space")] + { + vec.push(( + AllocatorSelector::Malloc(reserved.n_malloc), + &plan.base().malloc_space, + )); + reserved.n_malloc += 1; + } + // spaces in CommonPlan if include_common_plan { diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index 12e478bfbc..fdd1e42182 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -290,6 +290,7 @@ impl MallocSpace { address } + /// This implements the free() API for the malloc space. pub fn free_manually(&self, address: Address) { debug_assert!(!self.is_gc_space(), "we can only manually free objects if it is non-GC malloc space"); // Free the result @@ -298,6 +299,12 @@ impl MallocSpace { unset_alloc_bit(unsafe { address.to_object_reference() }); } + /// This implements the malloc_usable_size() API for the malloc. + pub fn malloc_usable_size(&self, address: Address) -> usize { + let offset_malloc_bit = is_offset_malloc(address); + get_malloc_usable_size(address, offset_malloc_bit) + } + /// Frees an address result that we get from maloc, but have ot yet returned to the user. /// This is used for retrying malloc in the case that malloc may give us some undesired addresses. /// This will clear offset bit if appriopriate, but this does not clear alloc bit diff --git a/vmbindings/dummyvm/Cargo.toml b/vmbindings/dummyvm/Cargo.toml index d0bea8a8b5..04db42fe70 100644 --- a/vmbindings/dummyvm/Cargo.toml +++ b/vmbindings/dummyvm/Cargo.toml @@ -21,3 +21,4 @@ atomic_refcell = "0.1.7" [features] default = [] is_mmtk_object = ["mmtk/is_mmtk_object"] +malloc_space = ["mmtk/malloc_space"] diff --git a/vmbindings/dummyvm/src/api.rs b/vmbindings/dummyvm/src/api.rs index 3c5aa55a8e..e465f8aad3 100644 --- a/vmbindings/dummyvm/src/api.rs +++ b/vmbindings/dummyvm/src/api.rs @@ -50,6 +50,12 @@ pub extern "C" fn mmtk_post_alloc(mutator: *mut Mutator, refer: ObjectR memory_manager::post_alloc::(unsafe { &mut *mutator }, refer, bytes, semantics) } +#[cfg(feature = "malloc_space")] +#[no_mangle] +pub extern "C" fn mmtk_free(address: Address) { + memory_manager::free::(&SINGLETON, address) +} + #[no_mangle] pub extern "C" fn mmtk_will_never_move(object: ObjectReference) -> bool { !object.is_movable() diff --git a/vmbindings/dummyvm/src/tests/fixtures/mod.rs b/vmbindings/dummyvm/src/tests/fixtures/mod.rs index 2eb249b1d0..d55194a2cc 100644 --- a/vmbindings/dummyvm/src/tests/fixtures/mod.rs +++ b/vmbindings/dummyvm/src/tests/fixtures/mod.rs @@ -3,9 +3,11 @@ use std::sync::Once; use mmtk::AllocationSemantics; use mmtk::util::{ObjectReference, VMThread, VMMutatorThread}; +use mmtk::Mutator; use crate::api::*; use crate::object_model::OBJECT_REF_OFFSET; +use crate::DummyVM; pub trait FixtureContent { fn create() -> Self; @@ -66,3 +68,19 @@ impl FixtureContent for SingleObject { SingleObject { objref } } } + +pub struct MutatorInstance { + pub mutator: *mut Mutator, +} + +impl FixtureContent for MutatorInstance { + fn create() -> Self { + const MB: usize = 1024 * 1024; + // 1MB heap + mmtk_gc_init(MB); + mmtk_initialize_collection(VMThread::UNINITIALIZED); + let mutator = mmtk_bind_mutator(VMMutatorThread(VMThread::UNINITIALIZED)); + + MutatorInstance { mutator } + } +} diff --git a/vmbindings/dummyvm/src/tests/malloc.rs b/vmbindings/dummyvm/src/tests/malloc_impl.rs similarity index 100% rename from vmbindings/dummyvm/src/tests/malloc.rs rename to vmbindings/dummyvm/src/tests/malloc_impl.rs diff --git a/vmbindings/dummyvm/src/tests/malloc_space.rs b/vmbindings/dummyvm/src/tests/malloc_space.rs new file mode 100644 index 0000000000..06bac927be --- /dev/null +++ b/vmbindings/dummyvm/src/tests/malloc_space.rs @@ -0,0 +1,21 @@ +// GITHUB-CI: MMTK_PLAN=all +// GITHUB-CI: FEATURES=malloc_space + +use crate::api::*; +use crate::tests::fixtures::{Fixture, MutatorInstance}; + +use mmtk::AllocationSemantics; + +lazy_static! { + static ref MUTATOR: Fixture = Fixture::new(); +} + +#[test] +pub fn malloc_free() { + MUTATOR.with_fixture(|fixture| { + let res = mmtk_alloc(fixture.mutator, 40, 8, 0, AllocationSemantics::Malloc); + assert!(!res.is_zero()); + assert!(res.is_aligned_to(8)); + mmtk_free(res); + }) +} diff --git a/vmbindings/dummyvm/src/tests/mod.rs b/vmbindings/dummyvm/src/tests/mod.rs index d8f2a202b9..adffeab2d4 100644 --- a/vmbindings/dummyvm/src/tests/mod.rs +++ b/vmbindings/dummyvm/src/tests/mod.rs @@ -11,7 +11,9 @@ mod allocate_without_initialize_collection; mod allocate_with_initialize_collection; mod allocate_with_disable_collection; mod allocate_with_re_enable_collection; -mod malloc; +mod malloc_impl; +#[cfg(feature = "malloc_space")] +mod malloc_space; #[cfg(feature = "is_mmtk_object")] mod conservatism; mod is_in_mmtk_spaces; From d426eed24c4192d07d376417f996318f0daf8305 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 19 Apr 2022 10:51:02 +1000 Subject: [PATCH 3/6] Add more tests --- src/plan/global.rs | 17 ++++++++++- src/policy/mallocspace/global.rs | 13 +++++---- src/policy/mallocspace/mod.rs | 2 +- vmbindings/dummyvm/Cargo.toml | 1 + vmbindings/dummyvm/src/api.rs | 6 ++++ vmbindings/dummyvm/src/tests/malloc_space.rs | 30 ++++++++++++++++++-- 6 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/plan/global.rs b/src/plan/global.rs index 2dc8a127a1..1bf5daaf02 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -988,12 +988,27 @@ use enum_map::Enum; #[repr(i32)] #[derive(Clone, Copy, Debug, Enum, PartialEq, Eq)] pub enum AllocationSemantics { + /// The default allocation semantic. Most allocation should use this semantic. Default = 0, + /// This semantic guarantees the memory won't be reclaimed by GC. Immortal = 1, + /// Large object. Generally a plan defines a constant `max_non_los_default_alloc_bytes`, + /// which specifies the max object size that can be allocated with the Default allocation semantic. + /// An object that is larger than the defined max size needs to be allocated with `Los`. Los = 2, + /// This semantic guarantees the memory has the execution permission. Code = 3, + /// This semantic allows the memory to be made read-only after allocation. + /// This semantic is not implemented yet. ReadOnly = 4, + /// Large object + Code. + // TODO: Do we need this? Can we merge this with code? LargeCode = 5, - /// Malloc and free through MMTk. + /// Malloc and free through MMTk. Note that when this is used, the user should treat the result + /// as if it is returned from malloc(): + /// * The memory needs to be manually freed by the user. + /// * The user should not use any other API other than `free()` and `malloc_usable_size()` for the + /// memory address we return. MMTk may panic if the user casts the memory address into an object + /// reference, and pass the object reference in MMTk's API. Malloc = 6, } diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index fdd1e42182..e7e8297671 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -55,8 +55,8 @@ pub struct MallocSpace { /// mark/trace_object yes | no /// sweep/release yes | no /// object reference normal | no object reference, any method related with object reference cannot be used - /// set alloc bit in post_alloc | in alloc (post_alloc should not be called) - /// clear alloc bit in sweep | when freed manually + /// set alloc bit in post_alloc | in alloc if global_alloc_bit is set + /// clear alloc bit in sweep | when freed manually if global_alloc_bit is set is_gc_space: AtomicBool, // Mapping between allocated address and its size - this is used to check correctness. @@ -275,7 +275,7 @@ impl MallocSpace { set_offset_malloc_bit(address); } - if !self.is_gc_space() { + if !self.is_gc_space() && cfg!(feature = "global_alloc_bit") { // if this is a non-GC space, we directly set alloc bit for the address set_alloc_bit(unsafe { address.to_object_reference() }); } @@ -295,8 +295,11 @@ impl MallocSpace { debug_assert!(!self.is_gc_space(), "we can only manually free objects if it is non-GC malloc space"); // Free the result self.free_malloc_result(address); - // This is non-GC malloc space. We assume address === object reference. - unset_alloc_bit(unsafe { address.to_object_reference() }); + // Unset alloc bit + if cfg!(feature = "global_alloc_bit") { + // This is non-GC malloc space. We assume address === object reference. + unset_alloc_bit(unsafe { address.to_object_reference() }); + } } /// This implements the malloc_usable_size() API for the malloc. diff --git a/src/policy/mallocspace/mod.rs b/src/policy/mallocspace/mod.rs index 07dec884ac..b24fdd5f2e 100644 --- a/src/policy/mallocspace/mod.rs +++ b/src/policy/mallocspace/mod.rs @@ -1,4 +1,4 @@ -///! A marksweep space that allocates from malloc. +///! Malloc space that allocates from malloc. mod global; pub mod metadata; diff --git a/vmbindings/dummyvm/Cargo.toml b/vmbindings/dummyvm/Cargo.toml index 04db42fe70..21b75a5771 100644 --- a/vmbindings/dummyvm/Cargo.toml +++ b/vmbindings/dummyvm/Cargo.toml @@ -22,3 +22,4 @@ atomic_refcell = "0.1.7" default = [] is_mmtk_object = ["mmtk/is_mmtk_object"] malloc_space = ["mmtk/malloc_space"] +nogc_multi_space = ["mmtk/nogc_multi_space"] \ No newline at end of file diff --git a/vmbindings/dummyvm/src/api.rs b/vmbindings/dummyvm/src/api.rs index e465f8aad3..e42f15628f 100644 --- a/vmbindings/dummyvm/src/api.rs +++ b/vmbindings/dummyvm/src/api.rs @@ -56,6 +56,12 @@ pub extern "C" fn mmtk_free(address: Address) { memory_manager::free::(&SINGLETON, address) } +#[cfg(feature = "malloc_space")] +#[no_mangle] +pub extern "C" fn mmtk_malloc_usable_size(address: Address) -> usize { + memory_manager::malloc_usable_size::(&SINGLETON, address) +} + #[no_mangle] pub extern "C" fn mmtk_will_never_move(object: ObjectReference) -> bool { !object.is_movable() diff --git a/vmbindings/dummyvm/src/tests/malloc_space.rs b/vmbindings/dummyvm/src/tests/malloc_space.rs index 06bac927be..b226f5a6f3 100644 --- a/vmbindings/dummyvm/src/tests/malloc_space.rs +++ b/vmbindings/dummyvm/src/tests/malloc_space.rs @@ -1,5 +1,10 @@ -// GITHUB-CI: MMTK_PLAN=all -// GITHUB-CI: FEATURES=malloc_space +// This runs with plan that is not malloc MS +// GITHUB-CI: MMTK_PLAN=NoGC +// GITHUB-CI: MMTK_PLAN=Immix +// GITHUB-CI: MMTK_PLAN=GenImmix +// GITHUB-CI: MMTK_PLAN=GenCopy +// GITHUB-CI: MMTK_PLAN=MarkCompact +// GITHUB-CI: FEATURES=malloc_space,nogc_multi_space use crate::api::*; use crate::tests::fixtures::{Fixture, MutatorInstance}; @@ -19,3 +24,24 @@ pub fn malloc_free() { mmtk_free(res); }) } + +#[test] +pub fn malloc_offset() { + MUTATOR.with_fixture(|fixture| { + let res = mmtk_alloc(fixture.mutator, 40, 8, 4, AllocationSemantics::Malloc); + assert!(!res.is_zero()); + assert!((res + 4usize).is_aligned_to(8)); + mmtk_free(res); + }) +} + +#[test] +pub fn malloc_usable_size() { + MUTATOR.with_fixture(|fixture| { + let res = mmtk_alloc(fixture.mutator, 40, 8, 0, AllocationSemantics::Malloc); + assert!(!res.is_zero()); + let size = mmtk_malloc_usable_size(res); + assert_eq!(size, 40); + mmtk_free(res); + }) +} From e43749d6f6871671f057572d84ad1e058bc0f09a Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 19 Apr 2022 11:06:40 +1000 Subject: [PATCH 4/6] cargo fmt --- src/plan/global.rs | 8 ++--- src/plan/marksweep/global.rs | 2 ++ src/policy/mallocspace/global.rs | 57 +++++++++++++++++++++++++++----- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/plan/global.rs b/src/plan/global.rs index 1bf5daaf02..07bdc1fae9 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -8,6 +8,8 @@ use crate::plan::transitive_closure::TransitiveClosure; use crate::plan::Mutator; use crate::policy::immortalspace::ImmortalSpace; use crate::policy::largeobjectspace::LargeObjectSpace; +#[cfg(feature = "malloc_space")] +use crate::policy::mallocspace::MallocSpace; use crate::policy::space::Space; use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; @@ -28,8 +30,6 @@ use crate::util::statistics::stats::Stats; use crate::util::ObjectReference; use crate::util::{VMMutatorThread, VMWorkerThread}; use crate::vm::*; -#[cfg(feature = "malloc_space")] -use crate::policy::mallocspace::MallocSpace; use downcast_rs::Downcast; use enum_map::EnumMap; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -378,7 +378,6 @@ pub struct BasePlan { pub analysis_manager: AnalysisManager, // Spaces in base plan - /// Code space allows execution. // TODO: a proper CodeSpace. #[cfg(feature = "code_space")] @@ -996,13 +995,12 @@ pub enum AllocationSemantics { /// which specifies the max object size that can be allocated with the Default allocation semantic. /// An object that is larger than the defined max size needs to be allocated with `Los`. Los = 2, - /// This semantic guarantees the memory has the execution permission. + /// This semantic guarantees the memory will not be moved and has the execution permission. Code = 3, /// This semantic allows the memory to be made read-only after allocation. /// This semantic is not implemented yet. ReadOnly = 4, /// Large object + Code. - // TODO: Do we need this? Can we merge this with code? LargeCode = 5, /// Malloc and free through MMTk. Note that when this is used, the user should treat the result /// as if it is returned from malloc(): diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index c1f61b3af9..21cf52f405 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -109,8 +109,10 @@ impl MarkSweep { ]); if cfg!(feature = "malloc_space") { + // We can remove this check once we no longer use malloc marksweep. panic!("We only allow one malloc space in use (marksweep currently uses malloc space and performs marksweep on it"); } + let res = MarkSweep { ms: MallocSpace::new(true, global_metadata_specs.clone()), common: CommonPlan::new( diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index e7e8297671..3b6e266fb4 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -41,6 +41,11 @@ const ASSERT_ALLOCATION: bool = false; /// it can be a non-GC space, the user will manage its allocation and free through a /// malloc/free API. In this case, we do not perform any GC work on the space, and it is /// purely the user's responsibility to manage objects in the space. +/// 3. We do not allow more than one MallocSpace in a plan. MMTk has an assumption that +/// each chunk belongs to one space. However, we cannot control the behavior of the underlying +/// malloc, and two malloc spaces may break the assumption. This should not be a big issue +/// as we would expect only one malloc space for `AllocationSemantics::Malloc`, and in the long +/// term, we will not use `MallocSpace` for GC. pub struct MallocSpace { phantom: PhantomData, active_bytes: AtomicUsize, @@ -82,7 +87,11 @@ impl SFT for MallocSpace { } fn is_live(&self, object: ObjectReference) -> bool { - debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + debug_assert!( + self.is_gc_space(), + "{}", + NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE + ); // If this is a GC space, we use mark bit to tell whether it is alive is_marked::(object, Some(Ordering::SeqCst)) } @@ -98,7 +107,11 @@ impl SFT for MallocSpace { // For malloc space, we need to further check the alloc bit. fn is_in_space(&self, object: ObjectReference) -> bool { - debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + debug_assert!( + self.is_gc_space(), + "{}", + NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE + ); is_alloced_by_malloc(object) } @@ -106,7 +119,11 @@ impl SFT for MallocSpace { #[cfg(feature = "is_mmtk_object")] #[inline(always)] fn is_mmtk_object(&self, addr: Address) -> bool { - debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + debug_assert!( + self.is_gc_space(), + "{}", + NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE + ); debug_assert!(!addr.is_zero()); // `addr` cannot be mapped by us. It should be mapped by the malloc library. debug_assert!(!addr.is_mapped()); @@ -114,7 +131,11 @@ impl SFT for MallocSpace { } fn initialize_object_metadata(&self, object: ObjectReference, _alloc: bool) { - debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + debug_assert!( + self.is_gc_space(), + "{}", + NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE + ); trace!("initialize_object_metadata for object {}", object); let page_addr = conversions::page_align_down(object.to_address()); set_page_mark(page_addr); @@ -128,7 +149,11 @@ impl SFT for MallocSpace { object: ObjectReference, _worker: GCWorkerMutRef, ) -> ObjectReference { - debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + debug_assert!( + self.is_gc_space(), + "{}", + NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE + ); let trace = trace.into_mut::(); self.trace_object(trace, object) } @@ -162,7 +187,11 @@ impl Space for MallocSpace { // We have assertions in a debug build. We allow this pattern for the release build. #[allow(clippy::let_and_return)] fn in_space(&self, object: ObjectReference) -> bool { - debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + debug_assert!( + self.is_gc_space(), + "{}", + NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE + ); let ret = is_alloced_by_malloc(object); @@ -292,7 +321,10 @@ impl MallocSpace { /// This implements the free() API for the malloc space. pub fn free_manually(&self, address: Address) { - debug_assert!(!self.is_gc_space(), "we can only manually free objects if it is non-GC malloc space"); + debug_assert!( + !self.is_gc_space(), + "we can only manually free objects if it is non-GC malloc space" + ); // Free the result self.free_malloc_result(address); // Unset alloc bit @@ -347,7 +379,11 @@ impl MallocSpace { trace: &mut T, object: ObjectReference, ) -> ObjectReference { - debug_assert!(self.is_gc_space(), "{}", NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE); + debug_assert!( + self.is_gc_space(), + "{}", + NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE + ); if object.is_null() { return object; @@ -415,7 +451,10 @@ impl MallocSpace { } pub fn sweep_chunk(&self, chunk_start: Address) { - debug_assert!(self.is_gc_space(), "sweep_chunk() should only be called in a GC malloc space"); + debug_assert!( + self.is_gc_space(), + "sweep_chunk() should only be called in a GC malloc space" + ); // Call the relevant sweep function depending on the location of the mark bits match *VM::VMObjectModel::LOCAL_MARK_BIT_SPEC { MetadataSpec::OnSide(local_mark_bit_side_spec) => { From 460e1222f7ca42ef578e90f48d889df84145577e Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 19 Apr 2022 11:26:13 +1000 Subject: [PATCH 5/6] Minor cleanup --- src/policy/mallocspace/global.rs | 3 ++- vmbindings/dummyvm/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index 3b6e266fb4..2e7d756e75 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -79,7 +79,8 @@ pub struct MallocSpace { pub work_live_bytes: AtomicUsize, } -const NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE: &str = "Manual malloc space cannot use methods with an ObjectReference argument (there is no object in manual malloc space"; +const NON_GC_MALLOC_SPACE_CANNOT_USE_OBJECTREFERENCE: &str = + "Manual malloc space cannot use methods with an ObjectReference argument"; impl SFT for MallocSpace { fn name(&self) -> &str { diff --git a/vmbindings/dummyvm/Cargo.toml b/vmbindings/dummyvm/Cargo.toml index 21b75a5771..8e4c075fee 100644 --- a/vmbindings/dummyvm/Cargo.toml +++ b/vmbindings/dummyvm/Cargo.toml @@ -22,4 +22,4 @@ atomic_refcell = "0.1.7" default = [] is_mmtk_object = ["mmtk/is_mmtk_object"] malloc_space = ["mmtk/malloc_space"] -nogc_multi_space = ["mmtk/nogc_multi_space"] \ No newline at end of file +nogc_multi_space = ["mmtk/nogc_multi_space"] From e50eef76de853bd7f930f8305a9764f728311f35 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 19 Apr 2022 12:00:40 +1000 Subject: [PATCH 6/6] Fix test --- vmbindings/dummyvm/src/tests/malloc_space.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/vmbindings/dummyvm/src/tests/malloc_space.rs b/vmbindings/dummyvm/src/tests/malloc_space.rs index b226f5a6f3..6325be4eb4 100644 --- a/vmbindings/dummyvm/src/tests/malloc_space.rs +++ b/vmbindings/dummyvm/src/tests/malloc_space.rs @@ -15,12 +15,17 @@ lazy_static! { static ref MUTATOR: Fixture = Fixture::new(); } +const SIZE: usize = 40; +const NO_OFFSET: isize = 0; +const ALIGN: usize = 8; +const OFFSET: isize = 4; + #[test] pub fn malloc_free() { MUTATOR.with_fixture(|fixture| { - let res = mmtk_alloc(fixture.mutator, 40, 8, 0, AllocationSemantics::Malloc); + let res = mmtk_alloc(fixture.mutator, SIZE, ALIGN, NO_OFFSET, AllocationSemantics::Malloc); assert!(!res.is_zero()); - assert!(res.is_aligned_to(8)); + assert!(res.is_aligned_to(ALIGN)); mmtk_free(res); }) } @@ -28,9 +33,9 @@ pub fn malloc_free() { #[test] pub fn malloc_offset() { MUTATOR.with_fixture(|fixture| { - let res = mmtk_alloc(fixture.mutator, 40, 8, 4, AllocationSemantics::Malloc); + let res = mmtk_alloc(fixture.mutator, SIZE, ALIGN, OFFSET, AllocationSemantics::Malloc); assert!(!res.is_zero()); - assert!((res + 4usize).is_aligned_to(8)); + assert!((res + OFFSET).is_aligned_to(ALIGN)); mmtk_free(res); }) } @@ -38,10 +43,10 @@ pub fn malloc_offset() { #[test] pub fn malloc_usable_size() { MUTATOR.with_fixture(|fixture| { - let res = mmtk_alloc(fixture.mutator, 40, 8, 0, AllocationSemantics::Malloc); + let res = mmtk_alloc(fixture.mutator, SIZE, ALIGN, NO_OFFSET, AllocationSemantics::Malloc); assert!(!res.is_zero()); let size = mmtk_malloc_usable_size(res); - assert_eq!(size, 40); + assert!(size >= SIZE); mmtk_free(res); }) }