RFR: JDK-8080511 - Refresh of jimage support

[Lois Foltan]

On 6/18/2015 10:59 AM, Karen Kinnear wrote:
> 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.

I have reviewed and am okay with checking in the hotspot code as is once 
testing is complete.  One comment inlined below.
Lois

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

Whatever mechanism you choose to use in get_decompressor, please also 
use for image_decompressor_init( )/createSymbol() within 
imageDecompressor.cpp as well.

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