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.
