-
Notifications
You must be signed in to change notification settings - Fork 78
Refactor both Mmapper impls into ChunkStateMmapper #1369
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.
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
Draft
wks
wants to merge
17
commits into
mmtk:master
Choose a base branch
from
wks:feature/mmapper-refactoring2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We make Mmapper a struct instead of trait. It will be solely responsible for transitioning the map states. Existing implementations, including `ByteMapMmapper` and `TwoLevelMmapper`, become the storage backends of `Mmapper`. They are only responsible for maintaining the (one-level or two-level) array of `Atomic<MapState>`, and provide high-level functions for bulk-setting and bulk-transitioning map states. Methods of `MapState` are moved to `Mmapper`.
They are now only responsible for the storage, not mmapping.
We keep the Mmapper trait, and create a ChunkStateMmapper that work at chunk granularity.
Unlike #1345, this PR retains the |
4 tasks
It is no longer a constant, but a property of the `Mmapper`.
Just use BYTES_IN_CHUNK
Because `ensure_mapped` is called very often, we want it to be fast.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The two existing
Mmapper
implementations, namelyByteMapMmapper
andTwoLevelMmapper
have much duplicated code.We introduce
ChunkStateMmapper
which implements a chunk-grainedMmapper
backed by a logical array ofMapState
elements, one for each chunk. It is responsible for actually callingmmap
on memory regions. TheByteMapMmapper
andTwoLevelMmapper
becomeByteMapStateStorate
andTwoLevelStateStorage
, respectively, and they are only responsible for holding the per-chunk states and iterating over ranges of memory to helpChunkStateMmapper
. We retains the traitMmapper
so that we can still implement other memory mappers that work at different granularities.MapState
is now a pure data type, and state-transitioning methods previously implemented inimpl MapState
are inlined into methods ofChunkStateMmapper
.We also removed the constant
MMAP_CHUNK_BYTES
. It was defined asBYTES_IN_CHUNK
, but it seemed to convey the message that the "mmap chunk" is special and can have a size that is different from "chunk". We instead added a functionMmapper::granularity()
which returns the actual granularity of the mmapper.