Fix stop-the-world race. #108
Merged
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
wks
force-pushed
the
fix-safepoint-sync2
branch
from
1224056 to
e618940
Compare
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.
wks
force-pushed
the
fix-safepoint-sync2
branch
from
e618940 to
7b4d068
Compare
|
I made some updates. Added comments, removed stale code, and added "MMTk" before the names of the context and the collector threads. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This PR fixes the race bug during stop-the-world.
This PR has two associated PRs:
openjdk: Fix stop-the-world bug. openjdk#9mmtk-core: Fix stop-the-world race. mmtk-core#450In short, the bug exists because the MMTk coordinator calls
SafepointSynchronnize::begin()directly. This races with the real VM thread.Fixing this involves introducing a dedicated "companion thread". This thread receives stop-the-world requests from the MMTk core. It will let the VM thread execute a VM Operation which does nothing but waiting for another start-the-world request from the MMTk. In this way, mmtk-core can request stop-the-world, then continue to do GC, then request start-the-world.
Concretely, the changes include:
openjdk: TheSafepointSynchronize::begin()method no longer reports which threads have stopped, because this method is used in all VMOperations that needs STW, not just GC.openjdk: Added a new VMOperation identifierVMOp_ThirdPartyHeapOperation.mmtk-openjdk: Introduce aMMTkVMCompanionThread. This is a new non-Java thread. The only thing it does, upon request from MMTk core, is to create aVM_MMTkSTWOperationand callVMThread::executeto execute it.The VM Thread then stops the world, executes the
VM_MMTkSTWOperation. It finishes when the mmtk-core requests start-the-world. Then the VM Thread will start the world and unblock the companion threadThe reason why we introduce a companion thread is that
VMThread::executewill block the caller until the VM thread finished executing the VM operation. There is an alternative. We could use "asynchronous" (non-blocking) VM operations, but this feature is removed in a later OpenJDK version.mmtk-core: Because now the only thread that callsSafepointSynchronize::begin()andSafepointSynchronize::end()is the real VM thread, we no longer need to let the coordinator request stop/start-the-world.There could be some performance impact because of the companion thread we introduced, so performance tests are needed. Some preliminary tests show no obvious performance regression.
Merging strategy
All three repositories should be updated at the same time.
If this is impossible, we should update
mmtk-coreandopenjdkfirst. Then apply code changes tommtk-openjdkand, at the same time, update its dependencies so that it depends on the updatedmmtk-coreandopenjdk.