[13] RFR(L) 8220623: [JVMCI] Update JVMCI to support JVMCI based Compiler compiled into shared library

Kim Barrett kim.barrett at oracle.com
Thu Apr 4 20:49:27 UTC 2019


> On Apr 4, 2019, at 3:22 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> New delta:
> http://cr.openjdk.java.net/~kvn/8220623/webrev_delta.06/
> 
> Full:
> http://cr.openjdk.java.net/~kvn/8220623/webrev.06/
> 
> New changes are based on Kim and Stefan suggestions:
> 
> - Moved JVMCI::oops_do() from JNIHandles to places where it should be called.
> - Moved JVMCI cleanup task to the beginning of ParallelCleaningTask::work().
> - Used JVMCI_ONLY macro with COMMA.
> - Disable JVMCI build on SPARC. We don't use it - neither Graal or AOT are built on SPARC. Disabling also helps to find missing JVMCI guards.
> 
> I ran hs-tier1-3 testing - it passed (hs-tier3 includes graal testing).
> I started hs-tier4..8-graal testing.
> I will do performance testing next.

------------------------------------------------------------------------------
src/hotspot/share/jvmci/jvmci.hpp

There is this new header file, declaring the JVMCI class.  But the
implementation seems to all be in JVMCIRuntime.cpp.  That's pretty
atypical code organization in HotSpot.

------------------------------------------------------------------------------
src/hotspot/share/jvmci/jvmci.hpp
  46   static JNIHandleBlock* _object_handles;

We moved away from JNIHandleBlock for "globals" (JNI global handles,
JNI global weak handles), replacing those uses with OopStorage,
because JNIHandleBlock and the way it was being used for "globals" had
various thread-safety issues.  JNIHandleBlock was retained for use
with local handles, where thread-safety isn't an issue for the most
part.  Trying to have a chain of blocks data structure that supported
both usage models was deemed to impose undesirable overhead on one or
the other (or both) use cases.  So, how have those issues been
addressed in the new uses introduced by JVMCI?  Should JVMCI be using
OopStorage rather than JNIHandleBlock?  (Yes, it should.)

Further searching, and JVMCI::is_global_handle is racy and can crash,
just like JDK-8174790.

------------------------------------------------------------------------------
src/hotspot/share/jvmci/jvmci.hpp
src/hotspot/share/jvmci/jvmciRuntime.cpp

1198 jobject JVMCI::make_global(Handle obj) {
1199   assert(_object_handles != NULL, "uninitialized");
1200   MutexLocker ml(JVMCI_lock);
1201   return _object_handles->allocate_handle(obj());
1202 }

This is returning a new kind of "jobject" that JNI knows nothing
about!  Any sort of JNI handle checking (such as is enabled by
-Xcheck:jni) will fail for one of these.

It looks like JVMCI used to be using JNI directly, but there has been
some decision to separate it into it's own near copy.  Why?

Maybe these pseudo-jobjects aren't supposed to ever be passed to a
"normal" JNI function.  If that's true, they should have a different
type, and not be jobject at all.

------------------------------------------------------------------------------
src/hotspot/share/prims/jni.cpp
1321   Klass* klass = java_lang_Class::as_Klass(JNIHandles::resolve_non_null(clazz));

Pre-existing: why are we resolving clazz twice, here and a few lines
away for the is_primitive check?

------------------------------------------------------------------------------

I've just started looking at the MetadataHandle stuff, and don't have
any comments there yet.




More information about the graal-dev mailing list