-
Notifications
You must be signed in to change notification settings - Fork 720
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
Atomic-free JNI work #1762
Atomic-free JNI work #1762
Conversation
- re-enable JNI critical for atomic-free - fix JNI check bug related to critical Signed-off-by: Graham Chapman <graham_chapman@ca.ibm.com>
jenkins test sanity zlinux jdk8 |
jenkins test sanity plinux jdk9 |
@@ -36,6 +36,10 @@ | |||
void | |||
MM_JNICriticalRegion::reacquireAccess(J9VMThread* vmThread, UDATA accessMask) | |||
{ | |||
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI) | |||
// Current thread must have entered the VM before acquiring exclusive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if these asserts could be enabled all of the time? Not asking you to change them just thinking out loud.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag has no meaning unless the ifdef is true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry thought that was a VMAccess check lol. My bad!
hasVMAccess = true; | ||
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */ | ||
MM_JNICriticalRegion::enterCriticalRegion(vmThread); | ||
if (!hasVMAccess) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also just making a note for myself to come look at the hasVMAccess. Not sure how one thread entering here could have it and another thread not have it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this code can be refactored/optimized at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just waiting for the CI builds to finish
Signed-off-by: Graham Chapman graham_chapman@ca.ibm.com