Skip to content

Replace FragmentedMapper with TwoLevelMmapper #1337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Replace FragmentedMapper with TwoLevelMmapper #1337

wants to merge 18 commits into from
+265 −209

Conversation

wks
Copy link
Collaborator

@wks wks commented Jul 9, 2025 •
edited
Loading

This PR replaces FragmentedMapper with TwoLevelMmapper.

TwoLevelMmapper does not use hash tables. It is also lock-free when looking up MapState entries, and also lock-free when lazily initializing slabs. It is intended to eliminate the pathological case of FragmentedMapper where a hash collision can result in a long-distance linear scan and many locking operations.

Fixes: #1336

@wks
Add MultiLevelMapper
fded673 
This is a replacement for `FragmentedMapper`.  It does not use hash
tables and is lock-free when looking up or lazily initializing the slabs
table.  It is intended to eliminate the pathological case of
`FragmentedMapper` where a hash collision can result in many locking
operations to find the matching or vacant slab table entry.
This was referenced Jul 9, 2025
Closed
Open
wks added 8 commits July 10, 2025 15:16
@wks
Merge branch 'master' into feature/fragmented-mapper-no-hash
c7cb998
@wks
Rename MultiLevelMapper to MultiLevelMmapper
96c76bd 
FragmentedMapper was FragmentedMmapper in JikesRVM.  FragmentedMapper
may be a misnomer introduced when porting it to Rust.
@wks
Completely remove FragmentedMapper
e54a28c 
The hash table design should have been a mistake.  Now that we are going
to replace it, we replace it completely.
@wks
Adjust comment
a212a6f 
x86_64 actualy has 47 bit address space for the user.
@wks
Rename it again to TwoLevelMmapper
461d5f4 
The current implememntation only has two levels.  We leave the name
"MultiLevelMmapper" to a true multi-level implementation in the future.
@wks
Minor comment change
0b7330a
@wks
Reimplement slab_table and get_or_allocate_slab_table
8142983
@wks
Comments on Relaxed order
9332852
@wks wks changed the title Add MultiLevelMapper Add TwoLevelMmapper Jul 11, 2025
@wks wks changed the title Add TwoLevelMmapper Replace FragmentedMapper with TwoLevelMmapper Jul 11, 2025
@wks
Copy link
Collaborator Author

wks commented Jul 11, 2025

This PR is intended as a quick fix for the pathological case of FragmentedMapper The newly added two_level_mmapper.rs inherits most of the code from FragmentedMapper, including both impl Mmapper for TwoLevelMmapper and the test cases. The impl Mmapper part should be as correct (or incorrect) as the FragmentedMapper, but I found that those parts have program logics of slab operations intermixed with (single-chunk or bulk) memory protection. I will refactor that part in another PR, and probably make TwoLevelMmapper share code with ByteMapMmapper.

@wks wks added the PR-extended-testing Run extended tests for the pull request label Jul 11, 2025
@wks wks mentioned this pull request Jul 15, 2025
Merged
@wks wks marked this pull request as ready for review July 15, 2025 05:42
@wks
Copy link
Collaborator Author

wks commented Jul 15, 2025

With this PR applied, the optcarrot emulator can run at 58-60 FPS even with the aggressive VO bit assertion on WB enabled. It means VO bit checks still have a cost, but it is much cheaper than 30 FPS (50% total time overhead).

I also ran some benchmarks on VO bit linear scanning for internal pointers. The data is shown in #1341 (comment). In short, this PR greatly improves the performance of internal pointer checks.

@wks wks requested a review from qinsoon July 15, 2025 05:46
wks added 6 commits July 15, 2025 16:14
@wks
Merge branch 'master' into feature/fragmented-mapper-no-hash
4b15f46
@wks
Add bench for mmapper
8d02ccd
@wks
Add more benches
6844d55
@wks
Implement Drop trait for OnceOptionBox
77cb6ef 
It will never be destructed because TwoLevelMmapper is a global
singleton and it only monotonically allocates new stubs.  But we still
add support for dropping for completeness.
@wks
Add basic tests for OnceOptionBox
c52fa7b
@wks
Add doc comments
f9d64b8
@wks
Copy link
Collaborator Author

wks commented Jul 17, 2025

Here are benchmark results of running several benchmarks in the DaCapo Chopin benchmark suite. I compared the master branch and this PR. They are run on vole.moma, 10 invocations each, at 2.4x min heap size (w.r.t. G1 in OpenJDK), with 5 plans, all using compressed oops. Each data in the plots below is the relative value of this PR compared to the master branch (i.e. >1 means slowdown).

https://squirrel.anu.edu.au/plotty/wks/noproject/#0|vole-2025-07-16-Wed-164018&benchmark^build^hfac^invocation^iteration^mmtk_gc&GC^time^time.other^time.stw&|10&iteration^1^4|20&1^invocation|30&1&benchmark^hfac^mmtk_gc&build;build1|10&build^1^build2|41&Histogram%20(with%20CI)^mmtk_gc^benchmark&

The effect of this PR on the times (total time, STW time and other time) is uncertain. Certain benchmark+plan combinations have speed-up and others have slow-down. GenCopy, for example, has the best relative STW time in avrora, fop, jython, lusearch, pmd, sunflow and xalan, but the worst relative STW time in graphchi and h2o.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qinsoon qinsoon Awaiting requested review from qinsoon

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
PR-extended-testing Run extended tests for the pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Inefficient locking in FragmentedMapper::get_or_optionally_allocate_slab_table
1 participant
@wks