Do not move root objects #705
Conversation
| let scanned_root_objects = self.roots().then(|| { | ||
| // We create an instance of E to use its `trace_object` method and its object queue. | ||
| let mut process_edges_work = | ||
| <PlanProcessEdges<E::VM, P, TRACE_KIND_FAST>>::new(vec![], false, mmtk); |
There was a problem hiding this comment.
This work packet is general for all the plans. TRACE_KIND_FAST is immix specific, and should not appear in general code.
src/scheduler/gc_work.rs
Outdated
| @@ -869,15 +871,113 @@ impl<E: ProcessEdgesWork, P: Plan<VM = E::VM> + PlanTraceObject<E::VM>> PlanScan | |||
| } | |||
| } | |||
|
|
|||
| impl<E: ProcessEdgesWork, P: Plan<VM = E::VM> + PlanTraceObject<E::VM>> ScanObjectsWork<E::VM> | |||
| for PlanScanObjects<E, P> | |||
| impl<E: ProcessEdgesWork, P: Plan<VM = E::VM> + PlanTraceObject<E::VM>, const KIND: TraceKind> | |||
There was a problem hiding this comment.
It looks like this const KIND: TraceKind parameter is not used.
| let scanned_root_objects = self.roots().then(|| { | ||
| // We create an instance of E to use its `trace_object` method and its object queue. | ||
| let mut process_edges_work = | ||
| <PlanProcessEdges<E::VM, P, TRACE_KIND_FAST>>::new(vec![], false, mmtk); |
There was a problem hiding this comment.
Here we hard-coded TRACE_KIND_FAST instead of using the KIND type parameter in PlanScanObjects<E, P, KIND>. It is correct for Immix, but it needs to be extended to other plans, though.
| <PlanProcessEdges<E::VM, P, TRACE_KIND_FAST>>::new(vec![], false, mmtk); | ||
|
|
||
| for object in buffer.iter().copied() { | ||
| let new_object = process_edges_work.trace_object(object); |
There was a problem hiding this comment.
Here trace_object is a method of process_edges_work. Generally speaking the ProcessEdgesWork (along with its TraceKind if relevant) is provided by the Plan (at schedule_collection) But I think whether we pin the object is a different dimension. These pinning roots needs to pinned regardless whether it is a normal collection or a defrag collection, or whether it is a marking trace or forwarding trace. Probably it is better to add one more argument to trace_object itself, or add a different trace_object_pin method to SFT and PlanTraceObject.
There was a problem hiding this comment.
In other words, TraceKind is local to a trace (should be part of TraceLocal which disappeared when we ported MMTk to Rust), while whether to pin or not is specific to this trace_object call.
|
This PR basically duplicates |
|
As |
Yes. I agree. Currently, |
|
Superseded by #897 |
This draft PR is an attempt to make sure that root objects should not move (at least for Immix in the current implementation). I've introduced a parameter
KINDtoPlanScanObjectsand made it explicit in implementation ofdo_work_common, such that when creating process edges work, I passTRACE_KIND_FASTwhich should make sure that the root objects are traced withtrace_object_without_moving. I guess other plans that allow tracing/marking objects in place could use the same strategy.A question here is whether objects would be transitively traced without moving; as far as I understand, it wouldn't, but perhaps @wenyuzhao knows the code a bit better.