Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Atomic-free JNI work #1762

Merged
merged 1 commit into from
Apr 25, 2018
Merged

Atomic-free JNI work #1762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions runtime/gc_glue_java/JNICriticalRegion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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!

Assert_MM_false(vmThread->inNative);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_DEBUG_VM_ACCESS)) {
Assert_MM_true(J9_VM_FUNCTION(vmThread, currentVMThread)(vmThread->javaVM) == vmThread);
}
Expand Down Expand Up @@ -73,6 +77,10 @@ MM_JNICriticalRegion::reacquireAccess(J9VMThread* vmThread, UDATA accessMask)
void
MM_JNICriticalRegion::releaseAccess(J9VMThread* vmThread, UDATA* accessMask)
{
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
// Current thread must have entered the VM before acquiring exclusive
Assert_MM_false(vmThread->inNative);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
if (J9_ARE_ANY_BITS_SET(vmThread->publicFlags, J9_PUBLIC_FLAGS_DEBUG_VM_ACCESS)) {
Assert_MM_true(J9_VM_FUNCTION(vmThread, currentVMThread)(vmThread->javaVM) == vmThread);
}
Expand Down
30 changes: 13 additions & 17 deletions runtime/gc_modron_standard/StandardAccessBarrier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

/*******************************************************************************
* Copyright (c) 1991, 2018 IBM Corp. and others
*
Expand Down Expand Up @@ -282,10 +281,8 @@ MM_StandardAccessBarrier::jniGetPrimitiveArrayCritical(J9VMThread* vmThread, jar
// acquire access and return a direct pointer
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
vmThread->jniCriticalDirectCount += 1;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
J9IndexableObject *arrayObject = (J9IndexableObject*)J9_JNI_UNWRAP_REFERENCE(array);
data = (void *)_extensions->indexableObjectModel.getDataPointerForContiguous(arrayObject);
if(NULL != isCopy) {
Expand Down Expand Up @@ -347,11 +344,10 @@ MM_StandardAccessBarrier::jniReleasePrimitiveArrayCritical(J9VMThread* vmThread,
Trc_MM_JNIReleasePrimitiveArrayCritical_invalid(vmThread, arrayObject, elems, data);
}

MM_JNICriticalRegion::exitCriticalRegion(vmThread);

#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
vmThread->jniCriticalDirectCount -= 1;
VM_VMAccess::inlineExitVMToJNI(vmThread);
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
}
}
Expand Down Expand Up @@ -418,12 +414,12 @@ MM_StandardAccessBarrier::jniGetStringCritical(J9VMThread* vmThread, jstring str
} else {
// acquire access and return a direct pointer
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
vmThread->jniCriticalDirectCount += 1;
hasVMAccess = true;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
if (!hasVMAccess) {
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
hasVMAccess = true;
}
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
J9Object *stringObject = (J9Object*)J9_JNI_UNWRAP_REFERENCE(str);
J9IndexableObject *valueObject = (J9IndexableObject*)J9VMJAVALANGSTRING_VALUE(vmThread, stringObject);

Expand Down Expand Up @@ -474,12 +470,12 @@ MM_StandardAccessBarrier::jniReleaseStringCritical(J9VMThread* vmThread, jstring
*/
// direct pointer, just drop access
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
vmThread->jniCriticalDirectCount -= 1;
hasVMAccess = true;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
if (!hasVMAccess) {
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
hasVMAccess = true;
}
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
}

