Introduce SpaceInspector and RegionInspector #1322
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| use crate::policy::sft::SFT; | ||
| use crate::util::linear_scan::RegionIterator; | ||
| use crate::util::{linear_scan::Region, Address, ObjectReference}; | ||
| use crate::util::metadata::side_metadata::spec_defs::VO_BIT; | ||
| use crate::util::heap::chunk_map::Chunk; | ||
|
|
||
| pub trait SpaceInspector { | ||
| fn name(&self) -> &str; | ||
| fn list_regions(&self, parent_region: Option<&dyn RegionInspector>) -> Vec<Box<dyn RegionInspector>>; | ||
|
There was a problem hiding this comment. I think we can just let this generic And it is better to let the return value be There was a problem hiding this comment.
We could separate the function into two if you think it is better. One to list the top-level regions, and the other to list sub-regions of one region.
This sounds reasonable. |
||
| } | ||
|
|
||
| pub(crate) fn list_child_regions<PARENT: Region, CHILD: Region + 'static>(region: &dyn RegionInspector) -> Option<Vec<Box<dyn RegionInspector>>> { | ||
| if region.region_type() == std::any::type_name::<PARENT>() { | ||
|
There was a problem hiding this comment. There should be no need to compare types by strings because both PARENT and CHILD are statically known. The problem is that generic trait specialization is an unstable feature. There is another way to avoid comparing by string. We can let each space define a trait: trait ImmixRegionInspector: RegionIterator {
type ChildType: Region;
const HAS_CHILD_REGION: bool;
fn get_child_inspectors(&self) -> Option<dyn ImmixRegionInspector> {
if Self::HAS_CHILF_REGION { Some(...) } else { None }
}
}
impl ImmixRegionInspector for Chunk {
type ChildType = crate::policy::immix::block::Block;
const HAS_CHILD_REGION = true;
}
impl ImmixRegionInspector for crate::policy::immix::block::Block {
type ChildType = crate::policy::immix::line::Line;
const HAS_CHILD_REGION = true;
}
impl ImmixRegionInspector for crate::policy::immix::line::Line {
type ChildType = Self; // We can't say None
const HAS_CHILD_REGION = false;
}Then we will let The main difference is that each space decides which trait to downcast into. So for There was a problem hiding this comment. I do hope to avoid those policy-specific types.
We should try get a simple solution. If a string comparison works, we don't need to introduce a new trait and several impls for the trait for every policy. |
||
| let start_child_region = CHILD::from_aligned_address(region.start()); | ||
| let end_child_region = CHILD::from_aligned_address(region.start() + region.size()); | ||
| Some(RegionIterator::<CHILD>::new(start_child_region, end_child_region) | ||
| .map(|r| Box::new(r) as Box<dyn RegionInspector>) | ||
| .collect()) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
| pub trait RegionInspector { | ||
| fn region_type(&self) -> &str; | ||
|
There was a problem hiding this comment. If something other than pub trait RegionInspector {
type RegionType: Region;
fn get_region(): Self::RegionType;
}And we can probably remove |
||
| fn start(&self) -> Address; | ||
| fn size(&self) -> usize; | ||
| #[cfg(feature = "vo_bit")] | ||
| fn list_objects(&self) -> Vec<ObjectReference> { | ||
| let mut objects = vec![]; | ||
| VO_BIT.scan_non_zero_values::<u8>(self.start(), self.start() + self.size(), &mut |address| { | ||
| use crate::util::metadata::vo_bit; | ||
| let object = vo_bit::get_object_ref_for_vo_addr(address); | ||
| objects.push(object); | ||
| }); | ||
| objects | ||
| } | ||
| } | ||
|
|
||
| impl<R: Region> RegionInspector for R { | ||
| fn region_type(&self) -> &str { | ||
| std::any::type_name::<R>() | ||
| } | ||
|
|
||
| fn start(&self) -> Address { | ||
| Region::start(self) | ||
| } | ||
|
|
||
| fn size(&self) -> usize { | ||
| Self::BYTES | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // GITHUB-CI: MMTK_PLAN=Immix | ||
| // GITHUB-CI: FEATURES=vo_bit | ||
|
|
||
| use std::collections::HashSet; | ||
|
|
||
| use constants::BYTES_IN_WORD; | ||
|
|
||
| use super::mock_test_prelude::*; | ||
|
|
||
| use crate::{util::*, AllocationSemantics, MMTK}; | ||
|
|
||
| lazy_static! { | ||
| static ref FIXTURE: Fixture<MutatorFixture> = Fixture::new(); | ||
| } | ||
|
|
||
| pub fn get_all_objects(mmtk: &'static MMTK<MockVM>) -> HashSet<ObjectReference> { | ||
| let mut result = HashSet::new(); | ||
| mmtk.enumerate_objects(|object| { | ||
| result.insert(object); | ||
| }); | ||
| result | ||
| } | ||
|
|
||
| #[test] | ||
| pub fn test_heap_inspector() { | ||
| with_mockvm( | ||
| default_setup, | ||
| || { | ||
| FIXTURE.with_fixture_mut(|fixture| { | ||
| let mmtk = fixture.mmtk(); | ||
| let mutator = &mut fixture.mutator; | ||
| let space_inspector = mmtk.inspect_spaces(); | ||
| assert!(space_inspector.len() > 0); | ||
|
|
||
| let get_immix_inspector = || { | ||
| space_inspector.iter().find(|s| s.name() == "immix").unwrap() | ||
| }; | ||
|
|
||
| { | ||
| let immix_space_inspector = get_immix_inspector(); | ||
| let chunk_inspector = immix_space_inspector.list_regions(None); | ||
| assert_eq!(chunk_inspector.len(), 0); | ||
| } | ||
|
|
||
| let mut new_obj = |size: usize, semantics: AllocationSemantics| { | ||
| let align = BYTES_IN_WORD; | ||
| let start = memory_manager::alloc(mutator, size, align, 0, semantics); | ||
| let object = MockVM::object_start_to_ref(start); | ||
| memory_manager::post_alloc(mutator, object, size, semantics); | ||
| object | ||
| }; | ||
|
|
||
| // Allocate one object | ||
| let object = new_obj(40, AllocationSemantics::Default); | ||
|
|
||
| { | ||
| let immix_space_inspector = get_immix_inspector(); | ||
| // Check chunks | ||
| let chunk_inspector = immix_space_inspector.list_regions(None); | ||
| assert_eq!(chunk_inspector.len(), 1); | ||
| assert_eq!(chunk_inspector[0].region_type(), "mmtk::util::heap::chunk_map::Chunk"); | ||
| let objects = chunk_inspector[0].list_objects(); | ||
| assert_eq!(objects.len(), 1); | ||
| assert_eq!(objects[0], object); | ||
| // Check blocks | ||
| let block_inspector = immix_space_inspector.list_regions(Some(&*chunk_inspector[0])); | ||
| assert_eq!(block_inspector.len(), 128); // 128 blocks in a chunk | ||
| assert_eq!(block_inspector[0].region_type(), "mmtk::policy::immix::block::Block"); | ||
| let objects = block_inspector[0].list_objects(); | ||
| assert_eq!(objects.len(), 1); | ||
| assert_eq!(objects[0], object); | ||
| // Check lines | ||
| let line_inspector = immix_space_inspector.list_regions(Some(&*block_inspector[0])); | ||
| assert_eq!(line_inspector.len(), 128); // 128 lines in a block | ||
| assert_eq!(line_inspector[0].region_type(), "mmtk::policy::immix::line::Line"); | ||
| let objects = line_inspector[0].list_objects(); | ||
| assert_eq!(objects.len(), 1); | ||
| assert_eq!(objects[0], object); | ||
| } | ||
|
|
||
| // Allocate another object | ||
| let object2 = new_obj(40, AllocationSemantics::Default); | ||
|
|
||
| { | ||
| let immix_space_inspector = get_immix_inspector(); | ||
| // Check checks | ||
| let chunk_inspector = immix_space_inspector.list_regions(None); | ||
| assert_eq!(chunk_inspector.len(), 1); | ||
| assert_eq!(chunk_inspector[0].region_type(), "mmtk::util::heap::chunk_map::Chunk"); | ||
| let objects = chunk_inspector[0].list_objects(); | ||
| assert_eq!(objects.len(), 2); | ||
| assert_eq!(objects[0], object); | ||
| assert_eq!(objects[1], object2); | ||
| } | ||
| }); | ||
| }, | ||
| no_cleanup, | ||
| ) | ||
| } |
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.
Yes. Having a
Vecdoes simplify the API a bit because it can be iterated, and the performance doesn't matter because (1) we are inspecting, and (2) there are probably not may spaces in a plan.