mmtk / mmtk-core

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

Fix issues in sanity GC #1079

Closed qinsoon closed 10 months ago

qinsoon commented 10 months ago

This PR fixes two issues in sanity GC.

wks commented 10 months ago

ScanObjectsWork::do_work_common used to contain code for adding root nodes into the sanity checker. Since https://github.com/mmtk/mmtk-core/commit/61d20e2dcd5b4743ef04a8118eb807bcd6f6e2e2, the processing of root nodes (the objects pointed by root edges from outside the object graph, i.e. pointed by stack slots and global variables) is moved to ProcessRootNode, and ScanObjects never processes any "root nodes". Therefore, it is right to replace the use of ScanObjects with ProcessRootNode. (I wonder why https://github.com/mmtk/mmtk-core/commit/61d20e2dcd5b4743ef04a8118eb807bcd6f6e2e2 changed the code related to sanity GC but didn't change the actual SanityChecker.)

I also think it is safe to remove the roots() method from the ScanObjectsWork trait, and the roots field from both ScanObjects and PlanScanObjects. The roots() method was only used by the code related to sanity checking. The fact that it is asserted to be false in do_work_common means it is not used now.

qinsoon commented 10 months ago

I also think it is safe to remove the roots() method from the ScanObjectsWork trait, and the roots field from both ScanObjects and PlanScanObjects. The roots() method was only used by the code related to sanity checking. The fact that it is asserted to be false in do_work_common means it is not used now.

I removed roots() in the trait and the fields in its implementations.

wks commented 10 months ago

There are some example code in the documentation that still uses the roots parameter.

qinsoon commented 10 months ago

binding-refs OPENJDK_BINDING_REPO=qinsoon/mmtk-openjdk OPENJDK_BINDING_REF=add-test-for-sanity-gc