mmtk / mmtk-core

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

Don't abuse the file name `global.rs` #1190

Open wks opened 2 months ago

wks commented 2 months ago

In Rust, the idiomatic place to put top-level items (functions, types, constants, ...) of a module is mod.rs.

In mmtk-core, many modules contain global.rs. The file name global.rs is intended for plans, policies, allocators and other things that contain global parts (data structures not specific to any mutator), mutator-specific parts, and GC worker-specific parts. For example, the Immix plan has global.rs, mutator.rs and gc_work.rs. That basically follows the pattern in JikesRVM MMTk, for example, Immix, ImmixMutator, ImmixCollector and ImmixTraceLocal. (Actually in Rust MMTk, global.rs basically does everything. gc_work.rs simply declares what ProcessEdgesWork to use. mutator.rs simply creates Mutator instances, and everything else is done by the Mutator itself.)

But the filename global.rs does not make sense for other things that don't have such distinction. For example,

The three places above may be abusing the name global.rs for module-level items, and should be in mod.rs, instead.

qinsoon commented 2 months ago

The plan modules were previously named as plan::plan (src/plan/plan.rs), plan::semispace::semispace (src/plan/semispace/semispace.rs), etc. This mostly follows what Java MMTk names plans. However, clippy does not like this. So the inner modules are renamed to global. It actually makes sense, as what are defined in the plan are the global-level stuff.

The global.rs files in metadata are simply a misuse.

We should definitely fix the latter. We may keep the former, or change it -- I don't mind too much.

qinsoon commented 2 months ago

More places with the filename global.rs:

These are wrong as well.