From 4c7ced370984eaccf72283560989b8ce30e41522 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 17 May 2022 14:11:23 +1000 Subject: [PATCH 1/7] Refactoring to the util::malloc module. Add malloc APIs. --- Cargo.toml | 5 +- src/memory_manager.rs | 103 ++++++++++++++++++ src/plan/global.rs | 24 +++- src/policy/mallocspace/global.rs | 3 +- src/util/malloc/library.rs | 61 +++++++++++ .../{malloc.rs => malloc/malloc_ms_util.rs} | 36 +----- src/util/malloc/mod.rs | 75 +++++++++++++ 7 files changed, 270 insertions(+), 37 deletions(-) create mode 100644 src/util/malloc/library.rs rename src/util/{malloc.rs => malloc/malloc_ms_util.rs} (75%) create mode 100644 src/util/malloc/mod.rs diff --git a/Cargo.toml b/Cargo.toml index 51cb2cc69a..101a85519e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ enum_derive = "0.1" libc = "0.2" jemalloc-sys = {version = "0.3.2", features = ["disable_initial_exec_tls"], optional = true } mimalloc-sys = {version = "0.1.6", optional = true } -hoard-sys = {version = "0.1.1", optional = true } +hoard-sys = {version = "0.1.1", optional = true, path = "/home/yilin/Code/hoard-sys" } lazy_static = "1.1" log = {version = "0.4", features = ["max_level_trace", "release_max_level_off"] } crossbeam-deque = "0.6" @@ -87,6 +87,9 @@ nogc_multi_space = [] # To collect statistics for each GC work packet. Enabling this may introduce a small overhead (several percentage slowdown on benchmark time). work_packet_stats = [] +# Count the malloc'd memory into the heap size +malloc_counted_size = [] + # Do not modify the following line - ci-common.sh matches it # -- Mutally exclusive features -- # Only one feature from each group can be provided. Otherwise build will fail. diff --git a/src/memory_manager.rs b/src/memory_manager.rs index 42831f725e..a33ecbcfb3 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -152,6 +152,109 @@ pub fn get_allocator_mapping( mmtk.plan.get_allocator_mapping()[semantics] } +/// The standard malloc. MMTk either uses its own allocator, or forward the call to a +/// library malloc. +#[cfg(not(feature = "malloc_counted_size"))] +pub fn mmtk_malloc( + size: usize, +) -> Address { + crate::util::malloc::malloc(size) +} + +/// The standard malloc except that with the feature `malloc_counted_size`, MMTk will count the allocated memory into its heap size. +/// Thus the method requires a reference to an MMTk instance. MMTk either uses its own allocator, or forward the call to a +/// library malloc. +#[cfg(feature = "malloc_counted_size")] +pub fn mmtk_malloc( + mmtk: &MMTK, + size: usize, +) -> Address { + let res = crate::util::malloc::malloc(size); + if !res.is_zero() { + mmtk.plan.base().increase_malloc_bytes_by(size); + } + return res; +} + +/// The standard calloc. +#[cfg(not(feature = "malloc_counted_size"))] +pub fn mmtk_calloc( + num: usize, + size: usize, +) -> Address { + crate::util::malloc::calloc(num, size) +} + +/// The standard calloc except that with the feature `malloc_counted_size`, MMTk will count the allocated memory into its heap size. +/// Thus the method requires a reference to an MMTk instance. +#[cfg(feature = "malloc_counted_size")] +pub fn mmtk_calloc( + mmtk: &MMTK, + num: usize, + size: usize, +) -> Address { + let res = crate::util::malloc::calloc(num, size); + if !res.is_zero() { + mmtk.plan.base().increase_malloc_bytes_by(num * size); + } + return res; +} + +/// The standard realloc. +#[cfg(not(feature = "malloc_counted_size"))] +pub fn mmtk_realloc( + addr: Address, + size: usize +) -> Address { + crate::util::malloc::realloc(addr, size) +} + +/// The standard realloc except that with the feature `malloc_counted_size`, MMTk will count the allocated memory into its heap size. +/// Thus the method requires a reference to an MMTk instance, and the size of the existing memory that will be reallocated. +/// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. +#[cfg(feature = "malloc_counted_size")] +pub fn mmtk_realloc_with_old_size( + mmtk: &MMTK, + addr: Address, + size: usize, + old_size: usize +) -> Address { + let res = crate::util::malloc::realloc(addr, size); + + if !addr.is_zero() { + mmtk.plan.base().decrease_malloc_bytes_by(old_size); + } + if size != 0 && !res.is_zero() { + mmtk.plan.base().increase_malloc_bytes_by(size); + } + + return res; +} + +/// The standard free. +/// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. +#[cfg(not(feature = "malloc_counted_size"))] +pub fn mmtk_free( + addr: Address +) { + crate::util::malloc::free(addr) +} + +/// The standard free except that with the feature `malloc_counted_size`, MMTk will count the allocated memory into its heap size. +/// Thus the method requires a reference to an MMTk instance, and the size of the memory to free. +/// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. +#[cfg(feature = "malloc_counted_size")] +pub fn mmtk_free_with_size( + mmtk: &MMTK, + addr: Address, + old_size: usize +) { + crate::util::malloc::free(addr); + if !addr.is_zero() { + mmtk.plan.base().decrease_malloc_bytes_by(old_size); + } +} + /// Run the main loop for the GC controller thread. This method does not return. /// /// Arguments: diff --git a/src/plan/global.rs b/src/plan/global.rs index 2c990cd115..aa1a7e8c03 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -369,9 +369,12 @@ pub struct BasePlan { /// Have we scanned all the stacks? stacks_prepared: AtomicBool, pub mutator_iterator_lock: Mutex<()>, - // A counter that keeps tracks of the number of bytes allocated since last stress test + /// A counter that keeps tracks of the number of bytes allocated since last stress test allocation_bytes: AtomicUsize, - // Wrapper around analysis counters + /// A counteer that keeps tracks of the number of bytes allocated by malloc + #[cfg(feature = "malloc_counted_size")] + malloc_bytes: AtomicUsize, + /// Wrapper around analysis counters #[cfg(feature = "analysis")] pub analysis_manager: AnalysisManager, @@ -511,6 +514,8 @@ impl BasePlan { scanned_stacks: AtomicUsize::new(0), mutator_iterator_lock: Mutex::new(()), allocation_bytes: AtomicUsize::new(0), + #[cfg(feature = "malloc_counted_size")] + malloc_bytes: AtomicUsize::new(0), #[cfg(feature = "analysis")] analysis_manager, } @@ -589,6 +594,12 @@ impl BasePlan { pages += self.ro_space.reserved_pages(); } + // If we need to count malloc'd size as part of our heap, we add it here. + #[cfg(feature = "malloc_counted_size")] + { + pages += crate::util::conversions::bytes_to_pages_up(self.malloc_bytes.load(Ordering::SeqCst)); + } + // 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. pages @@ -829,6 +840,15 @@ impl BasePlan { self.vm_space .verify_side_metadata_sanity(side_metadata_sanity_checker); } + + #[cfg(feature = "malloc_counted_size")] + pub(crate) fn increase_malloc_bytes_by(&self, size: usize) { + self.malloc_bytes.fetch_add(size, Ordering::SeqCst); + } + #[cfg(feature = "malloc_counted_size")] + pub(crate) fn decrease_malloc_bytes_by(&self, size: usize) { + self.malloc_bytes.fetch_sub(size, Ordering::SeqCst); + } } /** diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index 136a212ed8..d61d71e121 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -5,7 +5,8 @@ use crate::policy::space::SFT; use crate::util::constants::BYTES_IN_PAGE; use crate::util::heap::layout::heap_layout::VMMap; use crate::util::heap::PageResource; -use crate::util::malloc::*; +use crate::util::malloc::malloc_ms_util::*; +use crate::util::malloc::library::*; use crate::util::metadata::side_metadata::{ bzero_metadata, SideMetadataContext, SideMetadataSanity, SideMetadataSpec, }; diff --git a/src/util/malloc/library.rs b/src/util/malloc/library.rs new file mode 100644 index 0000000000..5022097984 --- /dev/null +++ b/src/util/malloc/library.rs @@ -0,0 +1,61 @@ +// Export one of the malloc libraries. + +#[cfg(feature = "malloc_jemalloc")] +pub use self::jemalloc::*; +#[cfg(feature = "malloc_mimalloc")] +pub use self::mimalloc::*; +#[cfg(feature = "malloc_hoard")] +pub use self::hoard::*; +#[cfg(not(any( + feature = "malloc_jemalloc", + feature = "malloc_mimalloc", + feature = "malloc_hoard", +)))] +pub use self::libc_malloc::*; + +// Different malloc libraries + +#[cfg(feature = "malloc_jemalloc")] +mod jemalloc { + // ANSI C + pub use jemalloc_sys::{malloc, calloc, free, realloc}; + // Posix + pub use jemalloc_sys::{malloc_usable_size, posix_memalign}; +} + +#[cfg(feature = "malloc_mimalloc")] +mod mimalloc { + // ANSI C + pub use mimalloc_sys::{ + mi_malloc as malloc, + mi_calloc as calloc, + mi_free as free, + mi_realloc as realloc, + }; + // Posix + pub use mimalloc_sys::{ + mi_posix_memalign as posix_memalign, + mi_malloc_usable_size as malloc_usable_size + }; +} + +#[cfg(feature = "malloc_hoard")] +mod hoard { + // ANSI C + pub use hoard_sys::{malloc, calloc, realloc, free}; + // Posix + pub use hoard_sys::{malloc_usable_size, posix_memalign}; +} + +/// If no malloc lib is specified, use the libc implementation +#[cfg(not(any( + feature = "malloc_jemalloc", + feature = "malloc_mimalloc", + feature = "malloc_hoard", +)))] +mod libc_malloc { + // ANSI C + pub use libc::{malloc, realloc, calloc, free}; + // Posix + pub use libc::{malloc_usable_size, posix_memalign}; +} diff --git a/src/util/malloc.rs b/src/util/malloc/malloc_ms_util.rs similarity index 75% rename from src/util/malloc.rs rename to src/util/malloc/malloc_ms_util.rs index 43b307f69c..482d2f9fd9 100644 --- a/src/util/malloc.rs +++ b/src/util/malloc/malloc_ms_util.rs @@ -1,27 +1,10 @@ use crate::util::constants::BYTES_IN_ADDRESS; use crate::util::Address; use crate::vm::VMBinding; -#[cfg(feature = "malloc_jemalloc")] -pub use jemalloc_sys::{calloc, free, malloc_usable_size, posix_memalign}; +use crate::util::malloc::library::*; -#[cfg(feature = "malloc_mimalloc")] -pub use mimalloc_sys::{ - mi_calloc as calloc, mi_calloc_aligned, mi_free as free, - mi_malloc_usable_size as malloc_usable_size, -}; - -#[cfg(feature = "malloc_hoard")] -pub use hoard_sys::{calloc, free, malloc_usable_size}; - -#[cfg(not(any( - feature = "malloc_jemalloc", - feature = "malloc_mimalloc", - feature = "malloc_hoard", -)))] -pub use libc::{calloc, free, malloc_usable_size, posix_memalign}; - -#[cfg(not(any(feature = "malloc_mimalloc", feature = "malloc_hoard",)))] -fn align_alloc(size: usize, align: usize) -> Address { +/// Allocate with alignment. This also guarantees the memory is zero initialized. +pub fn align_alloc(size: usize, align: usize) -> Address { let mut ptr = std::ptr::null_mut::(); let ptr_ptr = std::ptr::addr_of_mut!(ptr); let result = unsafe { posix_memalign(ptr_ptr, align, size) }; @@ -33,19 +16,6 @@ fn align_alloc(size: usize, align: usize) -> Address { address } -#[cfg(feature = "malloc_mimalloc")] -fn align_alloc(size: usize, align: usize) -> Address { - let raw = unsafe { mi_calloc_aligned(1, size, align) }; - Address::from_mut_ptr(raw) -} - -// hoard_sys does not provide align_alloc, -// we have to do it ourselves -#[cfg(feature = "malloc_hoard")] -fn align_alloc(size: usize, align: usize) -> Address { - align_offset_alloc::(size, align, 0) -} - // Beside returning the allocation result, // this will store the malloc result at (result - BYTES_IN_ADDRESS) fn align_offset_alloc(size: usize, align: usize, offset: isize) -> Address { diff --git a/src/util/malloc/mod.rs b/src/util/malloc/mod.rs new file mode 100644 index 0000000000..05e4fa9994 --- /dev/null +++ b/src/util/malloc/mod.rs @@ -0,0 +1,75 @@ +pub(crate) mod malloc_ms_util; +pub(crate) mod library; + +use crate::MMTK; +use crate::vm::VMBinding; +use crate::util::Address; + +#[inline(always)] +pub fn malloc(size: usize) -> Address { + Address::from_mut_ptr(unsafe { self::library::malloc(size) }) +} + +#[inline(always)] +pub fn calloc(num: usize, size: usize) -> Address { + Address::from_mut_ptr(unsafe { self::library::calloc(num, size) }) +} + +#[inline(always)] +pub fn realloc(addr: Address, size: usize) -> Address { + Address::from_mut_ptr(unsafe { self::library::realloc(addr.to_mut_ptr(), size) }) +} + +#[inline(always)] +pub fn free(addr: Address) { + unsafe { self::library::free(addr.to_mut_ptr()) } +} + +impl MMTK { + #[inline(always)] + pub fn malloc(&self, size: usize) -> Address { + #[cfg(feature = "malloc_counted_size")] + self.plan.base().increase_malloc_bytes_by(size); + + Address::from_mut_ptr(unsafe { self::library::malloc(size) }) + } + + #[inline(always)] + pub fn calloc(&self, num: usize, size: usize) -> Address { + #[cfg(feature = "malloc_counted_size")] + self.plan.base().increase_malloc_bytes_by(num * size); + + Address::from_mut_ptr(unsafe { self::library::calloc(num, size) }) + } +} + +#[cfg(feature = "malloc_counted_size")] +impl MMTK { + #[inline(always)] + pub fn realloc_with_old_size(&self, addr: Address, size: usize, old_size: usize) -> Address { + let base_plan = self.plan.base(); + base_plan.decrease_malloc_bytes_by(old_size); + base_plan.increase_malloc_bytes_by(size); + + Address::from_mut_ptr(unsafe { self::library::realloc(addr.to_mut_ptr(), size) }) + } + + #[inline(always)] + pub fn free_with_size(&self, addr: Address, size: usize) { + self.plan.base().decrease_malloc_bytes_by(size); + unsafe { self::library::free(addr.to_mut_ptr()) } + } +} + +#[cfg(not(feature = "malloc_counted_size"))] +impl MMTK { + #[inline(always)] + pub fn realloc(&self, addr: Address, size: usize) -> Address { + Address::from_mut_ptr(unsafe { self::library::realloc(addr.to_mut_ptr(), size) }) + } + + #[inline(always)] + pub fn free(&self, addr: Address) { + unsafe { self::library::free(addr.to_mut_ptr()) } + } +} From e2410d2d14d34abd80604c311c6323b290e779b1 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 17 May 2022 15:48:53 +1000 Subject: [PATCH 2/7] Add tests --- src/memory_manager.rs | 37 ++------ src/plan/global.rs | 8 +- src/policy/mallocspace/global.rs | 1 - src/util/malloc/library.rs | 22 ++--- src/util/malloc/malloc_ms_util.rs | 8 +- src/util/malloc/mod.rs | 6 +- vmbindings/dummyvm/Cargo.toml | 1 + vmbindings/dummyvm/src/api.rs | 44 ++++++++++ vmbindings/dummyvm/src/tests/fixtures/mod.rs | 19 +++++ vmbindings/dummyvm/src/tests/malloc.rs | 36 -------- vmbindings/dummyvm/src/tests/malloc_api.rs | 24 ++++++ .../dummyvm/src/tests/malloc_counted.rs | 84 +++++++++++++++++++ vmbindings/dummyvm/src/tests/malloc_ms.rs | 36 ++++++++ vmbindings/dummyvm/src/tests/mod.rs | 6 +- 14 files changed, 245 insertions(+), 87 deletions(-) delete mode 100644 vmbindings/dummyvm/src/tests/malloc.rs create mode 100644 vmbindings/dummyvm/src/tests/malloc_api.rs create mode 100644 vmbindings/dummyvm/src/tests/malloc_counted.rs create mode 100644 vmbindings/dummyvm/src/tests/malloc_ms.rs diff --git a/src/memory_manager.rs b/src/memory_manager.rs index a33ecbcfb3..5c0b1689cf 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -155,9 +155,7 @@ pub fn get_allocator_mapping( /// The standard malloc. MMTk either uses its own allocator, or forward the call to a /// library malloc. #[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_malloc( - size: usize, -) -> Address { +pub fn mmtk_malloc(size: usize) -> Address { crate::util::malloc::malloc(size) } @@ -165,10 +163,7 @@ pub fn mmtk_malloc( /// Thus the method requires a reference to an MMTk instance. MMTk either uses its own allocator, or forward the call to a /// library malloc. #[cfg(feature = "malloc_counted_size")] -pub fn mmtk_malloc( - mmtk: &MMTK, - size: usize, -) -> Address { +pub fn mmtk_malloc(mmtk: &MMTK, size: usize) -> Address { let res = crate::util::malloc::malloc(size); if !res.is_zero() { mmtk.plan.base().increase_malloc_bytes_by(size); @@ -178,21 +173,14 @@ pub fn mmtk_malloc( /// The standard calloc. #[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_calloc( - num: usize, - size: usize, -) -> Address { +pub fn mmtk_calloc(num: usize, size: usize) -> Address { crate::util::malloc::calloc(num, size) } /// The standard calloc except that with the feature `malloc_counted_size`, MMTk will count the allocated memory into its heap size. /// Thus the method requires a reference to an MMTk instance. #[cfg(feature = "malloc_counted_size")] -pub fn mmtk_calloc( - mmtk: &MMTK, - num: usize, - size: usize, -) -> Address { +pub fn mmtk_calloc(mmtk: &MMTK, num: usize, size: usize) -> Address { let res = crate::util::malloc::calloc(num, size); if !res.is_zero() { mmtk.plan.base().increase_malloc_bytes_by(num * size); @@ -202,10 +190,7 @@ pub fn mmtk_calloc( /// The standard realloc. #[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_realloc( - addr: Address, - size: usize -) -> Address { +pub fn mmtk_realloc(addr: Address, size: usize) -> Address { crate::util::malloc::realloc(addr, size) } @@ -217,7 +202,7 @@ pub fn mmtk_realloc_with_old_size( mmtk: &MMTK, addr: Address, size: usize, - old_size: usize + old_size: usize, ) -> Address { let res = crate::util::malloc::realloc(addr, size); @@ -234,9 +219,7 @@ pub fn mmtk_realloc_with_old_size( /// The standard free. /// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. #[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_free( - addr: Address -) { +pub fn mmtk_free(addr: Address) { crate::util::malloc::free(addr) } @@ -244,11 +227,7 @@ pub fn mmtk_free( /// Thus the method requires a reference to an MMTk instance, and the size of the memory to free. /// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. #[cfg(feature = "malloc_counted_size")] -pub fn mmtk_free_with_size( - mmtk: &MMTK, - addr: Address, - old_size: usize -) { +pub fn mmtk_free_with_size(mmtk: &MMTK, addr: Address, old_size: usize) { crate::util::malloc::free(addr); if !addr.is_zero() { mmtk.plan.base().decrease_malloc_bytes_by(old_size); diff --git a/src/plan/global.rs b/src/plan/global.rs index aa1a7e8c03..694945844e 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -597,7 +597,9 @@ impl BasePlan { // If we need to count malloc'd size as part of our heap, we add it here. #[cfg(feature = "malloc_counted_size")] { - pages += crate::util::conversions::bytes_to_pages_up(self.malloc_bytes.load(Ordering::SeqCst)); + pages += crate::util::conversions::bytes_to_pages_up( + self.malloc_bytes.load(Ordering::SeqCst), + ); } // The VM space may be used as an immutable boot image, in which case, we should not count @@ -849,6 +851,10 @@ impl BasePlan { pub(crate) fn decrease_malloc_bytes_by(&self, size: usize) { self.malloc_bytes.fetch_sub(size, Ordering::SeqCst); } + #[cfg(feature = "malloc_counted_size")] + pub fn get_malloc_bytes(&self) -> usize { + self.malloc_bytes.load(Ordering::SeqCst) + } } /** diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index d61d71e121..0c4bf884e3 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -6,7 +6,6 @@ use crate::util::constants::BYTES_IN_PAGE; use crate::util::heap::layout::heap_layout::VMMap; use crate::util::heap::PageResource; use crate::util::malloc::malloc_ms_util::*; -use crate::util::malloc::library::*; use crate::util::metadata::side_metadata::{ bzero_metadata, SideMetadataContext, SideMetadataSanity, SideMetadataSpec, }; diff --git a/src/util/malloc/library.rs b/src/util/malloc/library.rs index 5022097984..c1aaaf3a8c 100644 --- a/src/util/malloc/library.rs +++ b/src/util/malloc/library.rs @@ -1,24 +1,24 @@ // Export one of the malloc libraries. -#[cfg(feature = "malloc_jemalloc")] -pub use self::jemalloc::*; -#[cfg(feature = "malloc_mimalloc")] -pub use self::mimalloc::*; #[cfg(feature = "malloc_hoard")] pub use self::hoard::*; +#[cfg(feature = "malloc_jemalloc")] +pub use self::jemalloc::*; #[cfg(not(any( feature = "malloc_jemalloc", feature = "malloc_mimalloc", feature = "malloc_hoard", )))] pub use self::libc_malloc::*; +#[cfg(feature = "malloc_mimalloc")] +pub use self::mimalloc::*; // Different malloc libraries #[cfg(feature = "malloc_jemalloc")] mod jemalloc { // ANSI C - pub use jemalloc_sys::{malloc, calloc, free, realloc}; + pub use jemalloc_sys::{calloc, free, malloc, realloc}; // Posix pub use jemalloc_sys::{malloc_usable_size, posix_memalign}; } @@ -27,22 +27,18 @@ mod jemalloc { mod mimalloc { // ANSI C pub use mimalloc_sys::{ - mi_malloc as malloc, - mi_calloc as calloc, - mi_free as free, - mi_realloc as realloc, + mi_calloc as calloc, mi_free as free, mi_malloc as malloc, mi_realloc as realloc, }; // Posix pub use mimalloc_sys::{ - mi_posix_memalign as posix_memalign, - mi_malloc_usable_size as malloc_usable_size + mi_malloc_usable_size as malloc_usable_size, mi_posix_memalign as posix_memalign, }; } #[cfg(feature = "malloc_hoard")] mod hoard { // ANSI C - pub use hoard_sys::{malloc, calloc, realloc, free}; + pub use hoard_sys::{calloc, free, malloc, realloc}; // Posix pub use hoard_sys::{malloc_usable_size, posix_memalign}; } @@ -55,7 +51,7 @@ mod hoard { )))] mod libc_malloc { // ANSI C - pub use libc::{malloc, realloc, calloc, free}; + pub use libc::{calloc, free, malloc, realloc}; // Posix pub use libc::{malloc_usable_size, posix_memalign}; } diff --git a/src/util/malloc/malloc_ms_util.rs b/src/util/malloc/malloc_ms_util.rs index 482d2f9fd9..1f054a1ced 100644 --- a/src/util/malloc/malloc_ms_util.rs +++ b/src/util/malloc/malloc_ms_util.rs @@ -1,7 +1,7 @@ use crate::util::constants::BYTES_IN_ADDRESS; +use crate::util::malloc::library::*; use crate::util::Address; use crate::vm::VMBinding; -use crate::util::malloc::library::*; /// Allocate with alignment. This also guarantees the memory is zero initialized. pub fn align_alloc(size: usize, align: usize) -> Address { @@ -18,7 +18,7 @@ pub fn align_alloc(size: usize, align: usize) -> Address { // Beside returning the allocation result, // this will store the malloc result at (result - BYTES_IN_ADDRESS) -fn align_offset_alloc(size: usize, align: usize, offset: isize) -> Address { +pub fn align_offset_alloc(size: usize, align: usize, offset: isize) -> Address { // we allocate extra `align` bytes here, so we are able to handle offset let actual_size = size + align + BYTES_IN_ADDRESS; let raw = unsafe { calloc(1, actual_size) }; @@ -37,7 +37,7 @@ fn align_offset_alloc(size: usize, align: usize, offset: isize) - result } -fn offset_malloc_usable_size(address: Address) -> usize { +pub fn offset_malloc_usable_size(address: Address) -> usize { let malloc_res_ptr: *mut usize = (address - BYTES_IN_ADDRESS).to_mut_ptr(); let malloc_res = unsafe { *malloc_res_ptr } as *mut libc::c_void; unsafe { malloc_usable_size(malloc_res) } @@ -50,6 +50,8 @@ pub fn offset_free(address: Address) { unsafe { free(malloc_res) }; } +pub use crate::util::malloc::library::free; + /// get malloc usable size of an address /// is_offset_malloc: whether the address is allocated with some offset pub fn get_malloc_usable_size(address: Address, is_offset_malloc: bool) -> usize { diff --git a/src/util/malloc/mod.rs b/src/util/malloc/mod.rs index 05e4fa9994..259f5b8b7d 100644 --- a/src/util/malloc/mod.rs +++ b/src/util/malloc/mod.rs @@ -1,9 +1,9 @@ -pub(crate) mod malloc_ms_util; pub(crate) mod library; +pub mod malloc_ms_util; -use crate::MMTK; -use crate::vm::VMBinding; use crate::util::Address; +use crate::vm::VMBinding; +use crate::MMTK; #[inline(always)] pub fn malloc(size: usize) -> Address { diff --git a/vmbindings/dummyvm/Cargo.toml b/vmbindings/dummyvm/Cargo.toml index d0bea8a8b5..963420d014 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_counted_size = ["mmtk/malloc_counted_size"] diff --git a/vmbindings/dummyvm/src/api.rs b/vmbindings/dummyvm/src/api.rs index 61585ed67d..714472c50f 100644 --- a/vmbindings/dummyvm/src/api.rs +++ b/vmbindings/dummyvm/src/api.rs @@ -167,3 +167,47 @@ pub extern "C" fn mmtk_starting_heap_address() -> Address { pub extern "C" fn mmtk_last_heap_address() -> Address { memory_manager::last_heap_address() } + +#[no_mangle] +#[cfg(feature = "malloc_counted_size")] +pub extern "C" fn mmtk_malloc(size: usize) -> Address { + memory_manager::mmtk_malloc::(&SINGLETON, size) +} +#[no_mangle] +#[cfg(not(feature = "malloc_counted_size"))] +pub extern "C" fn mmtk_malloc(size: usize) -> Address { + memory_manager::mmtk_malloc(size) +} + +#[no_mangle] +#[cfg(feature = "malloc_counted_size")] +pub extern "C" fn mmtk_calloc(num: usize, size: usize) -> Address { + memory_manager::mmtk_calloc::(&SINGLETON, num, size) +} +#[no_mangle] +#[cfg(not(feature = "malloc_counted_size"))] +pub extern "C" fn mmtk_calloc(num: usize, size: usize) -> Address { + memory_manager::mmtk_calloc(num, size) +} + +#[no_mangle] +#[cfg(feature = "malloc_counted_size")] +pub extern "C" fn mmtk_realloc_with_old_size(addr: Address, size: usize, old_size: usize) -> Address { + memory_manager::mmtk_realloc_with_old_size::(&SINGLETON, addr, size, old_size) +} +#[no_mangle] +#[cfg(not(feature = "malloc_counted_size"))] +pub extern "C" fn mmtk_realloc(addr: Address, size: usize) -> Address { + memory_manager::mmtk_realloc(addr, size) +} + +#[no_mangle] +#[cfg(feature = "malloc_counted_size")] +pub extern "C" fn mmtk_free_with_size(addr: Address, old_size: usize) { + memory_manager::mmtk_free_with_size::(&SINGLETON, addr, old_size) +} +#[no_mangle] +#[cfg(not(feature = "malloc_counted_size"))] +pub extern "C" fn mmtk_free(addr: Address) { + memory_manager::mmtk_free(addr) +} diff --git a/vmbindings/dummyvm/src/tests/fixtures/mod.rs b/vmbindings/dummyvm/src/tests/fixtures/mod.rs index 2eb249b1d0..7414833e18 100644 --- a/vmbindings/dummyvm/src/tests/fixtures/mod.rs +++ b/vmbindings/dummyvm/src/tests/fixtures/mod.rs @@ -2,10 +2,12 @@ use atomic_refcell::AtomicRefCell; use std::sync::Once; use mmtk::AllocationSemantics; +use mmtk::MMTK; use mmtk::util::{ObjectReference, VMThread, VMMutatorThread}; use crate::api::*; use crate::object_model::OBJECT_REF_OFFSET; +use crate::DummyVM; pub trait FixtureContent { fn create() -> Self; @@ -66,3 +68,20 @@ impl FixtureContent for SingleObject { SingleObject { objref } } } + +pub struct MMTKSingleton { + pub mmtk: &'static MMTK +} + +impl FixtureContent for MMTKSingleton { + fn create() -> Self { + const MB: usize = 1024 * 1024; + // 1MB heap + mmtk_gc_init(MB); + mmtk_initialize_collection(VMThread::UNINITIALIZED); + + MMTKSingleton { + mmtk: &crate::SINGLETON, + } + } +} diff --git a/vmbindings/dummyvm/src/tests/malloc.rs b/vmbindings/dummyvm/src/tests/malloc.rs deleted file mode 100644 index c7d04cfbad..0000000000 --- a/vmbindings/dummyvm/src/tests/malloc.rs +++ /dev/null @@ -1,36 +0,0 @@ -use mmtk::util::malloc; -use crate::DummyVM; - -#[test] -fn test_malloc() { - let (address1, bool1) = malloc::alloc::(16, 8, 0); - let (address2, bool2) = malloc::alloc::(16, 32, 0); - let (address3, bool3) = malloc::alloc::(16, 8, 4); - let (address4, bool4) = malloc::alloc::(32, 64, 4); - - assert!(address1.is_aligned_to(8)); - assert!(address2.is_aligned_to(32)); - assert!((address3 + 4 as isize).is_aligned_to(8)); - assert!((address4 + 4 as isize).is_aligned_to(64)); - - assert!(!bool1); - #[cfg(feature = "malloc_hoard")] - assert!(bool2); - #[cfg(not(feature = "malloc_hoard"))] - assert!(!bool2); - assert!(bool3); - assert!(bool4); - - assert!(malloc::get_malloc_usable_size(address1, bool1) >= 16); - assert!(malloc::get_malloc_usable_size(address2, bool2) >= 16); - assert!(malloc::get_malloc_usable_size(address3, bool3) >= 16); - assert!(malloc::get_malloc_usable_size(address4, bool4) >= 32); - - unsafe { malloc::free(address1.to_mut_ptr()); } - #[cfg(feature = "malloc_hoard")] - malloc::offset_free(address2); - #[cfg(not(feature = "malloc_hoard"))] - unsafe { malloc::free(address2.to_mut_ptr()); } - malloc::offset_free(address3); - malloc::offset_free(address4); -} diff --git a/vmbindings/dummyvm/src/tests/malloc_api.rs b/vmbindings/dummyvm/src/tests/malloc_api.rs new file mode 100644 index 0000000000..4b31fd8773 --- /dev/null +++ b/vmbindings/dummyvm/src/tests/malloc_api.rs @@ -0,0 +1,24 @@ +use crate::api::*; + +#[test] +pub fn malloc_free() { + let res = mmtk_malloc(8); + assert!(!res.is_zero()); + mmtk_free(res); +} + +#[test] +pub fn calloc_free() { + let res = mmtk_calloc(1, 8); + assert!(!res.is_zero()); + mmtk_free(res); +} + +#[test] +pub fn realloc_free() { + let res1 = mmtk_malloc(8); + assert!(!res1.is_zero()); + let res2 = mmtk_realloc(res1, 16); + assert!(!res2.is_zero()); + mmtk_free(res2); +} diff --git a/vmbindings/dummyvm/src/tests/malloc_counted.rs b/vmbindings/dummyvm/src/tests/malloc_counted.rs new file mode 100644 index 0000000000..1fc831a944 --- /dev/null +++ b/vmbindings/dummyvm/src/tests/malloc_counted.rs @@ -0,0 +1,84 @@ +// GITHUB-CI: FEATURES=malloc_counted_size + +use crate::tests::fixtures::{Fixture, MMTKSingleton}; +use crate::api::*; + +lazy_static! { + static ref MMTK_SINGLETON: Fixture = Fixture::new(); +} + +#[test] +pub fn malloc_free() { + MMTK_SINGLETON.with_fixture(|fixture| { + let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); + + let res = mmtk_malloc(8); + assert!(!res.is_zero()); + let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before + 8, bytes_after_alloc); + + mmtk_free_with_size(res, 8); + let bytes_after_free = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before, bytes_after_free); + }); +} + +#[test] +pub fn calloc_free() { + MMTK_SINGLETON.with_fixture(|fixture| { + let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); + + let res = mmtk_calloc(1, 8); + assert!(!res.is_zero()); + let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before + 8, bytes_after_alloc); + + mmtk_free_with_size(res, 8); + let bytes_after_free = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before, bytes_after_free); + }); +} + +#[test] +pub fn realloc_grow() { + MMTK_SINGLETON.with_fixture(|fixture| { + let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); + + let res1 = mmtk_malloc(8); + assert!(!res1.is_zero()); + let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before + 8, bytes_after_alloc); + + // grow to 16 bytes + let res2 = mmtk_realloc_with_old_size(res1, 16, 8); + assert!(!res2.is_zero()); + let bytes_after_realloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before + 16, bytes_after_realloc); + + mmtk_free_with_size(res2, 16); + let bytes_after_free = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before, bytes_after_free); + }); +} + +#[test] +pub fn realloc_shrink() { + MMTK_SINGLETON.with_fixture(|fixture| { + let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); + + let res1 = mmtk_malloc(16); + assert!(!res1.is_zero()); + let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before + 16, bytes_after_alloc); + + // shrink to 8 bytes + let res2 = mmtk_realloc_with_old_size(res1, 8, 16); + assert!(!res2.is_zero()); + let bytes_after_realloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before + 8, bytes_after_realloc); + + mmtk_free_with_size(res2, 8); + let bytes_after_free = fixture.mmtk.get_plan().base().get_malloc_bytes(); + assert_eq!(bytes_before, bytes_after_free); + }); +} diff --git a/vmbindings/dummyvm/src/tests/malloc_ms.rs b/vmbindings/dummyvm/src/tests/malloc_ms.rs new file mode 100644 index 0000000000..0eeb513163 --- /dev/null +++ b/vmbindings/dummyvm/src/tests/malloc_ms.rs @@ -0,0 +1,36 @@ +use mmtk::util::malloc::malloc_ms_util; +use crate::DummyVM; + +#[test] +fn test_malloc() { + let (address1, bool1) = malloc_ms_util::alloc::(16, 8, 0); + let (address2, bool2) = malloc_ms_util::alloc::(16, 32, 0); + let (address3, bool3) = malloc_ms_util::alloc::(16, 8, 4); + let (address4, bool4) = malloc_ms_util::alloc::(32, 64, 4); + + assert!(address1.is_aligned_to(8)); + assert!(address2.is_aligned_to(32)); + assert!((address3 + 4 as isize).is_aligned_to(8)); + assert!((address4 + 4 as isize).is_aligned_to(64)); + + assert!(!bool1); + #[cfg(feature = "malloc_hoard")] + assert!(bool2); + #[cfg(not(feature = "malloc_hoard"))] + assert!(!bool2); + assert!(bool3); + assert!(bool4); + + assert!(malloc_ms_util::get_malloc_usable_size(address1, bool1) >= 16); + assert!(malloc_ms_util::get_malloc_usable_size(address2, bool2) >= 16); + assert!(malloc_ms_util::get_malloc_usable_size(address3, bool3) >= 16); + assert!(malloc_ms_util::get_malloc_usable_size(address4, bool4) >= 32); + + unsafe { malloc_ms_util::free(address1.to_mut_ptr()); } + #[cfg(feature = "malloc_hoard")] + malloc_ms_util::offset_free(address2); + #[cfg(not(feature = "malloc_hoard"))] + unsafe { malloc_ms_util::free(address2.to_mut_ptr()); } + malloc_ms_util::offset_free(address3); + malloc_ms_util::offset_free(address4); +} diff --git a/vmbindings/dummyvm/src/tests/mod.rs b/vmbindings/dummyvm/src/tests/mod.rs index d8f2a202b9..095ff11dc2 100644 --- a/vmbindings/dummyvm/src/tests/mod.rs +++ b/vmbindings/dummyvm/src/tests/mod.rs @@ -11,7 +11,11 @@ mod allocate_without_initialize_collection; mod allocate_with_initialize_collection; mod allocate_with_disable_collection; mod allocate_with_re_enable_collection; -mod malloc; +#[cfg(not(feature = "malloc_counted_size"))] +mod malloc_api; +#[cfg(feature = "malloc_counted_size")] +mod malloc_counted; +mod malloc_ms; #[cfg(feature = "is_mmtk_object")] mod conservatism; mod is_in_mmtk_spaces; From d2cddc3d687e65dfbb25785b55e56ed5f2fa95dc Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 7 Jun 2022 10:06:53 +1000 Subject: [PATCH 3/7] Update hoard-sys to 0.1.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 101a85519e..b339a509ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ enum_derive = "0.1" libc = "0.2" jemalloc-sys = {version = "0.3.2", features = ["disable_initial_exec_tls"], optional = true } mimalloc-sys = {version = "0.1.6", optional = true } -hoard-sys = {version = "0.1.1", optional = true, path = "/home/yilin/Code/hoard-sys" } +hoard-sys = {version = "0.1.2", optional = true } lazy_static = "1.1" log = {version = "0.4", features = ["max_level_trace", "release_max_level_off"] } crossbeam-deque = "0.6" From 5c7b08dbbbd745c3db42faa5f08095ce1854e995 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 7 Jun 2022 11:11:52 +1000 Subject: [PATCH 4/7] Clean up --- .github/scripts/ci-test.sh | 2 +- src/memory_manager.rs | 48 +++------ src/util/malloc/library.rs | 20 ++-- src/util/malloc/mod.rs | 100 ++++++++++-------- vmbindings/dummyvm/src/api.rs | 24 ++--- vmbindings/dummyvm/src/tests/fixtures/mod.rs | 19 +++- .../dummyvm/src/tests/malloc_counted.rs | 10 +- 7 files changed, 117 insertions(+), 106 deletions(-) diff --git a/.github/scripts/ci-test.sh b/.github/scripts/ci-test.sh index 76ef86873e..041a4bb816 100755 --- a/.github/scripts/ci-test.sh +++ b/.github/scripts/ci-test.sh @@ -18,7 +18,7 @@ cd vmbindings/dummyvm for fn in $(ls src/tests/*.rs); do t=$(basename -s .rs $fn) - if [[ $t == "mod.rs" ]]; then + if [[ $t == "mod" ]]; then continue fi diff --git a/src/memory_manager.rs b/src/memory_manager.rs index 5c0b1689cf..de8a993ade 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -154,8 +154,7 @@ pub fn get_allocator_mapping( /// The standard malloc. MMTk either uses its own allocator, or forward the call to a /// library malloc. -#[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_malloc(size: usize) -> Address { +pub fn malloc(size: usize) -> Address { crate::util::malloc::malloc(size) } @@ -163,34 +162,24 @@ pub fn mmtk_malloc(size: usize) -> Address { /// Thus the method requires a reference to an MMTk instance. MMTk either uses its own allocator, or forward the call to a /// library malloc. #[cfg(feature = "malloc_counted_size")] -pub fn mmtk_malloc(mmtk: &MMTK, size: usize) -> Address { - let res = crate::util::malloc::malloc(size); - if !res.is_zero() { - mmtk.plan.base().increase_malloc_bytes_by(size); - } - return res; +pub fn counted_malloc(mmtk: &MMTK, size: usize) -> Address { + crate::util::malloc::counted_malloc(mmtk, size) } /// The standard calloc. -#[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_calloc(num: usize, size: usize) -> Address { +pub fn calloc(num: usize, size: usize) -> Address { crate::util::malloc::calloc(num, size) } /// The standard calloc except that with the feature `malloc_counted_size`, MMTk will count the allocated memory into its heap size. /// Thus the method requires a reference to an MMTk instance. #[cfg(feature = "malloc_counted_size")] -pub fn mmtk_calloc(mmtk: &MMTK, num: usize, size: usize) -> Address { - let res = crate::util::malloc::calloc(num, size); - if !res.is_zero() { - mmtk.plan.base().increase_malloc_bytes_by(num * size); - } - return res; +pub fn counted_calloc(mmtk: &MMTK, num: usize, size: usize) -> Address { + crate::util::malloc::counted_calloc(mmtk, num, size) } /// The standard realloc. -#[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_realloc(addr: Address, size: usize) -> Address { +pub fn realloc(addr: Address, size: usize) -> Address { crate::util::malloc::realloc(addr, size) } @@ -198,28 +187,18 @@ pub fn mmtk_realloc(addr: Address, size: usize) -> Address { /// Thus the method requires a reference to an MMTk instance, and the size of the existing memory that will be reallocated. /// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. #[cfg(feature = "malloc_counted_size")] -pub fn mmtk_realloc_with_old_size( +pub fn realloc_with_old_size( mmtk: &MMTK, addr: Address, size: usize, old_size: usize, ) -> Address { - let res = crate::util::malloc::realloc(addr, size); - - if !addr.is_zero() { - mmtk.plan.base().decrease_malloc_bytes_by(old_size); - } - if size != 0 && !res.is_zero() { - mmtk.plan.base().increase_malloc_bytes_by(size); - } - - return res; + crate::util::malloc::realloc_with_old_size(mmtk, addr, size, old_size) } /// The standard free. /// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. -#[cfg(not(feature = "malloc_counted_size"))] -pub fn mmtk_free(addr: Address) { +pub fn free(addr: Address) { crate::util::malloc::free(addr) } @@ -227,11 +206,8 @@ pub fn mmtk_free(addr: Address) { /// Thus the method requires a reference to an MMTk instance, and the size of the memory to free. /// The `addr` in the arguments must be an address that is earlier returned from MMTk's `malloc()`, `calloc()` or `realloc()`. #[cfg(feature = "malloc_counted_size")] -pub fn mmtk_free_with_size(mmtk: &MMTK, addr: Address, old_size: usize) { - crate::util::malloc::free(addr); - if !addr.is_zero() { - mmtk.plan.base().decrease_malloc_bytes_by(old_size); - } +pub fn free_with_size(mmtk: &MMTK, addr: Address, old_size: usize) { + crate::util::malloc::free_with_size(mmtk, addr, old_size) } /// Run the main loop for the GC controller thread. This method does not return. diff --git a/src/util/malloc/library.rs b/src/util/malloc/library.rs index c1aaaf3a8c..84e5bd3aef 100644 --- a/src/util/malloc/library.rs +++ b/src/util/malloc/library.rs @@ -15,12 +15,16 @@ pub use self::mimalloc::*; // Different malloc libraries +// TODO: We should conditinally include some methods in the module, such as posix extension and GNU extension. + #[cfg(feature = "malloc_jemalloc")] mod jemalloc { // ANSI C pub use jemalloc_sys::{calloc, free, malloc, realloc}; // Posix - pub use jemalloc_sys::{malloc_usable_size, posix_memalign}; + pub use jemalloc_sys::posix_memalign; + // GNU + pub use jemalloc_sys::malloc_usable_size; } #[cfg(feature = "malloc_mimalloc")] @@ -30,9 +34,9 @@ mod mimalloc { mi_calloc as calloc, mi_free as free, mi_malloc as malloc, mi_realloc as realloc, }; // Posix - pub use mimalloc_sys::{ - mi_malloc_usable_size as malloc_usable_size, mi_posix_memalign as posix_memalign, - }; + pub use mimalloc_sys::mi_posix_memalign as posix_memalign; + // GNU + pub use mimalloc_sys::mi_malloc_usable_size as malloc_usable_size; } #[cfg(feature = "malloc_hoard")] @@ -40,7 +44,9 @@ mod hoard { // ANSI C pub use hoard_sys::{calloc, free, malloc, realloc}; // Posix - pub use hoard_sys::{malloc_usable_size, posix_memalign}; + pub use hoard_sys::posix_memalign; + // GNU + pub use hoard_sys::malloc_usable_size; } /// If no malloc lib is specified, use the libc implementation @@ -53,5 +59,7 @@ mod libc_malloc { // ANSI C pub use libc::{calloc, free, malloc, realloc}; // Posix - pub use libc::{malloc_usable_size, posix_memalign}; + pub use libc::posix_memalign; + // GNU + pub use libc::malloc_usable_size; } diff --git a/src/util/malloc/mod.rs b/src/util/malloc/mod.rs index 259f5b8b7d..0997313448 100644 --- a/src/util/malloc/mod.rs +++ b/src/util/malloc/mod.rs @@ -1,75 +1,89 @@ +/// Malloc provided by libraries pub(crate) mod library; +/// Using malloc as mark sweep free-list allocator pub mod malloc_ms_util; use crate::util::Address; +#[cfg(feature = "malloc_counted_size")] use crate::vm::VMBinding; +#[cfg(feature = "malloc_counted_size")] use crate::MMTK; +// The following expose a set of malloc API. They are currently implemented with +// the library malloc. When we have native malloc implementation, we should change +// their implementation to point to our native malloc. + +// We have two versions for each function: +// * a normal version: it has the signature that is compatible with the standard malloc library. +// * a counted version: the allocated/freed bytes are calculated into MMTk's heap. So extra arguments +// are needed to maintain allocated bytes properly. The API is inspired by Julia's counted malloc. +// The counted version is only available with the feature `malloc_counted_size`. + #[inline(always)] pub fn malloc(size: usize) -> Address { Address::from_mut_ptr(unsafe { self::library::malloc(size) }) } +#[cfg(feature = "malloc_counted_size")] #[inline(always)] -pub fn calloc(num: usize, size: usize) -> Address { - Address::from_mut_ptr(unsafe { self::library::calloc(num, size) }) +pub fn counted_malloc(mmtk: &MMTK, size: usize) -> Address { + let res = malloc(size); + if !res.is_zero() { + mmtk.plan.base().increase_malloc_bytes_by(size); + } + return res; } #[inline(always)] -pub fn realloc(addr: Address, size: usize) -> Address { - Address::from_mut_ptr(unsafe { self::library::realloc(addr.to_mut_ptr(), size) }) +pub fn calloc(num: usize, size: usize) -> Address { + Address::from_mut_ptr(unsafe { self::library::calloc(num, size) }) } +#[cfg(feature = "malloc_counted_size")] #[inline(always)] -pub fn free(addr: Address) { - unsafe { self::library::free(addr.to_mut_ptr()) } -} - -impl MMTK { - #[inline(always)] - pub fn malloc(&self, size: usize) -> Address { - #[cfg(feature = "malloc_counted_size")] - self.plan.base().increase_malloc_bytes_by(size); - - Address::from_mut_ptr(unsafe { self::library::malloc(size) }) +pub fn counted_calloc(mmtk: &MMTK, num: usize, size: usize) -> Address { + let res = calloc(num, size); + if !res.is_zero() { + mmtk.plan.base().increase_malloc_bytes_by(num * size); } + return res; +} - #[inline(always)] - pub fn calloc(&self, num: usize, size: usize) -> Address { - #[cfg(feature = "malloc_counted_size")] - self.plan.base().increase_malloc_bytes_by(num * size); - - Address::from_mut_ptr(unsafe { self::library::calloc(num, size) }) - } +#[inline(always)] +pub fn realloc(addr: Address, size: usize) -> Address { + Address::from_mut_ptr(unsafe { self::library::realloc(addr.to_mut_ptr(), size) }) } #[cfg(feature = "malloc_counted_size")] -impl MMTK { - #[inline(always)] - pub fn realloc_with_old_size(&self, addr: Address, size: usize, old_size: usize) -> Address { - let base_plan = self.plan.base(); - base_plan.decrease_malloc_bytes_by(old_size); - base_plan.increase_malloc_bytes_by(size); +#[inline(always)] +pub fn realloc_with_old_size( + mmtk: &MMTK, + addr: Address, + size: usize, + old_size: usize, +) -> Address { + let res = realloc(addr, size); - Address::from_mut_ptr(unsafe { self::library::realloc(addr.to_mut_ptr(), size) }) + if !addr.is_zero() { + mmtk.plan.base().decrease_malloc_bytes_by(old_size); } - - #[inline(always)] - pub fn free_with_size(&self, addr: Address, size: usize) { - self.plan.base().decrease_malloc_bytes_by(size); - unsafe { self::library::free(addr.to_mut_ptr()) } + if size != 0 && !res.is_zero() { + mmtk.plan.base().increase_malloc_bytes_by(size); } + + return res; } -#[cfg(not(feature = "malloc_counted_size"))] -impl MMTK { - #[inline(always)] - pub fn realloc(&self, addr: Address, size: usize) -> Address { - Address::from_mut_ptr(unsafe { self::library::realloc(addr.to_mut_ptr(), size) }) - } +#[inline(always)] +pub fn free(addr: Address) { + unsafe { self::library::free(addr.to_mut_ptr()) } +} - #[inline(always)] - pub fn free(&self, addr: Address) { - unsafe { self::library::free(addr.to_mut_ptr()) } +#[cfg(feature = "malloc_counted_size")] +#[inline(always)] +pub fn free_with_size(mmtk: &MMTK, addr: Address, old_size: usize) { + free(addr); + if !addr.is_zero() { + mmtk.plan.base().decrease_malloc_bytes_by(old_size); } } diff --git a/vmbindings/dummyvm/src/api.rs b/vmbindings/dummyvm/src/api.rs index 714472c50f..dfdd01b639 100644 --- a/vmbindings/dummyvm/src/api.rs +++ b/vmbindings/dummyvm/src/api.rs @@ -170,44 +170,40 @@ pub extern "C" fn mmtk_last_heap_address() -> Address { #[no_mangle] #[cfg(feature = "malloc_counted_size")] -pub extern "C" fn mmtk_malloc(size: usize) -> Address { - memory_manager::mmtk_malloc::(&SINGLETON, size) +pub extern "C" fn mmtk_counted_malloc(size: usize) -> Address { + memory_manager::counted_malloc::(&SINGLETON, size) } #[no_mangle] -#[cfg(not(feature = "malloc_counted_size"))] pub extern "C" fn mmtk_malloc(size: usize) -> Address { - memory_manager::mmtk_malloc(size) + memory_manager::malloc(size) } #[no_mangle] #[cfg(feature = "malloc_counted_size")] -pub extern "C" fn mmtk_calloc(num: usize, size: usize) -> Address { - memory_manager::mmtk_calloc::(&SINGLETON, num, size) +pub extern "C" fn mmtk_counted_calloc(num: usize, size: usize) -> Address { + memory_manager::counted_calloc::(&SINGLETON, num, size) } #[no_mangle] -#[cfg(not(feature = "malloc_counted_size"))] pub extern "C" fn mmtk_calloc(num: usize, size: usize) -> Address { - memory_manager::mmtk_calloc(num, size) + memory_manager::calloc(num, size) } #[no_mangle] #[cfg(feature = "malloc_counted_size")] pub extern "C" fn mmtk_realloc_with_old_size(addr: Address, size: usize, old_size: usize) -> Address { - memory_manager::mmtk_realloc_with_old_size::(&SINGLETON, addr, size, old_size) + memory_manager::realloc_with_old_size::(&SINGLETON, addr, size, old_size) } #[no_mangle] -#[cfg(not(feature = "malloc_counted_size"))] pub extern "C" fn mmtk_realloc(addr: Address, size: usize) -> Address { - memory_manager::mmtk_realloc(addr, size) + memory_manager::realloc(addr, size) } #[no_mangle] #[cfg(feature = "malloc_counted_size")] pub extern "C" fn mmtk_free_with_size(addr: Address, old_size: usize) { - memory_manager::mmtk_free_with_size::(&SINGLETON, addr, old_size) + memory_manager::free_with_size::(&SINGLETON, addr, old_size) } #[no_mangle] -#[cfg(not(feature = "malloc_counted_size"))] pub extern "C" fn mmtk_free(addr: Address) { - memory_manager::mmtk_free(addr) + memory_manager::free(addr) } diff --git a/vmbindings/dummyvm/src/tests/fixtures/mod.rs b/vmbindings/dummyvm/src/tests/fixtures/mod.rs index 7414833e18..8dad6dc8b9 100644 --- a/vmbindings/dummyvm/src/tests/fixtures/mod.rs +++ b/vmbindings/dummyvm/src/tests/fixtures/mod.rs @@ -1,5 +1,6 @@ use atomic_refcell::AtomicRefCell; use std::sync::Once; +use std::sync::Mutex; use mmtk::AllocationSemantics; use mmtk::MMTK; @@ -16,15 +17,27 @@ pub trait FixtureContent { pub struct Fixture { content: AtomicRefCell>>, once: Once, + serial_lock: Option>, } unsafe impl Sync for Fixture {} impl Fixture { + #[allow(dead_code)] pub fn new() -> Self { Self { content: AtomicRefCell::new(None), once: Once::new(), + serial_lock: None, + } + } + + #[allow(dead_code)] + pub fn new_serial_tests() -> Self { + Self { + content: AtomicRefCell::new(None), + once: Once::new(), + serial_lock: Some(Mutex::default()), } } @@ -34,7 +47,11 @@ impl Fixture { let mut borrow = self.content.borrow_mut(); *borrow = Some(content); }); - { + if let Some(mutex) = &self.serial_lock { + let _lock = mutex.lock(); + let borrow = self.content.borrow(); + func(borrow.as_ref().unwrap()) + } else { let borrow = self.content.borrow(); func(borrow.as_ref().unwrap()) } diff --git a/vmbindings/dummyvm/src/tests/malloc_counted.rs b/vmbindings/dummyvm/src/tests/malloc_counted.rs index 1fc831a944..1ed1d08552 100644 --- a/vmbindings/dummyvm/src/tests/malloc_counted.rs +++ b/vmbindings/dummyvm/src/tests/malloc_counted.rs @@ -4,7 +4,7 @@ use crate::tests::fixtures::{Fixture, MMTKSingleton}; use crate::api::*; lazy_static! { - static ref MMTK_SINGLETON: Fixture = Fixture::new(); + static ref MMTK_SINGLETON: Fixture = Fixture::new_serial_tests(); } #[test] @@ -12,7 +12,7 @@ pub fn malloc_free() { MMTK_SINGLETON.with_fixture(|fixture| { let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); - let res = mmtk_malloc(8); + let res = mmtk_counted_malloc(8); assert!(!res.is_zero()); let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); assert_eq!(bytes_before + 8, bytes_after_alloc); @@ -28,7 +28,7 @@ pub fn calloc_free() { MMTK_SINGLETON.with_fixture(|fixture| { let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); - let res = mmtk_calloc(1, 8); + let res = mmtk_counted_calloc(1, 8); assert!(!res.is_zero()); let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); assert_eq!(bytes_before + 8, bytes_after_alloc); @@ -44,7 +44,7 @@ pub fn realloc_grow() { MMTK_SINGLETON.with_fixture(|fixture| { let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); - let res1 = mmtk_malloc(8); + let res1 = mmtk_counted_malloc(8); assert!(!res1.is_zero()); let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); assert_eq!(bytes_before + 8, bytes_after_alloc); @@ -66,7 +66,7 @@ pub fn realloc_shrink() { MMTK_SINGLETON.with_fixture(|fixture| { let bytes_before = fixture.mmtk.get_plan().base().get_malloc_bytes(); - let res1 = mmtk_malloc(16); + let res1 = mmtk_counted_malloc(16); assert!(!res1.is_zero()); let bytes_after_alloc = fixture.mmtk.get_plan().base().get_malloc_bytes(); assert_eq!(bytes_before + 16, bytes_after_alloc); From d0bc14ab2f660f6e4570ff4d774bb9b9ffac0544 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 7 Jun 2022 11:27:45 +1000 Subject: [PATCH 5/7] Fix clippy --- src/util/malloc/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/malloc/mod.rs b/src/util/malloc/mod.rs index 0997313448..551b014026 100644 --- a/src/util/malloc/mod.rs +++ b/src/util/malloc/mod.rs @@ -31,7 +31,7 @@ pub fn counted_malloc(mmtk: &MMTK, size: usize) -> Address { if !res.is_zero() { mmtk.plan.base().increase_malloc_bytes_by(size); } - return res; + res } #[inline(always)] @@ -46,7 +46,7 @@ pub fn counted_calloc(mmtk: &MMTK, num: usize, size: usize) - if !res.is_zero() { mmtk.plan.base().increase_malloc_bytes_by(num * size); } - return res; + res } #[inline(always)] @@ -71,7 +71,7 @@ pub fn realloc_with_old_size( mmtk.plan.base().increase_malloc_bytes_by(size); } - return res; + res } #[inline(always)] From c62b150595cf38880b1f95f1ee0bccb65ae8004d Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 21 Jun 2022 14:36:15 +1000 Subject: [PATCH 6/7] Add gc_poll() --- docs/tutorial/code/mygc_semispace/global.rs | 4 ++-- src/memory_manager.rs | 20 ++++++++++++++++ src/plan/generational/copying/global.rs | 2 +- src/plan/generational/global.rs | 11 +++++---- src/plan/generational/immix/global.rs | 2 +- src/plan/global.rs | 26 +++++++++++++-------- src/plan/immix/global.rs | 4 ++-- src/plan/markcompact/global.rs | 4 ++-- src/plan/marksweep/global.rs | 4 ++-- src/plan/nogc/global.rs | 4 ++-- src/plan/pageprotect/global.rs | 4 ++-- src/plan/semispace/global.rs | 4 ++-- src/policy/mallocspace/global.rs | 2 +- src/policy/space.rs | 4 ++-- 14 files changed, 61 insertions(+), 34 deletions(-) diff --git a/docs/tutorial/code/mygc_semispace/global.rs b/docs/tutorial/code/mygc_semispace/global.rs index c0c09d0b00..b7d6233cfb 100644 --- a/docs/tutorial/code/mygc_semispace/global.rs +++ b/docs/tutorial/code/mygc_semispace/global.rs @@ -102,8 +102,8 @@ impl Plan for MyGC { // ANCHOR_END: schedule_collection // ANCHOR: collection_required() - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool { - self.base().collection_required(self, space_full, space) + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + self.base().collection_required(self, space_full) } // ANCHOR_END: collection_required() diff --git a/src/memory_manager.rs b/src/memory_manager.rs index 9d94e42763..5509344dad 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -211,6 +211,26 @@ pub fn free_with_size(mmtk: &MMTK, addr: Address, old_size: u crate::util::malloc::free_with_size(mmtk, addr, old_size) } +/// Poll for GC. MMTk will decide if a GC is needed. If so, this call will block +/// the current thread, and trigger a GC. Otherwise, it will simply return. +/// Usually a binding does not need to call this function. MMTk will poll for GC during its allocation. +/// However, if a binding uses counted malloc (which won't poll for GC), they may want to poll for GC manually. +/// This function should only be used by mutator threads. +pub fn gc_poll(mmtk: &MMTK, tls: VMMutatorThread) { + use crate::vm::{ActivePlan, Collection}; + debug_assert!( + VM::VMActivePlan::is_mutator(tls.0), + "gc_poll() can only be called by a mutator thread." + ); + + let plan = mmtk.get_plan(); + if plan.should_trigger_gc_when_heap_is_full() && plan.poll(false, None) { + debug!("Collection required"); + assert!(plan.is_initialized(), "GC is not allowed here: collection is not initialized (did you call initialize_collection()?)."); + VM::VMCollection::block_for_gc(tls); + } +} + /// Run the main loop for the GC controller thread. This method does not return. /// /// Arguments: diff --git a/src/plan/generational/copying/global.rs b/src/plan/generational/copying/global.rs index 3384760827..d53fc97580 100644 --- a/src/plan/generational/copying/global.rs +++ b/src/plan/generational/copying/global.rs @@ -64,7 +64,7 @@ impl Plan for GenCopy { } } - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool + fn collection_required(&self, space_full: bool, space: Option<&dyn Space>) -> bool where Self: Sized, { diff --git a/src/plan/generational/global.rs b/src/plan/generational/global.rs index f02032be0d..8611e1c418 100644 --- a/src/plan/generational/global.rs +++ b/src/plan/generational/global.rs @@ -106,7 +106,7 @@ impl Gen { &self, plan: &P, space_full: bool, - space: &dyn Space, + space: Option<&dyn Space>, ) -> bool { let nursery_full = self.nursery.reserved_pages() >= (conversions::bytes_to_pages_up(*self.common.base.options.max_nursery)); @@ -114,13 +114,14 @@ impl Gen { return true; } - if space_full && space.common().descriptor != self.nursery.common().descriptor { + if space_full + && space.is_some() + && space.unwrap().common().descriptor != self.nursery.common().descriptor + { self.next_gc_full_heap.store(true, Ordering::SeqCst); } - self.common - .base - .collection_required(plan, space_full, space) + self.common.base.collection_required(plan, space_full) } pub fn force_full_heap_collection(&self) { diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs index eb0714adf3..8e26cbf15a 100644 --- a/src/plan/generational/immix/global.rs +++ b/src/plan/generational/immix/global.rs @@ -96,7 +96,7 @@ impl Plan for GenImmix { self.gen.last_collection_full_heap() } - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool + fn collection_required(&self, space_full: bool, space: Option<&dyn Space>) -> bool where Self: Sized, { diff --git a/src/plan/global.rs b/src/plan/global.rs index 24bbd9dc62..ef1f90a295 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -203,7 +203,14 @@ pub trait Plan: 'static + Sync + Downcast { /// This is invoked once per GC by one worker thread. 'tls' is the worker thread that executes this method. fn release(&mut self, tls: VMWorkerThread); - fn poll(&self, space_full: bool, space: &dyn Space) -> bool { + /// This method is called periodically by the allocation subsystem + /// (by default, each time a page is consumed), and provides the + /// collector with an opportunity to collect. + /// + /// Arguments: + /// * `space_full`: Space request failed, must recover pages within 'space'. + /// * `space`: The space that triggered the poll. This could `None` if the poll is not triggered by a space. + fn poll(&self, space_full: bool, space: Option<&dyn Space>) -> bool { if self.collection_required(space_full, space) { // FIXME /*if space == META_DATA_SPACE { @@ -236,8 +243,12 @@ pub trait Plan: 'static + Sync + Downcast { false } - fn log_poll(&self, space: &dyn Space, message: &'static str) { - info!(" [POLL] {}: {}", space.get_name(), message); + fn log_poll(&self, space: Option<&dyn Space>, message: &'static str) { + if let Some(space) = space { + info!(" [POLL] {}: {}", space.get_name(), message); + } else { + info!(" [POLL] {}", message); + } } /** @@ -248,7 +259,7 @@ pub trait Plan: 'static + Sync + Downcast { * @param space TODO * @return true if a collection is requested by the plan. */ - fn collection_required(&self, space_full: bool, _space: &dyn Space) -> bool; + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool; // Note: The following methods are about page accounting. The default implementation should // work fine for non-copying plans. For copying plans, the plan should override any of these methods @@ -805,12 +816,7 @@ impl BasePlan { && (self.allocation_bytes.load(Ordering::SeqCst) > *self.options.stress_factor) } - pub(super) fn collection_required( - &self, - plan: &P, - space_full: bool, - _space: &dyn Space, - ) -> bool { + pub(super) fn collection_required(&self, plan: &P, space_full: bool) -> bool { let stress_force_gc = self.should_do_stress_gc(); if stress_force_gc { debug!( diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs index 2d40d32a8b..c910d097e8 100644 --- a/src/plan/immix/global.rs +++ b/src/plan/immix/global.rs @@ -51,8 +51,8 @@ pub const IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints { impl Plan for Immix { type VM = VM; - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool { - self.base().collection_required(self, space_full, space) + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + self.base().collection_required(self, space_full) } fn last_collection_was_exhaustive(&self) -> bool { diff --git a/src/plan/markcompact/global.rs b/src/plan/markcompact/global.rs index c7e3f451d8..5650c07494 100644 --- a/src/plan/markcompact/global.rs +++ b/src/plan/markcompact/global.rs @@ -163,8 +163,8 @@ impl Plan for MarkCompact { .add(crate::util::sanity::sanity_checker::ScheduleSanityGC::::new(self)); } - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool { - self.base().collection_required(self, space_full, space) + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + self.base().collection_required(self, space_full) } fn get_used_pages(&self) -> usize { diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index c06ac5f5c8..963702b823 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -72,8 +72,8 @@ impl Plan for MarkSweep { self.common.release(tls, true); } - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool { - self.base().collection_required(self, space_full, space) + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + self.base().collection_required(self, space_full) } fn get_used_pages(&self) -> usize { diff --git a/src/plan/nogc/global.rs b/src/plan/nogc/global.rs index 74bd1efd33..def42bb34e 100644 --- a/src/plan/nogc/global.rs +++ b/src/plan/nogc/global.rs @@ -48,8 +48,8 @@ impl Plan for NoGC { self.nogc_space.init(vm_map); } - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool { - self.base().collection_required(self, space_full, space) + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + self.base().collection_required(self, space_full) } fn base(&self) -> &BasePlan { diff --git a/src/plan/pageprotect/global.rs b/src/plan/pageprotect/global.rs index 6f5b8b47e2..1da5c53dfe 100644 --- a/src/plan/pageprotect/global.rs +++ b/src/plan/pageprotect/global.rs @@ -79,8 +79,8 @@ impl Plan for PageProtect { self.space.release(true); } - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool { - self.base().collection_required(self, space_full, space) + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + self.base().collection_required(self, space_full) } fn get_used_pages(&self) -> usize { diff --git a/src/plan/semispace/global.rs b/src/plan/semispace/global.rs index 6b8db36328..526a8b4543 100644 --- a/src/plan/semispace/global.rs +++ b/src/plan/semispace/global.rs @@ -110,8 +110,8 @@ impl Plan for SemiSpace { self.fromspace().release(); } - fn collection_required(&self, space_full: bool, space: &dyn Space) -> bool { - self.base().collection_required(self, space_full, space) + fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + self.base().collection_required(self, space_full) } fn get_collection_reserved_pages(&self) -> usize { diff --git a/src/policy/mallocspace/global.rs b/src/policy/mallocspace/global.rs index 95a452ebcc..ddd71691ba 100644 --- a/src/policy/mallocspace/global.rs +++ b/src/policy/mallocspace/global.rs @@ -238,7 +238,7 @@ impl MallocSpace { 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) { + if VM::VMActivePlan::global().poll(false, Some(self)) { assert!(VM::VMActivePlan::is_mutator(tls), "Polling in GC worker"); VM::VMCollection::block_for_gc(VMMutatorThread(tls)); return unsafe { Address::zero() }; diff --git a/src/policy/space.rs b/src/policy/space.rs index ae158260f4..5408af1cee 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -406,7 +406,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { trace!("Pages reserved"); trace!("Polling .."); - if should_poll && VM::VMActivePlan::global().poll(false, self.as_space()) { + if should_poll && VM::VMActivePlan::global().poll(false, Some(self.as_space())) { debug!("Collection required"); assert!(allow_gc, "GC is not allowed here: collection is not initialized (did you call initialize_collection()?)."); pr.clear_request(pages_reserved); @@ -484,7 +484,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { "Physical allocation failed when GC is not allowed!" ); - let gc_performed = VM::VMActivePlan::global().poll(true, self.as_space()); + let gc_performed = VM::VMActivePlan::global().poll(true, Some(self.as_space())); debug_assert!(gc_performed, "GC not performed when forced."); pr.clear_request(pages_reserved); drop(lock); // drop the lock before block From 83d94887371fc3f95a2a796f518d750019208ace Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Fri, 24 Jun 2022 11:43:03 +1000 Subject: [PATCH 7/7] Fix a conditional in gen.collection_required(). Add SerialFixture for dummyVM testing. --- src/plan/generational/global.rs | 12 +++-- vmbindings/dummyvm/src/tests/fixtures/mod.rs | 45 +++++++++++-------- .../dummyvm/src/tests/malloc_counted.rs | 4 +- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/plan/generational/global.rs b/src/plan/generational/global.rs index 8611e1c418..a5a4108aa4 100644 --- a/src/plan/generational/global.rs +++ b/src/plan/generational/global.rs @@ -114,10 +114,14 @@ impl Gen { return true; } - if space_full - && space.is_some() - && space.unwrap().common().descriptor != self.nursery.common().descriptor - { + // Is the GC triggered by nursery? + // - if space is none, it is not. Return false immediately. + // - if space is some, we further check its descriptor. + let is_triggered_by_nursery = space.map_or(false, |s| { + s.common().descriptor == self.nursery.common().descriptor + }); + // If space is full and the GC is not triggered by nursery, next GC will be full heap GC. + if space_full && !is_triggered_by_nursery { self.next_gc_full_heap.store(true, Ordering::SeqCst); } diff --git a/vmbindings/dummyvm/src/tests/fixtures/mod.rs b/vmbindings/dummyvm/src/tests/fixtures/mod.rs index 8dad6dc8b9..a3d30fbe50 100644 --- a/vmbindings/dummyvm/src/tests/fixtures/mod.rs +++ b/vmbindings/dummyvm/src/tests/fixtures/mod.rs @@ -1,3 +1,6 @@ +// Some tests are conditionally compiled. So not all the code in this module will be used. We simply allow dead code in this module. +#![allow(dead_code)] + use atomic_refcell::AtomicRefCell; use std::sync::Once; use std::sync::Mutex; @@ -14,30 +17,19 @@ pub trait FixtureContent { fn create() -> Self; } + pub struct Fixture { content: AtomicRefCell>>, once: Once, - serial_lock: Option>, } unsafe impl Sync for Fixture {} impl Fixture { - #[allow(dead_code)] pub fn new() -> Self { Self { content: AtomicRefCell::new(None), once: Once::new(), - serial_lock: None, - } - } - - #[allow(dead_code)] - pub fn new_serial_tests() -> Self { - Self { - content: AtomicRefCell::new(None), - once: Once::new(), - serial_lock: Some(Mutex::default()), } } @@ -47,14 +39,29 @@ impl Fixture { let mut borrow = self.content.borrow_mut(); *borrow = Some(content); }); - if let Some(mutex) = &self.serial_lock { - let _lock = mutex.lock(); - let borrow = self.content.borrow(); - func(borrow.as_ref().unwrap()) - } else { - let borrow = self.content.borrow(); - func(borrow.as_ref().unwrap()) + let borrow = self.content.borrow(); + func(borrow.as_ref().unwrap()) + } +} + +/// SerialFixture ensures all `with_fixture()` calls will be executed serially. +pub struct SerialFixture { + content: Mutex>> +} + +impl SerialFixture { + pub fn new() -> Self { + Self { + content: Mutex::new(None) + } + } + + pub fn with_fixture(&self, func: F) { + let mut c = self.content.lock().unwrap(); + if c.is_none() { + *c = Some(Box::new(T::create())); } + func(c.as_ref().unwrap()) } } diff --git a/vmbindings/dummyvm/src/tests/malloc_counted.rs b/vmbindings/dummyvm/src/tests/malloc_counted.rs index 1ed1d08552..3f003fbecc 100644 --- a/vmbindings/dummyvm/src/tests/malloc_counted.rs +++ b/vmbindings/dummyvm/src/tests/malloc_counted.rs @@ -1,10 +1,10 @@ // GITHUB-CI: FEATURES=malloc_counted_size -use crate::tests::fixtures::{Fixture, MMTKSingleton}; +use crate::tests::fixtures::{SerialFixture, MMTKSingleton}; use crate::api::*; lazy_static! { - static ref MMTK_SINGLETON: Fixture = Fixture::new_serial_tests(); + static ref MMTK_SINGLETON: SerialFixture = SerialFixture::new(); } #[test]