From 8b22d087c313cc3ad444846368be63073c4beb85 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 20 Jan 2025 15:43:23 +0800 Subject: [PATCH 1/3] Replace addr_rshift with log_data_meta_ratio It makes more sense. We also add conversion functions between data and meta sizes. --- src/util/conversions.rs | 10 ++++++ src/util/metadata/side_metadata/global.rs | 6 ++-- src/util/metadata/side_metadata/helpers.rs | 41 ++++++++++++++++++---- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/util/conversions.rs b/src/util/conversions.rs index a28b3ac186..017cc77fc0 100644 --- a/src/util/conversions.rs +++ b/src/util/conversions.rs @@ -94,6 +94,16 @@ pub fn bytes_to_formatted_string(bytes: usize) -> String { format!("{}{}", num, UNITS.last().unwrap()) } +/// Shift `num` by `bits` to the right. Add 1 to the result if any bits are shifted out to the +/// right. This is equivalent to dividing `num` by 2 to the power of `bits`, rounding towards +/// infinity. +/// +/// This function has undefined behavior if `bits` is greater or equal to the number of bits in +/// `usize`. +pub const fn rshift_align_up(num: usize, bits: usize) -> usize { + (num + ((1 << bits) - 1)) >> bits +} + #[cfg(test)] mod tests { use crate::util::conversions::*; diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index b36a1332ad..1eea859832 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -1362,12 +1362,10 @@ impl SideMetadataContext { pub fn calculate_reserved_pages(&self, data_pages: usize) -> usize { let mut total = 0; for spec in self.global.iter() { - let rshift = addr_rshift(spec); - total += (data_pages + ((1 << rshift) - 1)) >> rshift; + total += data_to_meta_size_round_up(spec, data_pages); } for spec in self.local.iter() { - let rshift = addr_rshift(spec); - total += (data_pages + ((1 << rshift) - 1)) >> rshift; + total += data_to_meta_size_round_up(spec, data_pages); } total } diff --git a/src/util/metadata/side_metadata/helpers.rs b/src/util/metadata/side_metadata/helpers.rs index c8dc979764..adb56a28e1 100644 --- a/src/util/metadata/side_metadata/helpers.rs +++ b/src/util/metadata/side_metadata/helpers.rs @@ -2,6 +2,7 @@ use super::ranges::BitOffset; use super::SideMetadataSpec; use crate::util::constants::LOG_BYTES_IN_PAGE; use crate::util::constants::{BITS_IN_WORD, BYTES_IN_PAGE, LOG_BITS_IN_BYTE}; +use crate::util::conversions::rshift_align_up; use crate::util::heap::layout::vm_layout::VMLayout; use crate::util::memory::{MmapAnnotation, MmapStrategy}; #[cfg(target_pointer_width = "32")] @@ -110,7 +111,7 @@ pub(crate) fn ensure_munmap_contiguous_metadata_space( let metadata_start = address_to_meta_address(spec, start); let mmap_start = metadata_start.align_down(BYTES_IN_PAGE); // nearest page-aligned ending address - let metadata_size = (size + ((1 << addr_rshift(spec)) - 1)) >> addr_rshift(spec); + let metadata_size = data_to_meta_size_round_up(spec, size); let mmap_size = (metadata_start + metadata_size).align_up(BYTES_IN_PAGE) - mmap_start; if mmap_size > 0 { ensure_munmap_metadata(mmap_start, mmap_size); @@ -135,7 +136,7 @@ pub(super) fn try_mmap_contiguous_metadata_space( let metadata_start = address_to_meta_address(spec, start); let mmap_start = metadata_start.align_down(BYTES_IN_PAGE); // nearest page-aligned ending address - let metadata_size = (size + ((1 << addr_rshift(spec)) - 1)) >> addr_rshift(spec); + let metadata_size = data_to_meta_size_round_up(spec, size); let mmap_size = (metadata_start + metadata_size).align_up(BYTES_IN_PAGE) - mmap_start; if mmap_size > 0 { if !no_reserve { @@ -185,14 +186,42 @@ pub(crate) fn address_to_meta_address( res } -pub(super) const fn addr_rshift(metadata_spec: &SideMetadataSpec) -> i32 { - ((LOG_BITS_IN_BYTE as usize) + metadata_spec.log_bytes_in_region - - (metadata_spec.log_num_of_bits)) as i32 +/// Return the base-2 logarithm of the ratio of data bits and metadata bits per region. +/// +/// Suppose a memory region has `data_bits` bits of data, and `meta_bits` bits of metadata for +/// `metadata_spec`, and the result of `log_data_meta_ratio(metadata_spec)` is `shift`, then +/// +/// - `data_bits >> shift == meta_bits` +/// - `meta_bits << shift == data_bits` +pub(super) const fn log_data_meta_ratio(metadata_spec: &SideMetadataSpec) -> usize { + let log_data_bits_in_region = (LOG_BITS_IN_BYTE as usize) + metadata_spec.log_bytes_in_region; + let log_meta_bits_in_region = metadata_spec.log_num_of_bits; + + // TODO: In theory, it is possible to construct a side metadata that has more metadata bits than + // data bits per region. But such pathological side metadata consumes way too much memory, and + // should never be used in any useful applications. It should be forbidden. + log_data_bits_in_region - log_meta_bits_in_region +} + +/// Calculate the amount of metadata needed for the give amount of data memory, round up to nearest +/// integer. `data_size` can be in any unit, e.g. bits, bytes, pages, blocks, chunks, etc., and the +/// result has the same unit. +pub(super) const fn data_to_meta_size_round_up( + metadata_spec: &SideMetadataSpec, + data_size: usize, +) -> usize { + rshift_align_up(data_size, log_data_meta_ratio(metadata_spec)) +} + +/// Calculate the amount of data governed by the give amount of metadata. `meta_size` can be in any +/// unit, e.g. bits, bytes, pages, blocks, chunks, etc., and the result has the same unit. +pub(super) const fn meta_to_data_size(metadata_spec: &SideMetadataSpec, meta_size: usize) -> usize { + meta_size << log_data_meta_ratio(metadata_spec) } #[allow(dead_code)] pub(super) const fn metadata_address_range_size(metadata_spec: &SideMetadataSpec) -> usize { - 1usize << (VMLayout::LOG_ARCH_ADDRESS_SPACE - addr_rshift(metadata_spec) as usize) + 1usize << (VMLayout::LOG_ARCH_ADDRESS_SPACE - log_data_meta_ratio(metadata_spec)) } pub(super) fn meta_byte_lshift(metadata_spec: &SideMetadataSpec, data_addr: Address) -> u8 { From 3ef3429743d36cd4b1bc8855ab5c9cfc94fa48ff Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 20 Jan 2025 16:31:16 +0800 Subject: [PATCH 2/3] Make triggering GC and resizing heap consistent. We count size metadata in when reporting pending allocation size to the GC trigger. This ensures the MemBalancer increases the heap size by the same amount (or larger due to over-estimation) as estimated by `Space::reserved_pages`. --- src/policy/lockfreeimmortalspace.rs | 6 +++++- src/policy/marksweepspace/malloc_ms/global.rs | 6 +++++- src/policy/space.rs | 18 ++++++++++++++++-- src/util/metadata/side_metadata/global.rs | 3 +++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/policy/lockfreeimmortalspace.rs b/src/policy/lockfreeimmortalspace.rs index 3863934414..7a7160d8bc 100644 --- a/src/policy/lockfreeimmortalspace.rs +++ b/src/policy/lockfreeimmortalspace.rs @@ -129,10 +129,14 @@ impl Space for LockFreeImmortalSpace { unsafe { sft_map.eager_initialize(self.as_sft(), self.start, self.total_bytes) }; } + fn estimate_side_meta_pages(&self, data_pages: usize) -> usize { + self.metadata.calculate_reserved_pages(data_pages) + } + fn reserved_pages(&self) -> usize { let cursor = self.cursor.load(Ordering::Relaxed); let data_pages = conversions::bytes_to_pages_up(self.limit - cursor); - let meta_pages = self.metadata.calculate_reserved_pages(data_pages); + let meta_pages = self.estimate_side_meta_pages(data_pages); data_pages + meta_pages } diff --git a/src/policy/marksweepspace/malloc_ms/global.rs b/src/policy/marksweepspace/malloc_ms/global.rs index 7327d80d67..83ec1e369b 100644 --- a/src/policy/marksweepspace/malloc_ms/global.rs +++ b/src/policy/marksweepspace/malloc_ms/global.rs @@ -215,6 +215,10 @@ impl Space for MallocSpace { "MallocSpace" } + fn estimate_side_meta_pages(&self, data_pages: usize) -> usize { + self.metadata.calculate_reserved_pages(data_pages) + } + #[allow(clippy::assertions_on_constants)] fn reserved_pages(&self) -> usize { use crate::util::constants::LOG_BYTES_IN_PAGE; @@ -222,7 +226,7 @@ impl Space for MallocSpace { debug_assert!(LOG_BYTES_IN_MALLOC_PAGE >= LOG_BYTES_IN_PAGE); let data_pages = self.active_pages.load(Ordering::SeqCst) << (LOG_BYTES_IN_MALLOC_PAGE - LOG_BYTES_IN_PAGE); - let meta_pages = self.metadata.calculate_reserved_pages(data_pages); + let meta_pages = self.estimate_side_meta_pages(data_pages); data_pages + meta_pages } diff --git a/src/policy/space.rs b/src/policy/space.rs index 7057638dde..3ce1ee3fe1 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -109,9 +109,12 @@ pub trait Space: 'static + SFT + Sync + Downcast { // Clear the request, and inform GC trigger about the pending allocation. pr.clear_request(pages_reserved); + + let meta_pages_reserved = self.estimate_side_meta_pages(pages_reserved); + let total_pages_reserved = pages_reserved + meta_pages_reserved; self.get_gc_trigger() .policy - .on_pending_allocation(pages_reserved); + .on_pending_allocation(total_pages_reserved); VM::VMCollection::block_for_gc(VMMutatorThread(tls)); // We have checked that this is mutator unsafe { Address::zero() } @@ -313,9 +316,20 @@ pub trait Space: 'static + SFT + Sync + Downcast { .mark_as_mapped(self.common().start, self.common().extent); } + /// Estimate the amount of side metadata memory needed for a give data memory size in pages. The + /// result will over-estimate the amount of metadata pages needed, with at least one page per + /// side metadata. This relatively accurately describes the number of side metadata pages the + /// space actually consumes. + /// + /// This function is used for both triggering GC (via [`Space::reserved_pages`]) and resizing + /// the heap (via [`crate::util::heap::GCTriggerPolicy::on_pending_allocation`]). + fn estimate_side_meta_pages(&self, data_pages: usize) -> usize { + self.common().metadata.calculate_reserved_pages(data_pages) + } + fn reserved_pages(&self) -> usize { let data_pages = self.get_page_resource().reserved_pages(); - let meta_pages = self.common().metadata.calculate_reserved_pages(data_pages); + let meta_pages = self.estimate_side_meta_pages(data_pages); data_pages + meta_pages } diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index 1eea859832..ffb3fb8f44 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -1362,6 +1362,9 @@ impl SideMetadataContext { pub fn calculate_reserved_pages(&self, data_pages: usize) -> usize { let mut total = 0; for spec in self.global.iter() { + // This rounds up. No matter how small `data_pages` is, the side metadata size will be + // at least one page. This behavior is *intended*. The over-estimated amount is used + // for triggering GC and resizing the heap. total += data_to_meta_size_round_up(spec, data_pages); } for spec in self.local.iter() { From 96614f4c7698b75198d1806c240e13c691862451 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 20 Jan 2025 16:47:12 +0800 Subject: [PATCH 3/3] Minor comment fixes --- src/util/conversions.rs | 5 ++--- src/util/metadata/side_metadata/global.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/util/conversions.rs b/src/util/conversions.rs index 017cc77fc0..e62dff0392 100644 --- a/src/util/conversions.rs +++ b/src/util/conversions.rs @@ -94,9 +94,8 @@ pub fn bytes_to_formatted_string(bytes: usize) -> String { format!("{}{}", num, UNITS.last().unwrap()) } -/// Shift `num` by `bits` to the right. Add 1 to the result if any bits are shifted out to the -/// right. This is equivalent to dividing `num` by 2 to the power of `bits`, rounding towards -/// infinity. +/// Shift `num` by `bits` to the right. Add 1 to the result if any `1` bits are shifted out to the +/// right. This is equivalent to dividing `num` by 2 to the power of `bits`, rounding up. /// /// This function has undefined behavior if `bits` is greater or equal to the number of bits in /// `usize`. diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index ffb3fb8f44..8730602a4b 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -1363,7 +1363,7 @@ impl SideMetadataContext { let mut total = 0; for spec in self.global.iter() { // This rounds up. No matter how small `data_pages` is, the side metadata size will be - // at least one page. This behavior is *intended*. The over-estimated amount is used + // at least one page. This behavior is *intended*. This over-estimated amount is used // for triggering GC and resizing the heap. total += data_to_meta_size_round_up(spec, data_pages); }