RFR: 8276658: Clean up JNI local handles code
David Holmes
dholmes at openjdk.java.net
Thu Nov 11 07:10:35 UTC 2021
On Wed, 10 Nov 2021 17:16:29 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
> JNI Local handles can only be created by JavaThread (there's an assert in make_local) but the fields are added to Thread.
> Move the fields to JavaThread and adding JavaThread* argument.
> Also, the global freelist isn't very useful now that global JNI handles don't use JNIHandleBlock, so the locking that claims incorrectly to block for safepoint is removed.
> Lastly, there's at least 3 places that duplicate pushing a new JNIHandleBlock to the thread for temporarily adding JNI local handles. These have been moved to common code with a JNIHandleMark object, moved from jvmci code.
> The commits are separate to help reviewing, but the entire change has been tested together with tier1-6.
> The commits in this change have been performance tested individually and together with no meaningful differences from mainline.
Hi Coleen,
Nice cleanup and refactoring! I'm not familiar with all the details but the reshuffling looks good to me.
One query and one minor issue below.
Thanks,
David
src/hotspot/share/compiler/compileBroker.cpp line 2324:
> 2322: // Remove the JNI handle block after the ciEnv destructor has run in
> 2323: // the previous block.
> 2324: pop_jni_handle_block();
Does the fact the JNIHandleMark destructor won't get executed until much later, at the end of this method, make any difference?
src/hotspot/share/runtime/vmThread.hpp line 63:
> 61: class VMThread: public NamedThread {
> 62: private:
> 63: volatile bool _is_running;
I don't see this being initialized to false.
-------------
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6336
More information about the serviceability-dev
mailing list