RFR: JDK-8080511 - Refresh of jimage support
lois.foltan at oracle.com
Thu Jun 18 17:55:18 UTC 2015
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.
> 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
> 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.
> On Jun 17, 2015, at 8:08 PM, Jim Laskey (Oracle) wrote:
>> 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>
>> - 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.
More information about the jigsaw-dev