Extract copy/free arrayCritical code from StandardAccessBarrier #20648
Conversation
|
@amicic Could you please review the changes? Thanks |
| J9InternalVMFunctions *functions, void *elems, J9IndexableObject **arrayObject, jint mode) | ||
| { | ||
| if (JNI_ABORT != mode) { | ||
| I_32 sizeInElements = (I_32)indexableObjectModel->getSizeInElements(*arrayObject); |
There was a problem hiding this comment.
use int32_t to match the style with other sizeInElements definitions/casts
| @@ -109,6 +109,95 @@ MM_ObjectAccessBarrier::tearDown(MM_EnvironmentBase *env) | |||
| { | |||
| } | |||
|
|
|||
| void | |||
| MM_ObjectAccessBarrier::copyArrayCritical(J9VMThread *vmThread, GC_ArrayObjectModel *indexableObjectModel, | |||
| J9InternalVMFunctions *functions, void **data, J9IndexableObject *arrayObject, jboolean *isCopy) | |||
There was a problem hiding this comment.
minor, i'd probably prefer not passing indexableObjectModel, but rather fetch it from _ext again
@dmitripivkine, opinion?
There was a problem hiding this comment.
I agree, let's get it from _ext
|
|
||
| void | ||
| MM_ObjectAccessBarrier::copyStringCritical(J9VMThread *vmThread, GC_ArrayObjectModel *indexableObjectModel, | ||
| J9InternalVMFunctions *functions, jchar **data, J9JavaVM *javaVM, |
There was a problem hiding this comment.
like with indexableObjectModel, probably cleaner not to pass javaVM
| @@ -408,43 +397,27 @@ MM_StandardAccessBarrier::jniReleasePrimitiveArrayCritical(J9VMThread* vmThread, | |||
| { | |||
| J9JavaVM *javaVM = vmThread->javaVM; | |||
| J9InternalVMFunctions *functions = javaVM->internalVMFunctions; | |||
There was a problem hiding this comment.
I feel like this is not used any more here (but only in new helper methods) or in any other get/release method. If so, we should not have it here and then not to pass it, but let the helpers fetch it.
| void | ||
| MM_ObjectAccessBarrier::copyStringCritical(J9VMThread *vmThread, GC_ArrayObjectModel *indexableObjectModel, | ||
| J9InternalVMFunctions *functions, jchar **data, J9JavaVM *javaVM, | ||
| J9IndexableObject *valueObject, J9Object *stringObject, jboolean *isCopy, bool isCompressed) |
There was a problem hiding this comment.
Looks like isCompressed can be removed from parameters and calculated inside this method. I believe it is not used on caller side, so there is no need to get it there. Is it correct?
There was a problem hiding this comment.
Standard does not, but seems like VLHGC will
There was a problem hiding this comment.
so, i'd just leave it for now, and when we review VLHGC, we'll decide if we should pass or not
LinHu2016
force-pushed
the
off-heap-incremental7
branch
from
9203378 to
f106683
Compare
|
jenkins test sanity xLinux,aix jdk11 |
| * @param isCopy[out] if it true, the array is successfully copied. | ||
| * @param isCompressed[in] if it is compressed. | ||
| */ | ||
| void copyStringCritical(J9VMThread *vmThread, jchar **data |
-Extract copyArrayCritical copyBackArrayCritical, copyStringCritical and freeStringCritical code from StandardAccessBarrier to ObjectAccessBarrier(baseclass). Signed-off-by: lhu <[email protected]>
LinHu2016
force-pushed
the
off-heap-incremental7
branch
from
f106683 to
f8423df
Compare
|
jenkins test sanity xLinux,aix jdk11 |
|
sanity.functional fails I see it recently changed |
|
Internal Axxon build 82030 passed. Ignoring sanity.functional failure and merging... |
|
I think it's a timing thing. New test material with older JVM. |
-Extract copyArrayCritical copyBackArrayCritical, copyStringCritical
and freeStringCritical code from StandardAccessBarrier to
ObjectAccessBarrier(baseclass).