Refactor log bits

qinsoon · qinsoon · commit 017a0609a12c · 2025-08-08T04:50:03.000Z
diff --git a/src/plan/generational/copying/global.rs b/src/plan/generational/copying/global.rs
@@ -110,6 +110,9 @@ impl<VM: VMBinding> Plan for GenCopy<VM> {
         let full_heap = !self.gen.is_current_gc_nursery();
         self.gen.release(tls);
         if full_heap {
+            if VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.is_on_side() {
+                self.fromspace().clear_side_log_bits();
+            }
             self.fromspace().release();
         }
     }
diff --git a/src/plan/generational/global.rs b/src/plan/generational/global.rs
@@ -74,6 +74,9 @@ impl<VM: VMBinding> CommonGenPlan<VM> {
     /// Release Gen. This should be called by a single thread in GC release work.
     pub fn release(&mut self, tls: VMWorkerThread) {
         let full_heap = !self.is_current_gc_nursery();
+        if VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.is_on_side() {
+            self.nursery.clear_side_log_bits();
+        }
         self.common.release(tls, full_heap);
         self.nursery.release();
     }
diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs
@@ -129,6 +129,9 @@ impl<VM: VMBinding> Plan for GenImmix<VM> {
         let full_heap = !self.gen.is_current_gc_nursery();
         self.gen.prepare(tls);
         if full_heap {
+            if VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.is_on_side() {
+                self.immix_space.clear_side_log_bits();
+            }
             self.immix_space.prepare(
                 full_heap,
                 Some(crate::policy::immix::defrag::StatsForDefrag::new(self)),
@@ -249,8 +252,6 @@ impl<VM: VMBinding> GenImmix<VM> {
         let immix_space = ImmixSpace::new(
             plan_args.get_space_args("immix_mature", true, false, VMRequest::discontiguous()),
             ImmixSpaceArgs {
-                // We need to unlog objects at tracing time since we currently clear all log bits during a major GC
-                unlog_object_when_traced: true,
                 // In GenImmix, young objects are not allocated in ImmixSpace directly.
                 #[cfg(feature = "vo_bit")]
                 mixed_age: false,
diff --git a/src/plan/generational/mod.rs b/src/plan/generational/mod.rs
@@ -45,6 +45,8 @@ pub const FULL_NURSERY_GC: bool = false;
 pub const GEN_CONSTRAINTS: PlanConstraints = PlanConstraints {
     moves_objects: true,
     needs_log_bit: ACTIVE_BARRIER.equals(BarrierSelector::ObjectBarrier),
+    unlog_allocated_object: true,
+    unlog_traced_object: true,
     barrier: ACTIVE_BARRIER,
     // We may trace duplicate edges in sticky immix (or any plan that uses object remembering barrier). See https://github.com/mmtk/mmtk-core/issues/743.
     may_trace_duplicate_edges: ACTIVE_BARRIER.equals(BarrierSelector::ObjectBarrier),
diff --git a/src/plan/global.rs b/src/plan/global.rs
@@ -416,6 +416,8 @@ impl<VM: VMBinding> CreateSpecificPlanArgs<'_, VM> {
             zeroed,
             permission_exec,
             vmrequest,
+            unlog_allocated_object: self.constraints.unlog_allocated_object,
+            unlog_traced_object: self.constraints.unlog_traced_object,
             global_side_metadata_specs: self.global_side_metadata_specs.clone(),
             vm_map: self.global_args.vm_map,
             mmapper: self.global_args.mmapper,
@@ -517,6 +519,28 @@ impl<VM: VMBinding> BasePlan<VM> {
         self.vm_space.release();
     }
 
+    pub fn clear_side_log_bits(&self) {
+        #[cfg(feature = "code_space")]
+        self.code_space.clear_side_log_bits();
+        #[cfg(feature = "code_space")]
+        self.code_lo_space.clear_side_log_bits();
+        #[cfg(feature = "ro_space")]
+        self.ro_space.clear_side_log_bits();
+        #[cfg(feature = "vm_space")]
+        self.vm_space.clear_side_log_bits();
+    }
+
+    pub fn set_side_log_bits(&self) {
+        #[cfg(feature = "code_space")]
+        self.code_space.set_side_log_bits();
+        #[cfg(feature = "code_space")]
+        self.code_lo_space.set_side_log_bits();
+        #[cfg(feature = "ro_space")]
+        self.ro_space.set_side_log_bits();
+        #[cfg(feature = "vm_space")]
+        self.vm_space.set_side_log_bits();
+    }
+
     pub fn end_of_gc(&mut self, _tls: VMWorkerThread) {
         // Do nothing here. None of the spaces needs end_of_gc.
     }
@@ -584,6 +608,7 @@ pub struct CommonPlan<VM: VMBinding> {
 
 impl<VM: VMBinding> CommonPlan<VM> {
     pub fn new(mut args: CreateSpecificPlanArgs<VM>) -> CommonPlan<VM> {
+        let needs_log_bit = args.constraints.needs_log_bit;
         CommonPlan {
             immortal: ImmortalSpace::new(args.get_space_args(
                 "immortal",
@@ -594,6 +619,7 @@ impl<VM: VMBinding> CommonPlan<VM> {
             los: LargeObjectSpace::new(
                 args.get_space_args("los", true, false, VMRequest::discontiguous()),
                 false,
+                needs_log_bit,
             ),
             nonmoving: Self::new_nonmoving_space(&mut args),
             base: BasePlan::new(args),
@@ -621,6 +647,18 @@ impl<VM: VMBinding> CommonPlan<VM> {
         self.base.release(tls, full_heap)
     }
 
+    pub fn clear_side_log_bits(&self) {
+        self.immortal.clear_side_log_bits();
+        self.los.clear_side_log_bits();
+        self.base.clear_side_log_bits();
+    }
+
+    pub fn set_side_log_bits(&self) {
+        self.immortal.set_side_log_bits();
+        self.los.set_side_log_bits();
+        self.base.set_side_log_bits();
+    }
+
     pub fn end_of_gc(&mut self, tls: VMWorkerThread) {
         self.end_of_gc_nonmoving_space();
         self.base.end_of_gc(tls);
@@ -648,7 +686,6 @@ impl<VM: VMBinding> CommonPlan<VM> {
                 NonMovingSpace::new(
                     space_args,
                     crate::policy::immix::ImmixSpaceArgs {
-                        unlog_object_when_traced: false,
                         #[cfg(feature = "vo_bit")]
                         mixed_age: false,
                         never_move_objects: true,
diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs
@@ -138,7 +138,6 @@ impl<VM: VMBinding> Immix<VM> {
         Self::new_with_args(
             plan_args,
             ImmixSpaceArgs {
-                unlog_object_when_traced: false,
                 #[cfg(feature = "vo_bit")]
                 mixed_age: false,
                 never_move_objects: false,
diff --git a/src/plan/pageprotect/global.rs b/src/plan/pageprotect/global.rs
@@ -110,6 +110,7 @@ impl<VM: VMBinding> PageProtect<VM> {
             space: LargeObjectSpace::new(
                 plan_args.get_space_args("pageprotect", true, false, VMRequest::discontiguous()),
                 true,
+                false, // PageProtect does not use log bit
             ),
             common: CommonPlan::new(plan_args),
         };
diff --git a/src/plan/plan_constraints.rs b/src/plan/plan_constraints.rs
@@ -45,6 +45,10 @@ pub struct PlanConstraints {
     /// `MutatorConfig::prepare_func`).  Those plans can set this to `false` so that the
     /// `PrepareMutator` work packets will not be created at all.
     pub needs_prepare_mutator: bool,
+    /// Should a policy unlog newly allocated objects?
+    pub unlog_allocated_object: bool,
+    /// Should a policy unlog traced objects?
+    pub unlog_traced_object: bool,
 }
 
 impl PlanConstraints {
@@ -67,6 +71,8 @@ impl PlanConstraints {
             barrier: BarrierSelector::NoBarrier,
             // If we use mark sweep as non moving space, we need to prepare mutator. See [`common_prepare_func`].
             needs_prepare_mutator: cfg!(feature = "marksweep_as_nonmoving"),
+            unlog_allocated_object: false,
+            unlog_traced_object: false,
         }
     }
 }
diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs
@@ -46,6 +46,8 @@ pub const STICKY_IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints {
     barrier: crate::plan::BarrierSelector::ObjectBarrier,
     // We may trace duplicate edges in sticky immix (or any plan that uses object remembering barrier). See https://github.com/mmtk/mmtk-core/issues/743.
     may_trace_duplicate_edges: true,
+    unlog_allocated_object: true,
+    unlog_traced_object: true,
     ..immix::IMMIX_CONSTRAINTS
 };
 
@@ -122,6 +124,10 @@ impl<VM: VMBinding> Plan for StickyImmix<VM> {
             self.immix.common.los.prepare(false);
         } else {
             self.full_heap_gc_count.lock().unwrap().inc();
+            if VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.is_on_side() {
+                self.immix.common.clear_side_log_bits();
+                self.immix.immix_space.clear_side_log_bits();
+            }
             self.immix.prepare(tls);
         }
     }
@@ -325,10 +331,6 @@ impl<VM: VMBinding> StickyImmix<VM> {
         let immix = immix::Immix::new_with_args(
             plan_args,
             crate::policy::immix::ImmixSpaceArgs {
-                // Every object we trace in nursery GC becomes a mature object.
-                // Every object we trace in full heap GC is a mature object. Thus in both cases,
-                // they should be unlogged.
-                unlog_object_when_traced: true,
                 // In StickyImmix, both young and old objects are allocated in the ImmixSpace.
                 #[cfg(feature = "vo_bit")]
                 mixed_age: true,
diff --git a/src/policy/copyspace.rs b/src/policy/copyspace.rs
@@ -201,12 +201,6 @@ impl<VM: VMBinding> CopySpace<VM> {
                 side_forwarding_status_table.bzero_metadata(start, size);
             }
 
-            if self.common.needs_log_bit {
-                if let MetadataSpec::OnSide(side) = *VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC {
-                    side.bzero_metadata(start, size);
-                }
-            }
-
             // Clear VO bits because all objects in the space are dead.
             #[cfg(feature = "vo_bit")]
             crate::util::metadata::vo_bit::bzero_vo_bit(start, size);
@@ -218,6 +212,20 @@ impl<VM: VMBinding> CopySpace<VM> {
         self.from_space.store(false, Ordering::SeqCst);
     }
 
+    pub fn clear_side_log_bits(&self) {
+        let log_bit = VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.extract_side_spec();
+        for (start, size) in self.pr.iterate_allocated_regions() {
+            log_bit.bzero_metadata(start, size);
+        }
+    }
+
+    pub fn set_side_log_bits(&self) {
+        let log_bit = VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.extract_side_spec();
+        for (start, size) in self.pr.iterate_allocated_regions() {
+            log_bit.bset_metadata(start, size);
+        }
+    }
+
     fn is_from_space(&self) -> bool {
         self.from_space.load(Ordering::SeqCst)
     }
diff --git a/src/policy/immix/immixspace.rs b/src/policy/immix/immixspace.rs
@@ -60,12 +60,6 @@ pub struct ImmixSpace<VM: VMBinding> {
 
 /// Some arguments for Immix Space.
 pub struct ImmixSpaceArgs {
-    /// Mark an object as unlogged when we trace an object.
-    /// Normally we set the log bit when we copy an object with [`crate::util::copy::CopySemantics::PromoteToMature`].
-    /// In sticky immix, we 'promote' an object to mature when we trace the object
-    /// (no matter we copy an object or not). So we have to use `PromoteToMature`, and instead
-    /// just set the log bit in the space when an object is traced.
-    pub unlog_object_when_traced: bool,
     /// Whether this ImmixSpace instance contains both young and old objects.
     /// This affects the updating of valid-object bits.  If some lines or blocks of this ImmixSpace
     /// instance contain young objects, their VO bits need to be updated during this GC.  Currently
@@ -301,7 +295,7 @@ impl<VM: VMBinding> ImmixSpace<VM> {
         args: crate::policy::space::PlanCreateSpaceArgs<VM>,
         mut space_args: ImmixSpaceArgs,
     ) -> Self {
-        if space_args.unlog_object_when_traced {
+        if args.unlog_traced_object {
             assert!(
                 args.constraints.needs_log_bit,
                 "Invalid args when the plan does not use log bit"
@@ -421,14 +415,6 @@ impl<VM: VMBinding> ImmixSpace<VM> {
                 unimplemented!("cyclic mark bits is not supported at the moment");
             }
 
-            if self.common.needs_log_bit {
-                if let MetadataSpec::OnSide(side) = *VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC {
-                    for chunk in self.chunk_map.all_chunks() {
-                        side.bzero_metadata(chunk.start(), Chunk::BYTES);
-                    }
-                }
-            }
-
             // Prepare defrag info
             if self.is_defrag_enabled() {
                 self.defrag.prepare(self, plan_stats.unwrap());
@@ -529,6 +515,20 @@ impl<VM: VMBinding> ImmixSpace<VM> {
         did_defrag
     }
 
+    pub fn clear_side_log_bits(&self) {
+        let log_bit = VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.extract_side_spec();
+        for chunk in self.chunk_map.all_chunks() {
+            log_bit.bzero_metadata(chunk.start(), Chunk::BYTES);
+        }
+    }
+
+    pub fn set_side_log_bits(&self) {
+        let log_bit = VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.extract_side_spec();
+        for chunk in self.chunk_map.all_chunks() {
+            log_bit.bset_metadata(chunk.start(), Chunk::BYTES);
+        }
+    }
+
     /// Generate chunk sweep tasks
     fn generate_sweep_tasks(&self) -> Vec<Box<dyn GCWork<VM>>> {
         self.defrag.mark_histograms.lock().clear();
@@ -728,7 +728,7 @@ impl<VM: VMBinding> ImmixSpace<VM> {
     }
 
     fn unlog_object_if_needed(&self, object: ObjectReference) {
-        if self.space_args.unlog_object_when_traced {
+        if self.common.unlog_traced_object {
             // Make sure the side metadata for the line can fit into one byte. For smaller line size, we should
             // use `mark_as_unlogged` instead to mark the bit.
             const_assert!(
diff --git a/src/policy/immortalspace.rs b/src/policy/immortalspace.rs
@@ -56,7 +56,7 @@ impl<VM: VMBinding> SFT for ImmortalSpace<VM> {
     fn initialize_object_metadata(&self, object: ObjectReference, _alloc: bool) {
         self.mark_state
             .on_object_metadata_initialization::<VM>(object);
-        if self.common.needs_log_bit {
+        if self.common.unlog_allocated_object {
             VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.mark_as_unlogged::<VM>(object, Ordering::SeqCst);
         }
         #[cfg(feature = "vo_bit")]
@@ -173,6 +173,20 @@ impl<VM: VMBinding> ImmortalSpace<VM> {
         self.mark_state.on_global_release::<VM>();
     }
 
+    pub fn clear_side_log_bits(&self) {
+        let log_bit = VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.extract_side_spec();
+        for (start, size) in self.pr.iterate_allocated_regions() {
+            log_bit.bzero_metadata(start, size);
+        }
+    }
+
+    pub fn set_side_log_bits(&self) {
+        let log_bit = VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.extract_side_spec();
+        for (start, size) in self.pr.iterate_allocated_regions() {
+            log_bit.bset_metadata(start, size);
+        }
+    }
+
     pub fn trace_object<Q: ObjectQueue>(
         &self,
         queue: &mut Q,
@@ -186,7 +200,7 @@ impl<VM: VMBinding> ImmortalSpace<VM> {
         );
         if self.mark_state.test_and_mark::<VM>(object) {
             // Set the unlog bit if required
-            if self.common.needs_log_bit {
+            if self.common.unlog_traced_object {
                 VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.store_atomic::<VM, u8>(
                     object,
                     1,
diff --git a/src/policy/largeobjectspace.rs b/src/policy/largeobjectspace.rs
diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs
diff --git a/src/policy/space.rs b/src/policy/space.rs
diff --git a/src/policy/vmspace.rs b/src/policy/vmspace.rs
diff --git a/src/util/copy/mod.rs b/src/util/copy/mod.rs

Original file line number	Diff line number	Diff line change
`@@ -110,6 +110,9 @@ impl<VM: VMBinding> Plan for GenCopy<VM> {`
`110`	`110`	`let full_heap = !self.gen.is_current_gc_nursery();`
`111`	`111`	`self.gen.release(tls);`
`112`	`112`	`if full_heap {`
	`113`	`+ if VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC.is_on_side() {`
	`114`	`+ self.fromspace().clear_side_log_bits();`
	`115`	`+ }`
`113`	`116`	`self.fromspace().release();`
`114`	`117`	`}`
`115`	`118`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,10 @@ pub struct PlanConstraints {`
`45`	`45`	/// `MutatorConfig::prepare_func`). Those plans can set this to `false` so that the
`46`	`46`	/// `PrepareMutator` work packets will not be created at all.
`47`	`47`	`pub needs_prepare_mutator: bool,`
	`48`	`+ /// Should a policy unlog newly allocated objects?`
	`49`	`+ pub unlog_allocated_object: bool,`
	`50`	`+ /// Should a policy unlog traced objects?`
	`51`	`+ pub unlog_traced_object: bool,`
`48`	`52`	`}`
`49`	`53`
`50`	`54`	`impl PlanConstraints {`
`@@ -67,6 +71,8 @@ impl PlanConstraints {`
`67`	`71`	`barrier: BarrierSelector::NoBarrier,`
`68`	`72`	// If we use mark sweep as non moving space, we need to prepare mutator. See [`common_prepare_func`].
`69`	`73`	`needs_prepare_mutator: cfg!(feature = "marksweep_as_nonmoving"),`
	`74`	`+ unlog_allocated_object: false,`
	`75`	`+ unlog_traced_object: false,`
`70`	`76`	`}`
`71`	`77`	`}`
`72`	`78`	`}`