Fix PowerPC specific issues #18637
Fix PowerPC specific issues #18637
Conversation
|
jenkins compile amac jdk21 |
runtime/vm/ContinuationHelpers.cpp
Outdated
| ContinuationState returnContinuationState = 0; | ||
| volatile ContinuationState oldContinuationState = 0; | ||
| volatile ContinuationState returnContinuationState = 0; | ||
| volatile ContinuationState newContinuationState = 0; |
There was a problem hiding this comment.
I'd remove volatile here, and in other methods where used for local variables (not for a pointer to a volatile location)
runtime/gc_base/GCExtensions.cpp
Outdated
| @@ -319,7 +319,7 @@ MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9obj | |||
| { | |||
| bool needScan = false; | |||
| #if JAVA_SPEC_VERSION >= 19 | |||
| ContinuationState volatile *continuationStatePtr = VM_ContinuationHelpers::getContinuationStateAddress(vmThread, objectPtr); | |||
| volatile ContinuationState *continuationStatePtr = VM_ContinuationHelpers::getContinuationStateAddress(vmThread, objectPtr); | |||
There was a problem hiding this comment.
while I prefer the new style, the old style should still be valid with identical effects - so I'd minimize the change and revert to the old style
|
jenkins compile aix jdk21 https://openj9-jenkins.osuosl.org/job/PullRequest-OpenJ9/4861/ |
runtime/vm/ContinuationHelpers.cpp
Outdated
|
|
||
| } else { | ||
| oldContinuationState = returnContinuationState; | ||
| } |
There was a problem hiding this comment.
this is hard to follow
either we need more comments or better we just start this loop from scratch: read oldContinuationState from continuationStatePtr and check isConcurrentlyScanned against oldContinuationState...
-Replace do_{_if_(condition)_{_..._continute;_}_}while_{false}; with
for(;;)_{_if_(condition)_{...}_else{break;_}_} for an infinite loop
with a break logic to handle waiting concurrent gc finish during
continuation mounting. compiler might generate wrong logic for
do_while infinite loop.
-fix small hole during mounting synchronization with concurrent
scanning(for the case concurrent scanning start just before resetting
pendingmount flag-before reading the value of state for atomic update)
- new assertion check for catching issue earlier.
signed-off-by: hulin <[email protected]>
|
The change looks good now (assuming it still fixes the problem), although I have hard time believing that the part that changed while to for loop had any effect. We'll have to investigate and understand that part more... |
|
jenkins test sanity aix,xLinux,win jdk21 |
|
jenkins test sanity win jdk21 |
|
Windows machines don't seem to be working too well atm. Edit - tried again later and it worked. |
|
jenkins test sanity xmac jdk21 |
| Assert_VM_true(VM_ContinuationHelpers::isMountedWithCarrierThread(oldContinuationState, currentThread)); | ||
| Assert_VM_true(VM_ContinuationHelpers::isPendingToBeMounted(oldContinuationState)); | ||
| ContinuationState newContinuationState = oldContinuationState; | ||
| VM_ContinuationHelpers::resetPendingState(&newContinuationState); | ||
| returnContinuationState = VM_AtomicSupport::lockCompareExchange(continuationStatePtr, oldContinuationState, newContinuationState); | ||
| } while (oldContinuationState != returnContinuationState); | ||
| Assert_VM_false(VM_ContinuationHelpers::isPendingToBeMounted(*continuationStatePtr)); | ||
| Assert_VM_false(VM_ContinuationHelpers::isConcurrentlyScanned(*continuationStatePtr)); |
There was a problem hiding this comment.
Actually, I think this assert is ambitious since it's based on newly re-read state. Another scan can start just after this one finished. It could be a rescan within the same cycle (concurrent card cleaning) or in another cycle of same type or overlapping cycle of the complement type.
My bad, no new scan can start while this is fully mounted. So, the assert is ok.
|
While windows and mac sanity testing failed, I don't see any real functional problems. |
|
Pls cherry pick for the 0.42 and 0.43 branches. |
-Replace
do_{_if_(condition)_{_..._continute;_}_}while_{false};withfor(;;)_{_if_(condition)_{...}_else{break;_}_}for an infinite loopwith a break logic to handle waiting concurrent gc finish during
continuation mounting. compiler might generate wrong logic for
do_while infinite loop.
-fix small hole during mounting synchronization with concurrent
scanning(for the case concurrent scanning start just before resetting
pendingmount flag-before reading the value of state for atomic update)
issue: #16728 (comment) [ISSUE3]
Signed-off-by: hulin [email protected]