search

mmtk / mmtk-core

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

Rename associated types in GCWorkContext #1116

Closed wks closed 7 months ago

wks commented 7 months ago

This PR renames the associated types ProcessEdgesWorkType to DefaultProcessEdges, and TPProcessEdges to PinningProcessEdges.

The old name TPProcessEdges was very confusing because that type alone is not sufficient to implement transitive pinning roots. We rename it to PinningProcessEdges, removing "transitive" from its name. We also renamed ProcessEdgesWorkType to DefaultProcessEdges for symmetry. We also updated comments to clarify the purposes of those type members.

We also updated the type parameters of ProcessEdgesWorkRootsWorkFactory<VM, DPE, PPE> and ProcessRootNode<VM, R2OPE, O2OPE> to clarify their purposes, at the expense of being slightly more verbose.

k-sareen commented 7 months ago

Normal -> Default perhaps?

wks commented 7 months ago

I did the obvious change -- renaming those type members and some generic type arguments.

I considered making is_pinning: bool a parameter of trace_object. (See: https://mmtk.zulipchat.com/#narrow/stream/262673-mmtk-core/topic/Pinning.20and.20TPProcessEdges/near/432461772) It may need some non-trivial refactoring, and may potentially change the API (I can already anticipate changing the public method ObjectTracer::trace_object). I'll try to do the refactoring the next time.

wks commented 7 months ago

Normal -> Default perhaps?

Yes. That sounds better.