Remove Space/PageResource/Allocator type parameters#108
Merged
Conversation
caizixian
reviewed
Jun 22, 2020
caizixian
approved these changes
Jun 24, 2020
steveblackburn
approved these changes
Jun 24, 2020
Contributor
steveblackburn
left a comment
steveblackburn
left a comment
There was a problem hiding this comment.
LGTM
This is good cleanup/refactoring.
wenyuzhao
approved these changes
Jun 24, 2020
qinsoon
commented
Jun 24, 2020
This was referenced Jun 24, 2020
caizixian
added
PR-ready-to-merge
PR-approved
Member
|
Bindings need to be fixed |
Member
Author
We need to merge the binding first, as the checks only check against the master of the binding. I will do this. |
Member
Author
|
I will merge this as the correctness test passed. The performance comparison failed due to this issue #95. |
wenyuzhao
added a commit
to wenyuzhao/mmtk-core
that referenced
this pull request
Sep 14, 2021
* Update to mmtk-core PR mmtk#108 * Fix MMTk mutator struct layout due to the use of trait objects _Note: Force merge this PR. CI tests failed because the mmtk-core dependency still points to master instead of the pr#108 branch. This is expected to be fixed automatically after the mmtk-core-pr-108 is merged._ Co-authored-by: Wenyu Zhao <[email protected]>
qinsoon
pushed a commit
to qinsoon/mmtk-core
that referenced
this pull request
Mar 28, 2023
Previously the MMTk coordinator thread pretents to be the VM thread and
calls SafepointSynchronize::begin() directly. This makes it race with
the real VM thread.
This commit introduces a dedicated "VM companion thread". The companion
therad requests the VM thread to execute a stop-the-world VM operation
(a VMOp_ThirdPartyHeapOperation), and the VM operation waits for the
stop-the-world GC to finish and lets the VM thread start the world
again. This ensures the real VM thread is the only thread that calls
SafepointSynchronize::begin(). The MMTk binding initiates stack
scanning during stop-the-world.
Also made minor fixes to MMTKCollectorThread and MMTKContextThread
- Removed is_VM_thread and is_GC_thread methods.
- It is not the VM thread.
- The `is_GC_thread` method is never used in the real VM thread,
and is removed in the upstream in a later revision.
- Initialize their native thread names on start.
- The native names are visible in GDB when debugging.
- Prepended "MMTk" to their names to make them more obvious.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Types with unnecessary type parameters such as
Space<VM, PR>,PageResource<VM, Space>,Allocator<VM, PR>make it impossible to write polymorphic code. This PR removes unnecessary type parameters, and use trait objects for those types wherever possible.Changes:
Space<VM, PR>->Space<VM>PageResource<VM, Space>->PageResource<VM>Allocator<VM, PR>->Allocator<VM>prfromCommonSpaceto each specific space so that each space implementation still knows the type of page resource to call methods specific to the page resource.This PR should fix most of issue #60, however, we should discuss separately. This PR also solves #19.