Make chunk map as global side metadata, and each space only lists its own chunks using chunk map #1304
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,20 +44,68 @@ impl Chunk { | |
| } | ||
| } | ||
|
|
||
| /// The allocation state for a chunk in the chunk map. It includes whether each chunk is allocated or free, and the space the chunk belongs to. | ||
| /// Highest bit: 0 = free, 1 = allocated | ||
| /// Lower 4 bits: Space index (0-15) | ||
| #[repr(transparent)] | ||
| #[derive(PartialEq, Clone, Copy)] | ||
| pub struct ChunkState(u8); | ||
|
|
||
| impl ChunkState { | ||
| const ALLOC_BIT_MASK: u8 = 0x80; | ||
| const SPACE_INDEX_MASK: u8 = 0x0F; | ||
|
|
||
| /// Create a new ChunkState that represents being allocated in the given space | ||
| pub fn allocated(space_index: usize) -> ChunkState { | ||
| debug_assert!(space_index < crate::util::heap::layout::heap_parameters::MAX_SPACES); | ||
| let mut encode = space_index as u8; | ||
| encode |= Self::ALLOC_BIT_MASK; | ||
| ChunkState(encode) | ||
| } | ||
| /// Create a new ChunkState that represents being free in the given space | ||
| pub fn free(space_index: usize) -> ChunkState { | ||
| debug_assert!(space_index < crate::util::heap::layout::heap_parameters::MAX_SPACES); | ||
| ChunkState(space_index as u8) | ||
| } | ||
| /// Is the chunk free? | ||
| pub fn is_free(&self) -> bool { | ||
| self.0 & Self::ALLOC_BIT_MASK == 0 | ||
| } | ||
| /// Is the chunk allocated? | ||
| pub fn is_allocated(&self) -> bool { | ||
| !self.is_free() | ||
| } | ||
| /// Get the space index of the chunk | ||
| pub fn get_space_index(&self) -> usize { | ||
| let index = (self.0 & Self::SPACE_INDEX_MASK) as usize; | ||
| debug_assert!(index < crate::util::heap::layout::heap_parameters::MAX_SPACES); | ||
| index | ||
| } | ||
| } | ||
|
|
||
| impl std::fmt::Debug for ChunkState { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| if self.is_free() { | ||
| write!(f, "Free({})", self.get_space_index()) | ||
| } else { | ||
| write!(f, "Allocated({})", self.get_space_index()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// A byte-map to record all the allocated chunks. | ||
| /// A plan can use this to maintain records for the chunks that they used, and the states of the chunks. | ||
| /// Any plan that uses the chunk map should include the `ALLOC_TABLE` spec in their local sidemetadata specs. | ||
| /// | ||
| /// A chunk map is created for a space (identified by the space index), and will only update or list chunks for that space. | ||
| pub struct ChunkMap { | ||
| /// The space that uses this chunk map. | ||
| space_index: usize, | ||
| /// The range of chunks that are used by the space. The range only records the lowest chunk and the highest chunk. | ||
| /// All the chunks that are used for the space are within the range, but not necessarily that all the chunks in the range | ||
| /// are used for the space. Spaces may be discontiguous, thus the range may include chunks that do not belong to the space. | ||
| /// We need to use the space index in the chunk map and the space index encoded with the chunk state to know if | ||
| /// the chunk belongs to the current space. | ||
| chunk_range: Mutex<Range<Chunk>>, | ||
| } | ||
|
|
||
|
|
@@ -66,22 +114,40 @@ impl ChunkMap { | |
| pub const ALLOC_TABLE: SideMetadataSpec = | ||
| crate::util::metadata::side_metadata::spec_defs::CHUNK_MARK; | ||
|
|
||
| pub fn new(space_index: usize) -> Self { | ||
| Self { | ||
| space_index, | ||
| chunk_range: Mutex::new(Chunk::ZERO..Chunk::ZERO), | ||
| } | ||
| } | ||
|
|
||
| /// Set a chunk as allocated, or as free. | ||
| pub fn set_allocated(&self, chunk: Chunk, allocated: bool) { | ||
|
There was a problem hiding this comment. I feel this function name is confusing. There was a problem hiding this comment. The boolean argument in There was a problem hiding this comment. I'm not sure why There was a problem hiding this comment. I felt after changing the argument from an enum to a boolean, There was a problem hiding this comment. Fair enough 👍 I'll wait for @wks to see if he has a better suggestion. |
||
| let state = if allocated { | ||
| ChunkState::allocated(self.space_index) | ||
| } else { | ||
| ChunkState::free(self.space_index) | ||
| }; | ||
| // Do nothing if the chunk is already in the expected state. | ||
| if self.get_any(chunk) == state { | ||
| return; | ||
| } | ||
| #[cfg(debug_assertions)] | ||
| { | ||
| let old_state = self.get_any(chunk); | ||
| // If a chunk is free, any space may use it. If a chunk is not free, only the current space may update its state. | ||
| assert!( | ||
| old_state.is_free() || old_state.get_space_index() == state.get_space_index(), | ||
| "Chunk {:?}: old state {:?}, new state {:?}. Cannot set to new state.", | ||
| chunk, | ||
| old_state, | ||
| state | ||
| ); | ||
| } | ||
| // Update alloc byte | ||
| unsafe { Self::ALLOC_TABLE.store::<u8>(chunk.start(), state.0) }; | ||
| // If this is a newly allcoated chunk, then expand the chunk range. | ||
| if state.is_allocated() { | ||
| debug_assert!(!chunk.start().is_zero()); | ||
| let mut range = self.chunk_range.lock(); | ||
| if range.start == Chunk::ZERO { | ||
|
|
@@ -96,20 +162,30 @@ impl ChunkMap { | |
| } | ||
| } | ||
|
|
||
| /// Get chunk state. Return None if the chunk does not belong to the space. | ||
| pub fn get(&self, chunk: Chunk) -> Option<ChunkState> { | ||
| let state = self.get_any(chunk); | ||
| (state.get_space_index() == self.space_index).then_some(state) | ||
| } | ||
|
|
||
| /// Get chunk state, regardless of the space. This should always be private. | ||
| fn get_any(&self, chunk: Chunk) -> ChunkState { | ||
|
There was a problem hiding this comment.
|
||
| let byte = unsafe { Self::ALLOC_TABLE.load::<u8>(chunk.start()) }; | ||
| ChunkState(byte) | ||
| } | ||
|
|
||
| /// A range of all chunks in the heap. | ||
| pub fn all_chunks(&self) -> impl Iterator<Item = Chunk> + '_ { | ||
| let chunk_range = self.chunk_range.lock(); | ||
| RegionIterator::<Chunk>::new(chunk_range.start, chunk_range.end) | ||
| .filter(|c| self.get(*c).is_some()) | ||
| } | ||
|
|
||
| /// A range of all chunks in the heap. | ||
| pub fn all_allocated_chunks(&self) -> impl Iterator<Item = Chunk> + '_ { | ||
| let chunk_range = self.chunk_range.lock(); | ||
| RegionIterator::<Chunk>::new(chunk_range.start, chunk_range.end) | ||
| .filter(|c| self.get(*c).is_some_and(|state| state.is_allocated())) | ||
| } | ||
|
Comment on lines
+177
to
189
There was a problem hiding this comment. I don't really understand why we need this. The issue here is that when a chunk is freed, we still associate a space index with it. But why is that the case? We can just have the There was a problem hiding this comment. I am just making sure There was a problem hiding this comment. I just did a grep of where the There was a problem hiding this comment. Also I don't think the above is a good idea anyway. We shouldn't be going through free chunks when clearing/setting metadata. It should almost always be the allocated chunks. Do you have a use-case in mind where it makes sense to have a space associated with a free chunk? There was a problem hiding this comment. I am not sure. I will try replace the current use of |
||
|
|
||
| /// Helper function to create per-chunk processing work packets for each allocated chunks. | ||
|
|
@@ -118,18 +194,9 @@ impl ChunkMap { | |
| func: impl Fn(Chunk) -> Box<dyn GCWork<VM>>, | ||
| ) -> Vec<Box<dyn GCWork<VM>>> { | ||
| let mut work_packets: Vec<Box<dyn GCWork<VM>>> = vec![]; | ||
| for chunk in self.all_allocated_chunks() { | ||
| work_packets.push(func(chunk)); | ||
| } | ||
| work_packets | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,10 +5,7 @@ use std::marker::PhantomData; | |
| use crate::vm::VMBinding; | ||
|
|
||
| use super::{ | ||
| heap::{chunk_map::ChunkMap, MonotonePageResource}, | ||
| linear_scan::Region, | ||
| metadata::{side_metadata::spec_defs::VO_BIT, vo_bit}, | ||
| Address, ObjectReference, | ||
|
|
@@ -84,12 +81,10 @@ pub(crate) fn enumerate_blocks_from_chunk_map<B>( | |
| ) where | ||
| B: BlockMayHaveObjects, | ||
| { | ||
| for chunk in chunk_map.all_allocated_chunks() { | ||
| for block in chunk.iter_region::<B>() { | ||
| if block.may_have_objects() { | ||
| enumerator.visit_address_range(block.start(), block.end()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.