RFR: JDK-8080511 - Refresh of jimage support

[Karen Kinnear]

Code review for the hotspot component:

Thank you for the cleanups to jvm.cpp to make them JVM_ENTRY and fix thread_state.

If you pass the testing and Lois ok's the hotspot code, then I am ok with checking in the
hotspot code as is.

Comments for next round of changes:

1. mutexLocker.cpp - new lock should say "false" not "true" for whether the VMThread should block

2. os.hpp
restartable_read_at is added but I don't see it used

3. imageFile.cpp
We've been asked to always used the counted form of strcpy - i.e. strncpy  to ensure we never
have buffer overruns. e.g. 311, 437, ...

4. jvm.cpp - seems odd that the interface would need to know if the resource was compressed 
or uncompressed. In the next round of API design, seems like the java code would pass in
the uncompressedAddress and let the internals handle buffering the compressed data and
doing the uncompressing transparently.

5. imageDecompressor.hpp
get_decompressor
I think you've mixed your exception handling approaches. You have a CHECK_NULL for new_symbol
call, which means it will throw an exception if it returns NULL. Then you check for HAS_PENDING_EXCEPTION
which is the logic you would use if did not have the CHECK, but then you throw the exception away.
If you want to throw an exception from get_decompressor you should pass in the last argument as TRAPS macro
which will give you the THREAD.

see macros in exceptions.hpp

So the way it is right now, you will throw an exception.

You have a choice - you can throw an exception
  pass in TRAPS to get_decompressor, keep the CHECK_NULL and remove the if HAS_PENDING_EXCEPTION.
  - this is the recommended approach

Alternative - change CHECK_NULL to pass in THREAD. Keep the rest. Given you are planning to take this
out of the vm - this would make more sense.

thanks,
Karen



On Jun 17, 2015, at 8:08 PM, Jim Laskey (Oracle) wrote:

> https://bugs.openjdk.java.net/browse/JDK-8080511
> 
> This is an long overdue refresh of the jimage support in the JDK9-dev repo.  This includes native support for reading jimage files, improved jrt-fs (java runtime file system) support for retrieving modules and packages from the runtime, and improved performance for langtools in the presence of jrt-fs.
> 
> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-top <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-top>
> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-jdk <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-jdk>
> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-hotspot <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-hotspot>
> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-langtools <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-langtools>
> 
> 
> Details:
> 
> - jrt-fs provides access, via the nio FileSystem API, to the classes in a .jimage file, organized by module or by package.
> - Shared code for jimage support converted to native.  Currently residing in hotspot, but will migrate to it’s own jdk library https://bugs.openjdk.java.net/browse/JDK-8087181 <https://bugs.openjdk.java.net/browse/JDK-8087181>
> - A new archive abstraction for class/resource sources.
> - java based implementation layer for jimage reading to allow backport to JDK8 (jrt-fs.jar - IDE support.)
> - JNI support for jimage into hotspot.
> - White box tests written to exercise native jimage support.
>