RFR: JDK-8080511 - Refresh of jimage support

[Jean-Francois Denise]

Hi Karen,
On 18 Jun 2015, at 16:59, Karen Kinnear <karen.kinnear at oracle.com> wrote:

> Code review for the hotspot component:
> 
> Thank you for the cleanups to jvm.cpp to make them JVM_ENTRY and fix thread_state.
> 
You are welcome.

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

Thanks.

> Comments for next round of changes:
> 
> 1. mutexLocker.cpp - new lock should say "false" not "true" for whether the VMThread should block

Done, along with not-leaf.

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

Removed.

> 
> 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, we will implement 3/4/5 in the next round.

> 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.
>> 
>