if (hasVMAccess) {
Expand Down
32 changes: 14 additions & 18 deletions runtime/gc_realtime/RealtimeAccessBarrier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

/*******************************************************************************
* Copyright (c) 1991, 2016 IBM Corp. and others
* Copyright (c) 1991, 2018 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -404,10 +403,8 @@ MM_RealtimeAccessBarrier::jniGetPrimitiveArrayCritical(J9VMThread* vmThread, jar
// acquire access and return a direct pointer
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
vmThread->jniCriticalDirectCount += 1;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
data = (void *)_extensions->indexableObjectModel.getDataPointerForContiguous(arrayObject);
if(NULL != isCopy) {
*isCopy = JNI_FALSE;
Expand Down Expand Up @@ -472,11 +469,10 @@ MM_RealtimeAccessBarrier::jniReleasePrimitiveArrayCritical(J9VMThread* vmThread,
Trc_MM_JNIReleasePrimitiveArrayCritical_invalid(vmThread, arrayObject, elems, data);
}

MM_JNICriticalRegion::exitCriticalRegion(vmThread);

#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
vmThread->jniCriticalDirectCount -= 1;
VM_VMAccess::inlineExitVMToJNI(vmThread);
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
}
}
Expand Down Expand Up @@ -549,12 +545,12 @@ MM_RealtimeAccessBarrier::jniGetStringCritical(J9VMThread* vmThread, jstring str
} else {
// acquire access and return a direct pointer
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
vmThread->jniCriticalDirectCount += 1;
hasVMAccess = true;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
if (!hasVMAccess) {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

VM_VMAccess::inlineEnterVMFromJNI(vmThread);
hasVMAccess = true;
}
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
J9Object *stringObject = (J9Object*)J9_JNI_UNWRAP_REFERENCE(str);
J9IndexableObject *valueObject = (J9IndexableObject*)J9VMJAVALANGSTRING_VALUE(vmThread, stringObject);

Expand Down Expand Up @@ -606,12 +602,12 @@ MM_RealtimeAccessBarrier::jniReleaseStringCritical(J9VMThread* vmThread, jstring
} else {
// direct pointer, just drop access
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
vmThread->jniCriticalDirectCount -= 1;
hasVMAccess = true;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
if (!hasVMAccess) {
VM_VMAccess::inlineEnterVMFromJNI(vmThread);
hasVMAccess = true;
}
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
}

if (hasVMAccess) {
Expand Down
19 changes: 1 addition & 18 deletions runtime/gc_vlhgc/VLHGCAccessBarrier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

/*******************************************************************************
* Copyright (c) 1991, 2017 IBM Corp. and others
* Copyright (c) 1991, 2018 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -289,11 +288,7 @@ MM_VLHGCAccessBarrier::jniGetPrimitiveArrayCritical(J9VMThread* vmThread, jarray
vmThread->jniCriticalCopyCount += 1;
} else {
// acquire access and return a direct pointer
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
vmThread->jniCriticalDirectCount += 1;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
Assert_MM_true(vmThread->publicFlags & J9_PUBLIC_FLAGS_VM_ACCESS);
arrayObject = (J9IndexableObject*)J9_JNI_UNWRAP_REFERENCE(array);
data = (void *)_extensions->indexableObjectModel.getDataPointerForContiguous(arrayObject);
Expand Down Expand Up @@ -363,11 +358,7 @@ MM_VLHGCAccessBarrier::jniReleasePrimitiveArrayCritical(J9VMThread* vmThread, ja
Assert_MM_true((*criticalCount) > 0);
MM_AtomicOperations::subtract(criticalCount, 1);
#endif /* defined(J9VM_GC_MODRON_COMPACTION) || defined(J9VM_GC_MODRON_SCAVENGER)*/
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
vmThread->jniCriticalDirectCount -= 1;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
}
VM_VMAccess::inlineExitVMToJNI(vmThread);
}
Expand Down Expand Up @@ -433,11 +424,7 @@ MM_VLHGCAccessBarrier::jniGetStringCritical(J9VMThread* vmThread, jstring str, j
vmThread->jniCriticalCopyCount += 1;
} else {
// acquire access and return a direct pointer
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
vmThread->jniCriticalDirectCount += 1;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::enterCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
Assert_MM_true(vmThread->publicFlags & J9_PUBLIC_FLAGS_VM_ACCESS);
data = (jchar*)_extensions->indexableObjectModel.getDataPointerForContiguous(valueObject);

Expand Down Expand Up @@ -497,11 +484,7 @@ MM_VLHGCAccessBarrier::jniReleaseStringCritical(J9VMThread* vmThread, jstring st
Assert_MM_true((*criticalCount) > 0);
MM_AtomicOperations::subtract(criticalCount, 1);
#endif /* defined(J9VM_GC_MODRON_COMPACTION) || defined(J9VM_GC_MODRON_SCAVENGER)*/
#if defined(J9VM_INTERP_ATOMIC_FREE_JNI)
vmThread->jniCriticalDirectCount -= 1;
#else /* J9VM_INTERP_ATOMIC_FREE_JNI */
MM_JNICriticalRegion::exitCriticalRegion(vmThread);
#endif /* J9VM_INTERP_ATOMIC_FREE_JNI */
}
VM_VMAccess::inlineExitVMToJNI(vmThread);
}
4 changes: 2 additions & 2 deletions runtime/jnichk/jnicheck.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ jniCheckArgs(const char *function, int exceptionSafe, int criticalSafe, J9JniChe
}

if (criticalSafe != CRITICAL_SAFE) {
if ((vmThread->jniVMAccessCount != 0) || (vmThread->jniCriticalCopyCount != 0) || (vmThread->jniCriticalDirectCount != 0)) {
if ((vmThread->jniCriticalCopyCount != 0) || (vmThread->jniCriticalDirectCount != 0)) {
if (criticalSafe == CRITICAL_WARN) {
if (warn) {
j9nls_printf(PORTLIB, J9NLS_WARNING, J9NLS_JNICHK_CRITICAL_UNSAFE_WARN, function);
Expand Down Expand Up @@ -1732,7 +1732,7 @@ methodExitHook(J9HookInterface** hook, UDATA eventNum, void* eventData, void* us
/* check for unreleased memory (a warning) before checking the critical section count */
jniCheckForUnreleasedMemory( (JNIEnv*)vmThread );

if (vmThread->jniVMAccessCount > 0) {
if ((vmThread->jniCriticalCopyCount != 0) || (vmThread->jniCriticalDirectCount != 0)) {
jniCheckFatalErrorNLS((JNIEnv*)vmThread, J9NLS_JNICHK_UNRELEASED_CRITICAL_SECTION);
}

Expand Down