From a4401cee9e8cef8e2e9f3dd136dfec1a55daac19 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 25 Apr 2024 16:33:55 +0800 Subject: [PATCH 1/7] Allow VM to query if the current GC moves objects. --- src/plan/generational/copying/global.rs | 4 ++++ src/plan/generational/immix/global.rs | 8 ++++++++ src/plan/global.rs | 5 +++++ src/plan/immix/global.rs | 4 ++++ src/plan/markcompact/global.rs | 4 ++++ src/plan/marksweep/global.rs | 4 ++++ src/plan/nogc/global.rs | 4 ++++ src/plan/pageprotect/global.rs | 4 ++++ src/plan/semispace/global.rs | 4 ++++ src/plan/sticky/immix/global.rs | 8 ++++++++ 10 files changed, 49 insertions(+) diff --git a/src/plan/generational/copying/global.rs b/src/plan/generational/copying/global.rs index faa4d73266..2e5da551ba 100644 --- a/src/plan/generational/copying/global.rs +++ b/src/plan/generational/copying/global.rs @@ -127,6 +127,10 @@ impl Plan for GenCopy { self.gen.get_used_pages() + self.tospace().reserved_pages() } + fn current_gc_may_move_object(&self) -> bool { + true + } + /// Return the number of pages available for allocation. Assuming all future allocations goes to nursery. fn get_available_pages(&self) -> usize { // super.get_available_pages() / 2 to reserve pages for copying diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs index 97b09b15fd..bffa2bd2d1 100644 --- a/src/plan/generational/immix/global.rs +++ b/src/plan/generational/immix/global.rs @@ -151,6 +151,14 @@ impl Plan for GenImmix { .set_next_gc_full_heap(CommonGenPlan::should_next_gc_be_full_heap(self)); } + fn current_gc_may_move_object(&self) -> bool { + if self.is_current_gc_nursery() { + true + } else { + self.immix_space.in_defrag() + } + } + fn get_collection_reserved_pages(&self) -> usize { self.gen.get_collection_reserved_pages() + self.immix_space.defrag_headroom_pages() } diff --git a/src/plan/global.rs b/src/plan/global.rs index 1e47665581..3fd02ef28f 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -299,6 +299,11 @@ pub trait Plan: 'static + HasSpaces + Sync + Downcast { true } + /// Return whether the current GC may move any object. The VM binding can make use of this + /// information and choose to or not to update some data structures that record the addresses + /// of objects. + fn current_gc_may_move_object(&self) -> bool; + /// An object is firstly reached by a sanity GC. So the object is reachable /// in the current GC, and all the GC work has been done for the object (such as /// tracing and releasing). A plan can implement this to diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs index 0741ba3a99..50e9595ae0 100644 --- a/src/plan/immix/global.rs +++ b/src/plan/immix/global.rs @@ -97,6 +97,10 @@ impl Plan for Immix { .store(self.immix_space.release(true), Ordering::Relaxed); } + fn current_gc_may_move_object(&self) -> bool { + self.immix_space.in_defrag() + } + fn get_collection_reserved_pages(&self) -> usize { self.immix_space.defrag_headroom_pages() } diff --git a/src/plan/markcompact/global.rs b/src/plan/markcompact/global.rs index 71e756de3f..7804d9757d 100644 --- a/src/plan/markcompact/global.rs +++ b/src/plan/markcompact/global.rs @@ -171,6 +171,10 @@ impl Plan for MarkCompact { fn get_collection_reserved_pages(&self) -> usize { 0 } + + fn current_gc_may_move_object(&self) -> bool { + true + } } impl MarkCompact { diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index 8715e34baf..c8cc1ccd2f 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -73,6 +73,10 @@ impl Plan for MarkSweep { self.base().collection_required(self, space_full) } + fn current_gc_may_move_object(&self) -> bool { + false + } + fn get_used_pages(&self) -> usize { self.common.get_used_pages() + self.ms.reserved_pages() } diff --git a/src/plan/nogc/global.rs b/src/plan/nogc/global.rs index b2d02f58d3..8cc8334cba 100644 --- a/src/plan/nogc/global.rs +++ b/src/plan/nogc/global.rs @@ -74,6 +74,10 @@ impl Plan for NoGC { unreachable!("GC triggered in nogc") } + fn current_gc_may_move_object(&self) -> bool { + false + } + fn get_used_pages(&self) -> usize { self.nogc_space.reserved_pages() + self.immortal.reserved_pages() diff --git a/src/plan/pageprotect/global.rs b/src/plan/pageprotect/global.rs index 9e9bcb1fdf..cc5efb78b0 100644 --- a/src/plan/pageprotect/global.rs +++ b/src/plan/pageprotect/global.rs @@ -62,6 +62,10 @@ impl Plan for PageProtect { self.base().collection_required(self, space_full) } + fn current_gc_may_move_object(&self) -> bool { + false + } + fn get_used_pages(&self) -> usize { self.space.reserved_pages() + self.common.get_used_pages() } diff --git a/src/plan/semispace/global.rs b/src/plan/semispace/global.rs index a55cbc0976..f1ea3483df 100644 --- a/src/plan/semispace/global.rs +++ b/src/plan/semispace/global.rs @@ -100,6 +100,10 @@ impl Plan for SemiSpace { self.base().collection_required(self, space_full) } + fn current_gc_may_move_object(&self) -> bool { + true + } + fn get_collection_reserved_pages(&self) -> usize { self.tospace().reserved_pages() } diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index 22e68d2f20..381e8b91c2 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -158,6 +158,14 @@ impl Plan for StickyImmix { self.gc_full_heap.load(Ordering::Relaxed) && self.immix.last_collection_was_exhaustive() } + fn current_gc_may_move_object(&self) -> bool { + if self.is_current_gc_nursery() { + !cfg!(feature = "sticky_immix_non_moving_nursery") + } else { + return self.get_immix_space().in_defrag(); + } + } + fn get_collection_reserved_pages(&self) -> usize { self.immix.get_collection_reserved_pages() } From e27e488137d237be2b75ca30c8b5aba9af380073 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 15 May 2024 18:00:30 +0800 Subject: [PATCH 2/7] Reset in-defrag state in end-of-gc --- src/plan/generational/immix/global.rs | 8 ++++---- src/plan/global.rs | 4 ++++ src/plan/immix/global.rs | 6 +++++- src/plan/sticky/immix/global.rs | 7 ++++--- src/policy/immix/defrag.rs | 4 ++-- src/policy/immix/immixspace.rs | 19 +++++++++++-------- 6 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs index bffa2bd2d1..ab0ccbf58d 100644 --- a/src/plan/generational/immix/global.rs +++ b/src/plan/generational/immix/global.rs @@ -137,10 +137,7 @@ impl Plan for GenImmix { let full_heap = !self.gen.is_current_gc_nursery(); self.gen.release(tls); if full_heap { - let did_defrag = self.immix_space.release(full_heap); - self.last_gc_was_defrag.store(did_defrag, Ordering::Relaxed); - } else { - self.last_gc_was_defrag.store(false, Ordering::Relaxed); + self.immix_space.release(full_heap); } self.last_gc_was_full_heap .store(full_heap, Ordering::Relaxed); @@ -149,6 +146,9 @@ impl Plan for GenImmix { fn end_of_gc(&mut self, _tls: VMWorkerThread) { self.gen .set_next_gc_full_heap(CommonGenPlan::should_next_gc_be_full_heap(self)); + + let did_defrag = self.immix_space.end_of_gc(); + self.last_gc_was_defrag.store(did_defrag, Ordering::Relaxed); } fn current_gc_may_move_object(&self) -> bool { diff --git a/src/plan/global.rs b/src/plan/global.rs index 3fd02ef28f..d28fdf7298 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -302,6 +302,10 @@ pub trait Plan: 'static + HasSpaces + Sync + Downcast { /// Return whether the current GC may move any object. The VM binding can make use of this /// information and choose to or not to update some data structures that record the addresses /// of objects. + /// + /// This function is callable during a GC. From the VM binding's point of view, the information + /// of whether the current GC is a defrag GC is available since `Collection::stop_mutators` is + /// called, and remains available until `resume_mutators`. fn current_gc_may_move_object(&self) -> bool; /// An object is firstly reached by a sanity GC. So the object is reachable diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs index 50e9595ae0..6292b01aa2 100644 --- a/src/plan/immix/global.rs +++ b/src/plan/immix/global.rs @@ -93,8 +93,12 @@ impl Plan for Immix { fn release(&mut self, tls: VMWorkerThread) { self.common.release(tls, true); // release the collected region + self.immix_space.release(true); + } + + fn end_of_gc(&mut self, _tls: VMWorkerThread) { self.last_gc_was_defrag - .store(self.immix_space.release(true), Ordering::Relaxed); + .store(self.immix_space.end_of_gc(), Ordering::Relaxed); } fn current_gc_may_move_object(&self) -> bool { diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index 381e8b91c2..07842c6687 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -126,9 +126,6 @@ impl Plan for StickyImmix { fn release(&mut self, tls: crate::util::VMWorkerThread) { if self.is_current_gc_nursery() { - let was_defrag = self.immix.immix_space.release(false); - self.immix - .set_last_gc_was_defrag(was_defrag, Ordering::Relaxed); self.immix.common.los.release(false); } else { self.immix.release(tls); @@ -140,6 +137,10 @@ impl Plan for StickyImmix { crate::plan::generational::global::CommonGenPlan::should_next_gc_be_full_heap(self); self.next_gc_full_heap .store(next_gc_full_heap, Ordering::Relaxed); + + let was_defrag = self.immix.immix_space.end_of_gc(); + self.immix + .set_last_gc_was_defrag(was_defrag, Ordering::Relaxed); } fn collection_required(&self, space_full: bool, space: Option>) -> bool { diff --git a/src/policy/immix/defrag.rs b/src/policy/immix/defrag.rs index 4b4b8bb1f4..4a700a09c1 100644 --- a/src/policy/immix/defrag.rs +++ b/src/policy/immix/defrag.rs @@ -205,9 +205,9 @@ impl Defrag { .store(threshold, Ordering::Release); } - /// Release work. Should be called in ImmixSpace::release. + /// Reset the in-defrag state. #[allow(clippy::assertions_on_constants)] - pub fn release(&self, _space: &ImmixSpace) { + pub fn reset_in_defrag(&self) { debug_assert!(super::DEFRAG); self.in_defrag_collection.store(false, Ordering::Release); } diff --git a/src/policy/immix/immixspace.rs b/src/policy/immix/immixspace.rs index a93541e81e..77887d50d9 100644 --- a/src/policy/immix/immixspace.rs +++ b/src/policy/immix/immixspace.rs @@ -440,12 +440,10 @@ impl ImmixSpace { } } - /// Release for the immix space. This is called when a GC finished. - /// Return whether this GC was a defrag GC, as a plan may want to know this. - pub fn release(&mut self, major_gc: bool) -> bool { - let did_defrag = self.defrag.in_defrag(); + /// Release for the immix space. + pub fn release(&mut self, major_gc: bool) { if major_gc { - // Update line_unavail_state for hole searching afte this GC. + // Update line_unavail_state for hole searching after this GC. if !super::BLOCK_ONLY { self.line_unavail_state.store( self.line_mark_state.load(Ordering::Acquire), @@ -460,12 +458,17 @@ impl ImmixSpace { // Sweep chunks and blocks let work_packets = self.generate_sweep_tasks(); self.scheduler().work_buckets[WorkBucketStage::Release].bulk_add(work_packets); - if super::DEFRAG { - self.defrag.release(self); - } self.lines_consumed.store(0, Ordering::Relaxed); + } + /// This is called when a GC finished. + /// Return whether this GC was a defrag GC, as a plan may want to know this. + pub fn end_of_gc(&mut self) -> bool { + let did_defrag = self.defrag.in_defrag(); + if super::DEFRAG { + self.defrag.reset_in_defrag(); + } did_defrag } From 4f3b815260bc5dfa43b0a6d277e1c841083eac33 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 May 2024 10:38:41 +0800 Subject: [PATCH 3/7] Clarify current_gc_may_move_object is not available in resume_mutators --- src/plan/global.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plan/global.rs b/src/plan/global.rs index d28fdf7298..496d57725e 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -305,7 +305,8 @@ pub trait Plan: 'static + HasSpaces + Sync + Downcast { /// /// This function is callable during a GC. From the VM binding's point of view, the information /// of whether the current GC is a defrag GC is available since `Collection::stop_mutators` is - /// called, and remains available until `resume_mutators`. + /// called, and remains available until (but not including) `resume_mutators` at which time the + /// current GC has just finished. fn current_gc_may_move_object(&self) -> bool; /// An object is firstly reached by a sanity GC. So the object is reachable From 9a8d74ea3e2566f112d5632c18bc0cec3b687582 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 May 2024 12:32:04 +0800 Subject: [PATCH 4/7] Fix docs --- docs/userguide/src/tutorial/code/mygc_semispace/global.rs | 7 +++++++ docs/userguide/src/tutorial/mygc/ss/collection.md | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs index bbbf6fe6f5..8aa745b3e4 100644 --- a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs +++ b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs @@ -71,6 +71,13 @@ impl Plan for MyGC { } // ANCHOR_END: create_copy_config + // Modify + // ANCHOR: current_gc_may_move_object + fn current_gc_may_move_object(&self) -> bool { + true + } + // ANCHOR_END: current_gc_may_move_object + // Modify // ANCHOR: schedule_collection fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { diff --git a/docs/userguide/src/tutorial/mygc/ss/collection.md b/docs/userguide/src/tutorial/mygc/ss/collection.md index 758022349d..5c89ac0685 100644 --- a/docs/userguide/src/tutorial/mygc/ss/collection.md +++ b/docs/userguide/src/tutorial/mygc/ss/collection.md @@ -29,6 +29,13 @@ space here. {{#include ../../code/mygc_semispace/global.rs:create_copy_config}} ``` +Because the semispace GC copies objects in every single GC, we modify the method +`current_gc_may_move_object()` in `MyGC` so that it always returns `true`. + +```rust +{{#include ../../code/mygc_semispace/global.rs:current_gc_may_move_object}} +``` + ## Introduce collection to MyGC plan Add a new method to `Plan for MyGC`, `schedule_collection()`. This function From e8e27663641d93cdd183244140dac02fdf612e23 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 May 2024 15:17:44 +0800 Subject: [PATCH 5/7] Fix confusing comment --- src/plan/global.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plan/global.rs b/src/plan/global.rs index 496d57725e..efdffa4084 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -304,9 +304,9 @@ pub trait Plan: 'static + HasSpaces + Sync + Downcast { /// of objects. /// /// This function is callable during a GC. From the VM binding's point of view, the information - /// of whether the current GC is a defrag GC is available since `Collection::stop_mutators` is - /// called, and remains available until (but not including) `resume_mutators` at which time the - /// current GC has just finished. + /// of whether the current GC moves object or not is available since `Collection::stop_mutators` + /// is called, and remains available until (but not including) `resume_mutators` at which time + /// the current GC has just finished. fn current_gc_may_move_object(&self) -> bool; /// An object is firstly reached by a sanity GC. So the object is reachable From ccd8e611d47f8963e749aa1d344dee876886a68e Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 May 2024 15:29:46 +0800 Subject: [PATCH 6/7] StickyImmix should still release immix_space in nursery GC --- src/plan/sticky/immix/global.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index 07842c6687..a585277add 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -126,6 +126,7 @@ impl Plan for StickyImmix { fn release(&mut self, tls: crate::util::VMWorkerThread) { if self.is_current_gc_nursery() { + self.immix.immix_space.release(false); self.immix.common.los.release(false); } else { self.immix.release(tls); From affd84f16580e2bf6d997c94ba3cb984f99358aa Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 16 May 2024 15:33:32 +0800 Subject: [PATCH 7/7] Use constant instead of feature --- src/plan/sticky/immix/global.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index a585277add..4a0d9ab14c 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -7,6 +7,7 @@ use crate::plan::PlanConstraints; use crate::policy::gc_work::TraceKind; use crate::policy::gc_work::TRACE_KIND_TRANSITIVE_PIN; use crate::policy::immix::ImmixSpace; +use crate::policy::immix::PREFER_COPY_ON_NURSERY_GC; use crate::policy::immix::TRACE_KIND_FAST; use crate::policy::sft::SFT; use crate::policy::space::Space; @@ -162,9 +163,9 @@ impl Plan for StickyImmix { fn current_gc_may_move_object(&self) -> bool { if self.is_current_gc_nursery() { - !cfg!(feature = "sticky_immix_non_moving_nursery") + PREFER_COPY_ON_NURSERY_GC } else { - return self.get_immix_space().in_defrag(); + self.get_immix_space().in_defrag() } }