diff --git a/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs b/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs index 3eee9db448..f19c1e25b0 100644 --- a/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs +++ b/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs @@ -23,7 +23,7 @@ impl crate::scheduler::GCWorkContext for MyGCWorkContext2 { type PlanType = MyGC; type ProcessEdgesWorkType = PlanProcessEdges, DEFAULT_TRACE>; } -// ANCHOR: workcontext_plan +// ANCHOR_END: workcontext_plan use crate::util::ObjectReference; use crate::util::copy::CopySemantics; diff --git a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs index 677c821a87..27d272f918 100644 --- a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs +++ b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs @@ -14,7 +14,7 @@ use crate::scheduler::*; // Modify use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; use crate::util::heap::VMRequest; -use crate::util::metadata::side_metadata::{SideMetadataSanity, SideMetadataContext}; +use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::opaque_pointer::*; use crate::vm::VMBinding; use enum_map::EnumMap; @@ -24,18 +24,20 @@ use std::sync::atomic::{AtomicBool, Ordering}; // Add // Remove #[allow(unused_imports)]. // Remove handle_user_collection_request(). -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; // Modify // ANCHOR: plan_def -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct MyGC { pub hi: AtomicBool, - #[trace(CopySemantics::DefaultCopy)] + #[space] + #[copy_semantics(CopySemantics::DefaultCopy)] pub copyspace0: CopySpace, - #[trace(CopySemantics::DefaultCopy)] + #[space] + #[copy_semantics(CopySemantics::DefaultCopy)] pub copyspace1: CopySpace, - #[fallback_trace] + #[parent] pub common: CommonPlan, } // ANCHOR_END: plan_def @@ -51,8 +53,6 @@ pub const MYGC_CONSTRAINTS: PlanConstraints = PlanConstraints { // ANCHOR_END: constraints impl Plan for MyGC { - type VM = VM; - fn constraints(&self) -> &'static PlanConstraints { &MYGC_CONSTRAINTS } @@ -74,15 +74,6 @@ impl Plan for MyGC { } // ANCHOR_END: create_copy_config - // ANCHOR: get_spaces - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.common.get_spaces(); - ret.push(&self.copyspace0); - ret.push(&self.copyspace1); - ret - } - // ANCHOR_EN: get_spaces - // Modify // ANCHOR: schedule_collection fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { @@ -188,12 +179,7 @@ impl MyGC { common: CommonPlan::new(plan_args), }; - // Use SideMetadataSanity to check if each spec is valid. This is also needed for check - // side metadata in extreme_assertions. - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - res.common.verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.copyspace0.verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.copyspace1.verify_side_metadata_sanity(&mut side_metadata_sanity_checker); + res.verify_side_metadata_sanity(); res } diff --git a/docs/userguide/src/tutorial/mygc/ss/alloc.md b/docs/userguide/src/tutorial/mygc/ss/alloc.md index fed3c4078c..1da743a41a 100644 --- a/docs/userguide/src/tutorial/mygc/ss/alloc.md +++ b/docs/userguide/src/tutorial/mygc/ss/alloc.md @@ -66,13 +66,16 @@ Finished code (step 2): {{#include ../../code/mygc_semispace/global.rs:plan_def}} ``` -Note that we have attributes on some fields. These attributes tell MMTk's macros on -how to generate code to trace objects in this plan. Although there are other approaches that -you can implement object tracing, in this tutorial we use the macros, as it is the simplest. -Make sure you import the macros. We will discuss on what those attributes mean in later sections. +Note that `MyGC` now also derives `PlanTraceObject` besides `HasSpaces`, and we +have attributes on some fields. These attributes tell MMTk's macros how to +generate code to visit each space of this plan as well as trace objects in this +plan. Although there are other approaches that you can implement object +tracing, in this tutorial we use the macros, as it is the simplest. Make sure +you import the macros. We will discuss on what those attributes mean in later +sections. ```rust -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; ``` ### Implement the Plan trait for MyGC @@ -230,4 +233,4 @@ by the common plan: With this, you should have the allocation working, but not garbage collection. Try building again. If you run HelloWorld or Fannkunchredux, they should -work. DaCapo's lusearch should fail, as it requires garbage to be collected. \ No newline at end of file +work. DaCapo's lusearch should fail, as it requires garbage to be collected. diff --git a/docs/userguide/src/tutorial/mygc/ss/collection.md b/docs/userguide/src/tutorial/mygc/ss/collection.md index 48722de3c4..af5e123e65 100644 --- a/docs/userguide/src/tutorial/mygc/ss/collection.md +++ b/docs/userguide/src/tutorial/mygc/ss/collection.md @@ -181,11 +181,19 @@ it can use `PlanProcessEdges`. You can manually provide an implementation of `PlanTraceObject` for `MyGC`. But you can also use the derive macro MMTK provides, and the macro will generate an implementation of `PlanTraceObject`: -* add `#[derive(PlanTraceObject)]` for `MyGC` (import the macro properly: `use mmtk_macros::PlanTraceObject`) -* add `#[trace(CopySemantics::Default)]` to both copy space fields, `copyspace0` and `copyspace1`. This tells the macro to generate - trace code for both spaces, and for any copying in the spaces, use `CopySemantics::DefaultCopy` that we have configured early. -* add `#[fallback_trace]` to `common`. This tells the macro that if an object is not found in any space with `#[trace]` in ths plan, - try find the space for the object in the 'parent' plan. In our case, we fall back to the `CommonPlan`, as the object may be + +* Make sure `MyGC` already has the `#[derive(HasSpaces)]` attribute because all plans need to + implement the `HasSpaces` trait anyway. (import the macro properly: `use mmtk_macros::HasSpaces`) +* Add `#[derive(PlanTraceObject)]` for `MyGC` (import the macro properly: `use mmtk_macros::PlanTraceObject`) +* Add both `#[space]` and `#[copy_semantics(CopySemantics::Default)]` to both copy space fields, + `copyspace0` and `copyspace1`. `#[space]` tells the macro that both `copyspace0` and `copyspace1` + are spaces in the `MyGC` plan, and the generated trace code will check both spaces. + `#[copy_semantics(CopySemantics::DefaultCopy)]` specifies the copy semantics to use when tracing + objects in the corresponding space. +* Add `#[parent]` to `common`. This tells the macro that there are more spaces defined in `common` + and its nested structs. If an object is not found in any space with `#[space]` in this plan, + the trace code will try to find the space for the object in the 'parent' plan. In our case, the + trace code will proceed by checking spaces in the `CommonPlan`, as the object may be in large object space or immortal space in the common plan. `CommonPlan` also implements `PlanTraceObject`, so it knows how to find a space for the object and trace it in the same way. @@ -238,7 +246,7 @@ In the end, use `MyGCProcessEdges` as `ProcessEdgesWorkType` in the `GCWorkConte ## Summary You should now have MyGC working and able to collect garbage. All three -benchmarks should be able to pass now. +benchmarks should be able to pass now. If the benchmarks pass - good job! You have built a functional copying collector! diff --git a/macros/Cargo.toml b/macros/Cargo.toml index 0600ded07d..9928c78563 100644 --- a/macros/Cargo.toml +++ b/macros/Cargo.toml @@ -13,6 +13,6 @@ proc-macro = true [dependencies] proc-macro2 = "1.0.37" -syn = { version = "1.0.91", features = ["extra-traits"] } +syn = { version = "2.0.29", features = ["extra-traits"] } quote = "1.0.18" proc-macro-error = "1.0.4" diff --git a/macros/src/has_spaces_impl.rs b/macros/src/has_spaces_impl.rs new file mode 100644 index 0000000000..dbf6da6ed3 --- /dev/null +++ b/macros/src/has_spaces_impl.rs @@ -0,0 +1,82 @@ +use proc_macro2::TokenStream as TokenStream2; +use proc_macro_error::abort_call_site; +use quote::quote; +use syn::{DeriveInput, Field}; + +use crate::util; + +pub(crate) fn derive(input: DeriveInput) -> TokenStream2 { + let ident = input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + + let syn::Data::Struct(syn::DataStruct { + fields: syn::Fields::Named(ref fields), + .. + }) = input.data else { + abort_call_site!("`#[derive(HasSpaces)]` only supports structs with named fields."); + }; + + let spaces = util::get_fields_with_attribute(fields, "space"); + let parent = util::get_unique_field_with_attribute(fields, "parent"); + + let items = generate_impl_items(&spaces, &parent); + + quote! { + impl #impl_generics crate::plan::HasSpaces for #ident #ty_generics #where_clause { + type VM = VM; + + #items + } + } +} + +pub(crate) fn generate_impl_items<'a>( + space_fields: &[&'a Field], + parent_field: &Option<&'a Field>, +) -> TokenStream2 { + // Currently we implement callback-style visitor methods. + // Iterators should be more powerful, but is more difficult to implement. + + let mut space_visitors = vec![]; + let mut space_visitors_mut = vec![]; + + for f in space_fields { + let f_ident = f.ident.as_ref().unwrap(); + + let visitor = quote! { + __func(&self.#f_ident); + }; + + let visitor_mut = quote! { + __func(&mut self.#f_ident); + }; + + space_visitors.push(visitor); + space_visitors_mut.push(visitor_mut); + } + + let (parent_visitor, parent_visitor_mut) = if let Some(f) = parent_field { + let f_ident = f.ident.as_ref().unwrap(); + let visitor = quote! { + self.#f_ident.for_each_space(__func) + }; + let visitor_mut = quote! { + self.#f_ident.for_each_space_mut(__func) + }; + (visitor, visitor_mut) + } else { + (quote! {}, quote! {}) + }; + + quote! { + fn for_each_space(&self, __func: &mut dyn FnMut(&dyn Space)) { + #(#space_visitors)* + #parent_visitor + } + + fn for_each_space_mut(&mut self, __func: &mut dyn FnMut(&mut dyn Space)) { + #(#space_visitors_mut)* + #parent_visitor_mut + } + } +} diff --git a/macros/src/lib.rs b/macros/src/lib.rs index 53f6b544a6..64b08ee9b6 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -4,66 +4,60 @@ extern crate quote; extern crate syn; use proc_macro::TokenStream; -use proc_macro_error::abort_call_site; use proc_macro_error::proc_macro_error; -use quote::quote; use syn::parse_macro_input; use syn::DeriveInput; +mod has_spaces_impl; mod plan_trace_object_impl; mod util; const DEBUG_MACRO_OUTPUT: bool = false; -/// Generally a plan needs to add these attributes in order for the macro to work. The macro will -/// generate an implementation of `PlanTraceObject` for the plan. With `PlanTraceObject`, the plan use -/// `PlanProcessEdges` for GC tracing. The attributes only affects code generation in the macro, thus -/// only affects the generated `PlanTraceObject` implementation. -/// * add `#[derive(PlanTraceObject)]` to the plan struct. -/// * add `#[trace]` to each space field the plan struct has. If the policy is a copying policy, -/// it needs to further specify the copy semantic (`#[trace(CopySemantics::X)]`) -/// * add `#[fallback_trace]` to the parent plan if the plan is composed with other plans (or parent plans). -/// For example, `GenImmix` is composed with `Gen`, `Gen` is composed with `CommonPlan`, `CommonPlan` is composed -/// with `BasePlan`. -/// * add `#[post_scan]` to any space field that has some policy-specific post_scan_object(). For objects in those spaces, -/// `post_scan_object()` in the policy will be called after `VM::VMScanning::scan_object()`. +/// This macro will generate an implementation of `HasSpaces` for a plan or any structs that +/// contain spaces, including `Gen`, `CommonPlan` and `BasePlan`. +/// +/// The `HasSpaces` trait is responsible for enumerating spaces in a struct. When using this +/// derive macro, the user should do the following. +/// +/// * Make sure the struct has a generic type parameter named `VM` which requires `VMBinding`. +/// For example, `struct MyPlan` will work. +/// * Add `#[space]` for each space field in the struct. +/// * Add `#[parent]` to the field that contain more space fields. This attribute is usually +/// added to `Gen`, `CommonPlan` or `BasePlan` fields. There can be at most one parent in +/// a struct. #[proc_macro_error] -#[proc_macro_derive(PlanTraceObject, attributes(trace, post_scan, fallback_trace))] -pub fn derive_plan_trace_object(input: TokenStream) -> TokenStream { +#[proc_macro_derive(HasSpaces, attributes(space, parent))] +pub fn derive_has_spaces(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); - let ident = input.ident; - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); - - let output = if let syn::Data::Struct(syn::DataStruct { - fields: syn::Fields::Named(ref fields), - .. - }) = input.data - { - let spaces = util::get_fields_with_attribute(fields, "trace"); - let post_scan_spaces = util::get_fields_with_attribute(fields, "post_scan"); - let fallback = util::get_unique_field_with_attribute(fields, "fallback_trace"); + let output = has_spaces_impl::derive(input); - let trace_object_function = - plan_trace_object_impl::generate_trace_object(&spaces, &fallback, &ty_generics); - let post_scan_object_function = plan_trace_object_impl::generate_post_scan_object( - &post_scan_spaces, - &fallback, - &ty_generics, - ); - let may_move_objects_function = - plan_trace_object_impl::generate_may_move_objects(&spaces, &fallback, &ty_generics); - quote! { - impl #impl_generics crate::plan::PlanTraceObject #ty_generics for #ident #ty_generics #where_clause { - #trace_object_function - - #post_scan_object_function + output.into() +} - #may_move_objects_function - } - } - } else { - abort_call_site!("`#[derive(PlanTraceObject)]` only supports structs with named fields.") - }; +/// The macro will generate an implementation of `PlanTraceObject` for the plan. With +/// `PlanTraceObject`, the plan will be able to use `PlanProcessEdges` for GC tracing. +/// +/// The user should add `#[space]` and `#[parent]` attributes to fields as specified by the +/// `HasSpaces` trait. When using this derive macro, all spaces must implement the +/// `PolicyTraceObject` trait. The generated `trace_object` method will check for spaces in the +/// current plan and, if the object is not in any of them, check for plans in the parent struct. +/// The parent struct must also implement the `PlanTraceObject` trait. +/// +/// In addition, the user can add the following attributes to fields in order to control the +/// behavior of the generated `trace_object` method. +/// +/// * Add `#[copy_semantics(CopySemantics::X)]` to a space field to specify that when tracing +/// objects in that space, `Some(CopySemantics::X)` will be passed to the `Space::trace_object` +/// method as the `copy` argument. +/// * Add `#[post_scan]` to any space field that has some policy-specific `post_scan_object()`. For +/// objects in those spaces, `post_scan_object()` in the policy will be called after +/// `VM::VMScanning::scan_object()`. +#[proc_macro_error] +#[proc_macro_derive(PlanTraceObject, attributes(space, parent, copy_semantics, post_scan))] +pub fn derive_plan_trace_object(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + let output = plan_trace_object_impl::derive(input); // Debug the output - use the following code to debug the generated code (when cargo exapand is not working) if DEBUG_MACRO_OUTPUT { diff --git a/macros/src/plan_trace_object_impl.rs b/macros/src/plan_trace_object_impl.rs index 5dd0b045dc..b5241bfd04 100644 --- a/macros/src/plan_trace_object_impl.rs +++ b/macros/src/plan_trace_object_impl.rs @@ -1,9 +1,41 @@ use proc_macro2::TokenStream as TokenStream2; +use proc_macro_error::abort_call_site; use quote::quote; -use syn::{Field, TypeGenerics}; +use syn::{DeriveInput, Expr, Field, TypeGenerics}; use crate::util; +pub(crate) fn derive(input: DeriveInput) -> TokenStream2 { + let ident = input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + + let syn::Data::Struct(syn::DataStruct { + fields: syn::Fields::Named(ref fields), + .. + }) = input.data else { + abort_call_site!("`#[derive(PlanTraceObject)]` only supports structs with named fields."); + }; + + let spaces = util::get_fields_with_attribute(fields, "space"); + let post_scan_spaces = util::get_fields_with_attribute(fields, "post_scan"); + let parent = util::get_unique_field_with_attribute(fields, "parent"); + + let trace_object_function = generate_trace_object(&spaces, &parent, &ty_generics); + let post_scan_object_function = + generate_post_scan_object(&post_scan_spaces, &parent, &ty_generics); + let may_move_objects_function = generate_may_move_objects(&spaces, &parent, &ty_generics); + + quote! { + impl #impl_generics crate::plan::PlanTraceObject #ty_generics for #ident #ty_generics #where_clause { + #trace_object_function + + #post_scan_object_function + + #may_move_objects_function + } + } +} + pub(crate) fn generate_trace_object<'a>( space_fields: &[&'a Field], parent_field: &Option<&'a Field>, @@ -15,21 +47,26 @@ pub(crate) fn generate_trace_object<'a>( let f_ty = &f.ty; // Figure out copy - let trace_attr = util::get_field_attribute(f, "trace").unwrap(); - let copy = if !trace_attr.tokens.is_empty() { - use syn::Token; - use syn::NestedMeta; - use syn::punctuated::Punctuated; - - let args = trace_attr.parse_args_with(Punctuated::::parse_terminated).unwrap(); - // CopySemantics::X is a path. - if let Some(NestedMeta::Meta(syn::Meta::Path(p))) = args.first() { - quote!{ Some(#p) } - } else { - quote!{ None } + let maybe_copy_semantics_attr = util::get_field_attribute(f, "copy_semantics"); + let copy = match maybe_copy_semantics_attr { + None => quote!{ None }, + Some(attr) => match &attr.meta { + syn::Meta::Path(_) => { + // #[copy_semantics] + abort_call_site!("The `#[copy_semantics(expr)]` macro needs an argument."); + }, + syn::Meta::List(list) => { + // #[copy_semantics(BlahBlah)] + let copy_semantics = list.parse_args::().unwrap_or_else(|_| { + abort_call_site!("In `#[copy_semantics(expr)]`, expr must be an expression."); + }); + quote!{ Some(#copy_semantics) } + }, + syn::Meta::NameValue(_) => { + // #[copy_semantics = BlahBlah] + abort_call_site!("The #[copy_semantics] macro does not support the name-value form."); + }, } - } else { - quote!{ None } }; quote! { diff --git a/macros/src/util.rs b/macros/src/util.rs index ed6f53d880..19e9249338 100644 --- a/macros/src/util.rs +++ b/macros/src/util.rs @@ -5,11 +5,11 @@ pub fn get_field_attribute<'f>(field: &'f Field, attr_name: &str) -> Option<&'f let attrs = field .attrs .iter() - .filter(|a| a.path.is_ident(attr_name)) + .filter(|a| a.path().is_ident(attr_name)) .collect::>(); if attrs.len() > 1 { let second_attr = attrs.get(1).unwrap(); - abort! { second_attr.path.span(), "Duplicated attribute: #[{}]", attr_name } + abort! { second_attr.path().span(), "Duplicated attribute: #[{}]", attr_name } }; attrs.get(0).cloned() @@ -35,7 +35,7 @@ pub fn get_unique_field_with_attribute<'f>( result = Some(field); continue; } else { - let span = attr.path.span(); + let span = attr.path().span(); abort! { span, "At most one field in a struct can have the #[{}] attribute.", attr_name }; } } diff --git a/src/memory_manager.rs b/src/memory_manager.rs index 8383a7aa72..fc9d6b8c42 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -92,7 +92,10 @@ pub fn mmtk_init(builder: &MMTKBuilder) -> Box> { /// Currently we do not allow removing regions from VM space. #[cfg(feature = "vm_space")] pub fn set_vm_space(mmtk: &'static mut MMTK, start: Address, size: usize) { - unsafe { mmtk.get_plan_mut() }.base_mut().vm_space.set_vm_region(start, size); + unsafe { mmtk.get_plan_mut() } + .base_mut() + .vm_space + .set_vm_region(start, size); } /// Request MMTk to create a mutator for the given thread. The ownership diff --git a/src/plan/generational/copying/global.rs b/src/plan/generational/copying/global.rs index 7c7f2409bc..7f5d2f4c97 100644 --- a/src/plan/generational/copying/global.rs +++ b/src/plan/generational/copying/global.rs @@ -18,7 +18,6 @@ use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; use crate::util::heap::VMRequest; -use crate::util::metadata::side_metadata::SideMetadataSanity; use crate::util::Address; use crate::util::ObjectReference; use crate::util::VMWorkerThread; @@ -27,24 +26,24 @@ use crate::ObjectQueue; use enum_map::EnumMap; use std::sync::atomic::{AtomicBool, Ordering}; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct GenCopy { - #[fallback_trace] + #[parent] pub gen: CommonGenPlan, pub hi: AtomicBool, - #[trace(CopySemantics::Mature)] + #[space] + #[copy_semantics(CopySemantics::Mature)] pub copyspace0: CopySpace, - #[trace(CopySemantics::Mature)] + #[space] + #[copy_semantics(CopySemantics::Mature)] pub copyspace1: CopySpace, } pub const GENCOPY_CONSTRAINTS: PlanConstraints = crate::plan::generational::GEN_CONSTRAINTS; impl Plan for GenCopy { - type VM = VM; - fn constraints(&self) -> &'static PlanConstraints { &GENCOPY_CONSTRAINTS } @@ -72,13 +71,6 @@ impl Plan for GenCopy { self.gen.collection_required(self, space_full, space) } - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.gen.get_spaces(); - ret.push(&self.copyspace0); - ret.push(&self.copyspace1); - ret - } - fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { let is_full_heap = self.requires_full_heap_collection(); self.base().set_collection_kind::(self); @@ -227,17 +219,7 @@ impl GenCopy { copyspace1, }; - // Use SideMetadataSanity to check if each spec is valid. This is also needed for check - // side metadata in extreme_assertions. - { - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - res.gen - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.copyspace0 - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.copyspace1 - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - } + res.verify_side_metadata_sanity(); res } diff --git a/src/plan/generational/global.rs b/src/plan/generational/global.rs index 1250278ccf..4db35d8f58 100644 --- a/src/plan/generational/global.rs +++ b/src/plan/generational/global.rs @@ -7,7 +7,6 @@ use crate::policy::space::Space; use crate::scheduler::*; use crate::util::copy::CopySemantics; use crate::util::heap::VMRequest; -use crate::util::metadata::side_metadata::SideMetadataSanity; use crate::util::statistics::counter::EventCounter; use crate::util::Address; use crate::util::ObjectReference; @@ -17,17 +16,18 @@ use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; /// Common implementation for generational plans. Each generational plan /// should include this type, and forward calls to it where possible. -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct CommonGenPlan { /// The nursery space. - #[trace(CopySemantics::PromoteToMature)] + #[space] + #[copy_semantics(CopySemantics::PromoteToMature)] pub nursery: CopySpace, /// The common plan. - #[fallback_trace] + #[parent] pub common: CommonPlan, /// Is this GC full heap? pub gc_full_heap: AtomicBool, @@ -59,19 +59,6 @@ impl CommonGenPlan { } } - /// Verify side metadata specs used in the spaces in Gen. - pub fn verify_side_metadata_sanity(&self, sanity: &mut SideMetadataSanity) { - self.common.verify_side_metadata_sanity(sanity); - self.nursery.verify_side_metadata_sanity(sanity); - } - - /// Get spaces in generation plans - pub fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.common.get_spaces(); - ret.push(&self.nursery); - ret - } - /// Prepare Gen. This should be called by a single thread in GC prepare work. pub fn prepare(&mut self, tls: VMWorkerThread) { let full_heap = !self.is_current_gc_nursery(); diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs index 70f8bd3351..3373688157 100644 --- a/src/plan/generational/immix/global.rs +++ b/src/plan/generational/immix/global.rs @@ -29,20 +29,21 @@ use enum_map::EnumMap; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; /// Generational immix. This implements the functionality of a two-generation copying /// collector where the higher generation is an immix space. /// See the PLDI'08 paper by Blackburn and McKinley for a description /// of the algorithm: . -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct GenImmix { /// Generational plan, which includes a nursery space and operations related with nursery. - #[fallback_trace] + #[parent] pub gen: CommonGenPlan, /// An immix space as the mature space. #[post_scan] - #[trace(CopySemantics::Mature)] + #[space] + #[copy_semantics(CopySemantics::Mature)] pub immix_space: ImmixSpace, /// Whether the last GC was a defrag GC for the immix space. pub last_gc_was_defrag: AtomicBool, @@ -65,8 +66,6 @@ pub const GENIMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for GenImmix { - type VM = VM; - fn constraints(&self) -> &'static PlanConstraints { &GENIMMIX_CONSTRAINTS } @@ -98,12 +97,6 @@ impl Plan for GenImmix { self.gen.collection_required(self, space_full, space) } - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.gen.get_spaces(); - ret.push(&self.immix_space); - ret - } - // GenImmixMatureProcessEdges and GenImmixMatureProcessEdges // are different types. However, it seems clippy does not recognize the constant type parameter and thinks we have identical blocks // in different if branches. @@ -259,18 +252,7 @@ impl GenImmix { last_gc_was_full_heap: AtomicBool::new(false), }; - // Use SideMetadataSanity to check if each spec is valid. This is also needed for check - // side metadata in extreme_assertions. - { - use crate::util::metadata::side_metadata::SideMetadataSanity; - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - genimmix - .gen - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - genimmix - .immix_space - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - } + genimmix.verify_side_metadata_sanity(); genimmix } diff --git a/src/plan/global.rs b/src/plan/global.rs index 941089430f..c230c5891b 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -33,7 +33,7 @@ use enum_map::EnumMap; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; pub fn create_mutator( tls: VMMutatorThread, @@ -127,9 +127,7 @@ pub fn create_plan( } // Each space now has a fixed address for its lifetime. It is safe now to initialize SFT. - plan.get_spaces() - .into_iter() - .for_each(|s| s.initialize_sft()); + plan.for_each_space(&mut |s| s.initialize_sft()); plan } @@ -156,7 +154,8 @@ pub fn create_gc_worker_context( /// 2. Create a vector of all the side metadata specs with `SideMetadataContext::new_global_specs()`, /// the parameter is a vector of global side metadata specs that are specific to the plan. /// 3. Initialize all the spaces the plan uses with the heap meta, and the global metadata specs vector. -/// 4. Create a `SideMetadataSanity` object, and invoke verify_side_metadata_sanity() for each space (or +/// 4. Invoke the `verify_side_metadata_sanity()` method of the plan. +/// It will create a `SideMetadataSanity` object, and invoke verify_side_metadata_sanity() for each space (or /// invoke verify_side_metadata_sanity() in `CommonPlan`/`BasePlan` for the spaces in the common/base plan). /// /// Methods in this trait: @@ -167,9 +166,7 @@ pub fn create_gc_worker_context( /// We should avoid having methods with the same name in both Plan and BasePlan, as this may confuse people, and /// they may call a wrong method by mistake. // TODO: Some methods that are not overriden can be moved from the trait to BasePlan. -pub trait Plan: 'static + Sync + Downcast { - type VM: VMBinding; - +pub trait Plan: 'static + HasSpaces + Sync + Downcast { fn constraints(&self) -> &'static PlanConstraints; /// Create a copy config for this plan. A copying GC plan MUST override this method, @@ -194,9 +191,6 @@ pub trait Plan: 'static + Sync + Downcast { &self.base().options } - /// get all the spaces in the plan - fn get_spaces(&self) -> Vec<&dyn Space>; - fn get_allocator_mapping(&self) -> &'static EnumMap; #[cfg(feature = "sanity")] @@ -371,6 +365,14 @@ pub trait Plan: 'static + Sync + Downcast { fn sanity_check_object(&self, _object: ObjectReference) -> bool { true } + + /// Call `space.verify_side_metadata_sanity` for all spaces in this plan. + fn verify_side_metadata_sanity(&self) { + let mut side_metadata_sanity_checker = SideMetadataSanity::new(); + self.for_each_space(&mut |space| { + space.verify_side_metadata_sanity(&mut side_metadata_sanity_checker); + }) + } } impl_downcast!(Plan assoc VM); @@ -385,7 +387,7 @@ pub enum GcStatus { /** BasePlan should contain all plan-related state and functions that are _fundamental_ to _all_ plans. These include VM-specific (but not plan-specific) features such as a code space or vm space, which are fundamental to all plans for a given VM. Features that are common to _many_ (but not intrinsically _all_) plans should instead be included in CommonPlan. */ -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct BasePlan { /// Whether MMTk is now ready for collection. This is set to true when initialize_collection() is called. pub initialized: AtomicBool, @@ -431,13 +433,13 @@ pub struct BasePlan { // Spaces in base plan #[cfg(feature = "code_space")] - #[trace] + #[space] pub code_space: ImmortalSpace, #[cfg(feature = "code_space")] - #[trace] + #[space] pub code_lo_space: ImmortalSpace, #[cfg(feature = "ro_space")] - #[trace] + #[space] pub ro_space: ImmortalSpace, /// A VM space is a space allocated and populated by the VM. Currently it is used by JikesRVM @@ -453,7 +455,7 @@ pub struct BasePlan { /// - The `is_in_mmtk_spaces` currently returns `true` if the given object reference is in /// the VM space. #[cfg(feature = "vm_space")] - #[trace] + #[space] pub vm_space: VMSpace, } @@ -566,19 +568,6 @@ impl BasePlan { } } - pub fn get_spaces(&self) -> Vec<&dyn Space> { - vec![ - #[cfg(feature = "code_space")] - &self.code_space, - #[cfg(feature = "code_space")] - &self.code_lo_space, - #[cfg(feature = "ro_space")] - &self.ro_space, - #[cfg(feature = "vm_space")] - &self.vm_space, - ] - } - /// The application code has requested a collection. pub fn handle_user_collection_request(&self, tls: VMMutatorThread, force: bool) { if force || !*self.options.ignore_system_gc { @@ -862,23 +851,6 @@ impl BasePlan { space_full || stress_force_gc || heap_full } - #[allow(unused_variables)] // depending on the enabled features, base may not be used. - #[allow(clippy::needless_pass_by_ref_mut)] // depending on the enabled features, base may not be used. - pub(crate) fn verify_side_metadata_sanity( - &self, - side_metadata_sanity_checker: &mut SideMetadataSanity, - ) { - #[cfg(feature = "code_space")] - self.code_space - .verify_side_metadata_sanity(side_metadata_sanity_checker); - #[cfg(feature = "ro_space")] - self.ro_space - .verify_side_metadata_sanity(side_metadata_sanity_checker); - #[cfg(feature = "vm_space")] - 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); @@ -896,16 +868,16 @@ impl BasePlan { /** CommonPlan is for representing state and features used by _many_ plans, but that are not fundamental to _all_ plans. Examples include the Large Object Space and an Immortal space. Features that are fundamental to _all_ plans must be included in BasePlan. */ -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct CommonPlan { - #[trace] + #[space] pub immortal: ImmortalSpace, - #[trace] + #[space] pub los: LargeObjectSpace, // TODO: We should use a marksweep space for nonmoving. - #[trace] + #[space] pub nonmoving: ImmortalSpace, - #[fallback_trace] + #[parent] pub base: BasePlan, } @@ -930,14 +902,6 @@ impl CommonPlan { } } - pub fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.base.get_spaces(); - ret.push(&self.immortal); - ret.push(&self.los); - ret.push(&self.nonmoving); - ret - } - pub fn get_used_pages(&self) -> usize { self.immortal.reserved_pages() + self.los.reserved_pages() @@ -995,25 +959,42 @@ impl CommonPlan { pub fn get_nonmoving(&self) -> &ImmortalSpace { &self.nonmoving } - - pub(crate) fn verify_side_metadata_sanity( - &self, - side_metadata_sanity_checker: &mut SideMetadataSanity, - ) { - self.base - .verify_side_metadata_sanity(side_metadata_sanity_checker); - self.immortal - .verify_side_metadata_sanity(side_metadata_sanity_checker); - self.los - .verify_side_metadata_sanity(side_metadata_sanity_checker); - self.nonmoving - .verify_side_metadata_sanity(side_metadata_sanity_checker); - } } use crate::policy::gc_work::TraceKind; use crate::vm::VMBinding; +/// A trait for anything that contains spaces. +/// Examples include concrete plans as well as `Gen`, `CommonPlan` and `BasePlan`. +/// All plans must implement this trait. +/// +/// This trait provides methods for enumerating spaces in a struct, including spaces in nested +/// struct. +/// +/// This trait can be implemented automatically by adding the `#[derive(HasSpaces)]` attribute to a +/// struct. It uses the derive macro defined in the `mmtk-macros` crate. +/// +/// This trait visits spaces as `dyn`, so it should only be used when performance is not critical. +/// For performance critical methods that visit spaces in a plan, such as `trace_object`, it is +/// recommended to define a trait (such as `PlanTraceObject`) for concrete plans to implement, and +/// implement (by hand or automatically) the method without `dyn`. +pub trait HasSpaces { + // The type of the VM. + type VM: VMBinding; + + /// Visit each space field immutably. + /// + /// If `Self` contains nested fields that contain more spaces, this method shall visit spaces + /// in the outer struct first. + fn for_each_space(&self, func: &mut dyn FnMut(&dyn Space)); + + /// Visit each space field mutably. + /// + /// If `Self` contains nested fields that contain more spaces, this method shall visit spaces + /// in the outer struct first. + fn for_each_space_mut(&mut self, func: &mut dyn FnMut(&mut dyn Space)); +} + /// A plan that uses `PlanProcessEdges` needs to provide an implementation for this trait. /// Generally a plan does not need to manually implement this trait. Instead, we provide /// a procedural macro that helps generate an implementation. Please check `macros/trace_object`. diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs index d9c2682f50..b4ed586087 100644 --- a/src/plan/immix/global.rs +++ b/src/plan/immix/global.rs @@ -16,7 +16,6 @@ use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; use crate::util::heap::VMRequest; use crate::util::metadata::side_metadata::SideMetadataContext; -use crate::util::metadata::side_metadata::SideMetadataSanity; use crate::vm::VMBinding; use crate::{policy::immix::ImmixSpace, util::opaque_pointer::VMWorkerThread}; use std::sync::atomic::AtomicBool; @@ -24,14 +23,15 @@ use std::sync::atomic::AtomicBool; use atomic::Ordering; use enum_map::EnumMap; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct Immix { #[post_scan] - #[trace(CopySemantics::DefaultCopy)] + #[space] + #[copy_semantics(CopySemantics::DefaultCopy)] pub immix_space: ImmixSpace, - #[fallback_trace] + #[parent] pub common: CommonPlan, last_gc_was_defrag: AtomicBool, } @@ -47,8 +47,6 @@ pub const IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for Immix { - type VM = VM; - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { self.base().collection_required(self, space_full) } @@ -73,12 +71,6 @@ impl Plan for Immix { } } - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.common.get_spaces(); - ret.push(&self.immix_space); - ret - } - fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { self.base().set_collection_kind::(self); self.base().set_gc_status(GcStatus::GcPrepare); @@ -156,15 +148,7 @@ impl Immix { last_gc_was_defrag: AtomicBool::new(false), }; - { - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - immix - .common - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - immix - .immix_space - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - } + immix.verify_side_metadata_sanity(); immix } diff --git a/src/plan/markcompact/global.rs b/src/plan/markcompact/global.rs index 96117e6a39..9f524259e5 100644 --- a/src/plan/markcompact/global.rs +++ b/src/plan/markcompact/global.rs @@ -17,7 +17,7 @@ use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::CopySemantics; use crate::util::heap::VMRequest; -use crate::util::metadata::side_metadata::{SideMetadataContext, SideMetadataSanity}; +use crate::util::metadata::side_metadata::SideMetadataContext; #[cfg(not(feature = "vo_bit"))] use crate::util::metadata::vo_bit::VO_BIT_SIDE_METADATA_SPEC; use crate::util::opaque_pointer::*; @@ -25,13 +25,14 @@ use crate::vm::VMBinding; use enum_map::EnumMap; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct MarkCompact { - #[trace(CopySemantics::DefaultCopy)] + #[space] + #[copy_semantics(CopySemantics::DefaultCopy)] pub mc_space: MarkCompactSpace, - #[fallback_trace] + #[parent] pub common: CommonPlan, } @@ -45,18 +46,10 @@ pub const MARKCOMPACT_CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for MarkCompact { - type VM = VM; - fn constraints(&self) -> &'static PlanConstraints { &MARKCOMPACT_CONSTRAINTS } - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.common.get_spaces(); - ret.push(&self.mc_space); - ret - } - fn base(&self) -> &BasePlan { &self.common.base } @@ -208,13 +201,7 @@ impl MarkCompact { common: CommonPlan::new(plan_args), }; - // Use SideMetadataSanity to check if each spec is valid. This is also needed for check - // side metadata in extreme_assertions. - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - res.common - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.mc_space - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); + res.verify_side_metadata_sanity(); res } diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index cbb31a5284..34a59f5a10 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -12,11 +12,11 @@ use crate::policy::space::Space; use crate::scheduler::GCWorkScheduler; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::heap::VMRequest; -use crate::util::metadata::side_metadata::{SideMetadataContext, SideMetadataSanity}; +use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::VMWorkerThread; use crate::vm::VMBinding; use enum_map::EnumMap; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; #[cfg(feature = "malloc_mark_sweep")] pub type MarkSweepSpace = crate::policy::marksweepspace::malloc_ms::MallocSpace; @@ -28,11 +28,11 @@ pub type MarkSweepSpace = crate::policy::marksweepspace::native_ms::MarkSwee #[cfg(not(feature = "malloc_mark_sweep"))] use crate::policy::marksweepspace::native_ms::MAX_OBJECT_SIZE; -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct MarkSweep { - #[fallback_trace] + #[parent] common: CommonPlan, - #[trace] + #[space] ms: MarkSweepSpace, } @@ -47,14 +47,6 @@ pub const MS_CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for MarkSweep { - type VM = VM; - - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.common.get_spaces(); - ret.push(&self.ms); - ret - } - fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { self.base().set_collection_kind::(self); self.base().set_gc_status(GcStatus::GcPrepare); @@ -120,11 +112,8 @@ impl MarkSweep { common: CommonPlan::new(plan_args), }; - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - res.common - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.ms - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); + res.verify_side_metadata_sanity(); + res } diff --git a/src/plan/mod.rs b/src/plan/mod.rs index b2c415464d..c338fd4384 100644 --- a/src/plan/mod.rs +++ b/src/plan/mod.rs @@ -24,6 +24,7 @@ pub(crate) use global::create_mutator; pub(crate) use global::create_plan; pub use global::AllocationSemantics; pub(crate) use global::GcStatus; +pub(crate) use global::HasSpaces; pub use global::Plan; pub(crate) use global::PlanTraceObject; diff --git a/src/plan/nogc/global.rs b/src/plan/nogc/global.rs index ed0fd5266a..057d1ba5b9 100644 --- a/src/plan/nogc/global.rs +++ b/src/plan/nogc/global.rs @@ -11,40 +11,36 @@ use crate::scheduler::GCWorkScheduler; use crate::util::alloc::allocators::AllocatorSelector; #[allow(unused_imports)] use crate::util::heap::VMRequest; -use crate::util::metadata::side_metadata::{SideMetadataContext, SideMetadataSanity}; +use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::opaque_pointer::*; use crate::vm::VMBinding; use enum_map::EnumMap; +use mmtk_macros::HasSpaces; #[cfg(not(feature = "nogc_lock_free"))] use crate::policy::immortalspace::ImmortalSpace as NoGCImmortalSpace; #[cfg(feature = "nogc_lock_free")] use crate::policy::lockfreeimmortalspace::LockFreeImmortalSpace as NoGCImmortalSpace; +#[derive(HasSpaces)] pub struct NoGC { + #[parent] pub base: BasePlan, + #[space] pub nogc_space: NoGCImmortalSpace, + #[space] pub immortal: ImmortalSpace, + #[space] pub los: ImmortalSpace, } pub const NOGC_CONSTRAINTS: PlanConstraints = PlanConstraints::default(); impl Plan for NoGC { - type VM = VM; - fn constraints(&self) -> &'static PlanConstraints { &NOGC_CONSTRAINTS } - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.base.get_spaces(); - ret.push(&self.nogc_space); - ret.push(&self.immortal); - ret.push(&self.los); - ret - } - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { self.base().collection_required(self, space_full) } @@ -117,13 +113,7 @@ impl NoGC { base: BasePlan::new(plan_args), }; - // Use SideMetadataSanity to check if each spec is valid. This is also needed for check - // side metadata in extreme_assertions. - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - res.base() - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.nogc_space - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); + res.verify_side_metadata_sanity(); res } diff --git a/src/plan/pageprotect/global.rs b/src/plan/pageprotect/global.rs index 5c812c14e9..1e8021fae8 100644 --- a/src/plan/pageprotect/global.rs +++ b/src/plan/pageprotect/global.rs @@ -18,12 +18,13 @@ use crate::{ }; use enum_map::EnumMap; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct PageProtect { - #[trace] + #[space] pub space: LargeObjectSpace, + #[parent] pub common: CommonPlan, } @@ -33,18 +34,10 @@ pub const CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for PageProtect { - type VM = VM; - fn constraints(&self) -> &'static PlanConstraints { &CONSTRAINTS } - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.common.get_spaces(); - ret.push(&self.space); - ret - } - fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { self.base().set_collection_kind::(self); self.base().set_gc_status(GcStatus::GcPrepare); @@ -114,16 +107,7 @@ impl PageProtect { common: CommonPlan::new(plan_args), }; - // Use SideMetadataSanity to check if each spec is valid. This is also needed for check - // side metadata in extreme_assertions. - { - use crate::util::metadata::side_metadata::SideMetadataSanity; - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - ret.common - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - ret.space - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - } + ret.verify_side_metadata_sanity(); ret } diff --git a/src/plan/semispace/global.rs b/src/plan/semispace/global.rs index 2485fcb9ea..ec74ec6c4c 100644 --- a/src/plan/semispace/global.rs +++ b/src/plan/semispace/global.rs @@ -13,23 +13,25 @@ use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; use crate::util::heap::VMRequest; -use crate::util::metadata::side_metadata::{SideMetadataContext, SideMetadataSanity}; +use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::opaque_pointer::VMWorkerThread; use crate::{plan::global::BasePlan, vm::VMBinding}; use std::sync::atomic::{AtomicBool, Ordering}; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; use enum_map::EnumMap; -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct SemiSpace { pub hi: AtomicBool, - #[trace(CopySemantics::DefaultCopy)] + #[space] + #[copy_semantics(CopySemantics::DefaultCopy)] pub copyspace0: CopySpace, - #[trace(CopySemantics::DefaultCopy)] + #[space] + #[copy_semantics(CopySemantics::DefaultCopy)] pub copyspace1: CopySpace, - #[fallback_trace] + #[parent] pub common: CommonPlan, } @@ -44,8 +46,6 @@ pub const SS_CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for SemiSpace { - type VM = VM; - fn constraints(&self) -> &'static PlanConstraints { &SS_CONSTRAINTS } @@ -65,13 +65,6 @@ impl Plan for SemiSpace { } } - fn get_spaces(&self) -> Vec<&dyn Space> { - let mut ret = self.common.get_spaces(); - ret.push(&self.copyspace0); - ret.push(&self.copyspace1); - ret - } - fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { self.base().set_collection_kind::(self); self.base().set_gc_status(GcStatus::GcPrepare); @@ -159,17 +152,7 @@ impl SemiSpace { common: CommonPlan::new(plan_args), }; - // Use SideMetadataSanity to check if each spec is valid. This is also needed for check - // side metadata in extreme_assertions. - { - let mut side_metadata_sanity_checker = SideMetadataSanity::new(); - res.common - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.copyspace0 - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - res.copyspace1 - .verify_side_metadata_sanity(&mut side_metadata_sanity_checker); - } + res.verify_side_metadata_sanity(); res } diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index e01bdba61a..c38b3d590f 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -21,14 +21,14 @@ use atomic::Ordering; use std::sync::atomic::AtomicBool; use std::sync::{Arc, Mutex}; -use mmtk_macros::PlanTraceObject; +use mmtk_macros::{HasSpaces, PlanTraceObject}; use super::gc_work::StickyImmixMatureGCWorkContext; use super::gc_work::StickyImmixNurseryGCWorkContext; -#[derive(PlanTraceObject)] +#[derive(HasSpaces, PlanTraceObject)] pub struct StickyImmix { - #[fallback_trace] + #[parent] immix: immix::Immix, gc_full_heap: AtomicBool, next_gc_full_heap: AtomicBool, @@ -45,8 +45,6 @@ pub const STICKY_IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for StickyImmix { - type VM = VM; - fn constraints(&self) -> &'static crate::plan::PlanConstraints { &STICKY_IMMIX_CONSTRAINTS } @@ -104,10 +102,6 @@ impl Plan for StickyImmix { } } - fn get_spaces(&self) -> Vec<&dyn crate::policy::space::Space> { - self.immix.get_spaces() - } - fn get_allocator_mapping( &self, ) -> &'static enum_map::EnumMap diff --git a/src/util/heap/layout/map64.rs b/src/util/heap/layout/map64.rs index ea9db40044..ec4383d010 100644 --- a/src/util/heap/layout/map64.rs +++ b/src/util/heap/layout/map64.rs @@ -56,7 +56,9 @@ impl Map64 { // elide the storing of 0 for each of the element. Using standard vector creation, // such as `vec![SpaceDescriptor::UNINITIALIZED; MAX_CHUNKS]`, will cause severe // slowdown during start-up. - descriptor_map: unsafe { new_zeroed_vec::(vm_layout().max_chunks()) }, + descriptor_map: unsafe { + new_zeroed_vec::(vm_layout().max_chunks()) + }, high_water, base_address, fl_page_resources: vec![None; MAX_SPACES], diff --git a/src/util/metadata/side_metadata/sanity.rs b/src/util/metadata/side_metadata/sanity.rs index ae1377a10f..46f9889bf8 100644 --- a/src/util/metadata/side_metadata/sanity.rs +++ b/src/util/metadata/side_metadata/sanity.rs @@ -433,7 +433,7 @@ pub fn verify_bzero(metadata_spec: &SideMetadataSpec, start: Address, size: usiz } } None => { - panic!("Invalid Metadata Spec!"); + panic!("Invalid Metadata Spec: {}", metadata_spec.name); } } }