mmtk-core icon indicating copy to clipboard operation
mmtk-core copied to clipboard

Remove outdated fields in PlanConstraints

Open qinsoon opened this issue 2 years ago • 2 comments

PlanConstraints are constant for each plan. But some of its fields are unused, or superseded by other mechanisms.

moves_objects

https://github.com/mmtk/mmtk-core/blob/3dbdd7ae1076fb022f483b8b91cf37425dc6f0ee/src/plan/plan_constraints.rs#L11

moves_objects is superseded by may_move_objects() in PlanTraceObject, which is a constant value based on the trace kind. It is helpful for plans like Immix, for which, some traces (defrag) move objects and other traces (fast) do not move objects.

gc_header_bits and gc_header_words

https://github.com/mmtk/mmtk-core/blob/3dbdd7ae1076fb022f483b8b91cf37425dc6f0ee/src/plan/plan_constraints.rs#L12-L13

We now have metadata specs, like mark bit, log bit. We no longer need a plan to specify the GC bits/words they need.

num_specialized_scans

https://github.com/mmtk/mmtk-core/blob/3dbdd7ae1076fb022f483b8b91cf37425dc6f0ee/src/plan/plan_constraints.rs#L14

This is unused. This seems related with object scanning, which is now in the bindings.

max_non_los_copy_bytes

https://github.com/mmtk/mmtk-core/blob/3dbdd7ae1076fb022f483b8b91cf37425dc6f0ee/src/plan/plan_constraints.rs#L21

This is currently unused. We need to decide whether we need to check object size before copying (currently we assume we won't). The difference is that in one case, we could copy nursery->mature or nursery->LOS, and in the other case, we only copy nursery->mature (large objects are directly allocated into LOS' logical nursery). A related issue: https://github.com/mmtk/mmtk-core/issues/452.

needs_linear_scan

https://github.com/mmtk/mmtk-core/blob/3dbdd7ae1076fb022f483b8b91cf37425dc6f0ee/src/plan/plan_constraints.rs#L30

Java MMTk uses this in object model. I am not sure how it works. We need to further check this. However, for MMTk core, I assume we will allow linear scan in some policies and when global_alloc_bit is enabled. So we may need to make sure this value is set properly.

needs_concurrent_workers

https://github.com/mmtk/mmtk-core/blob/3dbdd7ae1076fb022f483b8b91cf37425dc6f0ee/src/plan/plan_constraints.rs#L31

Currently unused.

generate_gc_trace

https://github.com/mmtk/mmtk-core/blob/3dbdd7ae1076fb022f483b8b91cf37425dc6f0ee/src/plan/plan_constraints.rs#L32

This should be removed. It is related to GCTrace in Java MMTk, and we do not plan to port it.

qinsoon avatar May 09 '22 00:05 qinsoon

I think, to me at least, part of why these values are useful is if I just want to compare two different plans. Comparing MarkCompact to SemiSpace, I can immediately see that oh MarkCompact uses 1 extra header word and also requires an extra scan in comparison to SemiSpace, instead of having to read its source code.

Though I'll note that we should fix how gc_header_words is used. This is something that I encountered when I was trying to add +1 extra word to header for SemiSpace. Simply increasing the gc_header_words in the plan constraint didn't work since currently the fastpath and everything is hardcoded only for MarkCompact. I will be submitting an issue for this for both mmtk-core and mmtk-openjdk.

k-sareen avatar May 10 '22 04:05 k-sareen

https://github.com/mmtk/mmtk-core/pull/1028 removed the following fields:

  • gc_header_bits/words
  • num_specialized_scans
  • generate_gc_trace

qinsoon avatar Nov 23 '23 12:11 qinsoon