mmtk / mmtk-core

Memory Management ToolKit
https://www.mmtk.io
Other
375 stars 69 forks source link

Remove or refactor `GcStatus` #1061

Open wks opened 10 months ago

wks commented 10 months ago

The GcStatus type is defined as following:

pub enum GcStatus {
    NotInGC,
    GcPrepare,
    GcProper,
}

It is inherited from JikesRVM. The status is set during different phases of a GC.

But its value is never read in the Rust MMTK. Neither MMTK::gc_in_progress() nor MMTk::gc_in_progress_proper() is called anywhere in the Rust MMTk or any of its bindings.

In JikesRVM,

It looks like the GC status mainly helps in concurrent GC algorithms, but the lxr branch of Rust MMTk does not read the gc_status field, either.

So I think GcStatus can be safely removed from MMTK without any harm.

Repurposing

When I was trying to remove the coordinator thread (See https://github.com/mmtk/mmtk-core/issues/1053), I realized that with the coordinator thread removed, we need a state machine to record the GC status, in a more fine-grained way than the current GcStatus and the current fields of GCReqeuster, and the states will affect the behavior of several things, including triggering GC, and the behavior of the last parked GC worker (to open new buckets or to wait for the GC to be triggered). I think we can re-purpose the GcStatus type for this.

(Update: https://github.com/mmtk/mmtk-core/pull/1067 which fixes #1053 now uses its own state for that purpose, and will not affect this issue.)

wks commented 9 months ago

Actually it is not "unused". Its most important use is the function MMTK::set_gc_status itself. This function marks the start and end of each GC, and is used to distinguish between mutator time and GC time in statistics. Such statistics is part of the output of harness_end.