RFR: 8265753: Remove manual JavaThread transitions to blocked [v3]

Robbin Ehn rehn at openjdk.java.net
Mon May 17 08:29:41 UTC 2021


On Thu, 13 May 2021 05:27:42 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixes for Dan
>
> src/hotspot/share/runtime/interfaceSupport.inline.hpp line 207:
> 
>> 205:     assert(_thread->thread_state() == _thread_in_vm, "coming from wrong thread state");
>> 206:     assert(!_thread->has_last_Java_frame() || _thread->frame_anchor()->walkable(), "Unwalkable stack in vm->native transition");
>> 207:     _thread->set_thread_state(_thread_in_native);
> 
> After doing a heavyweight transition for many many years I find it somewhat disconcerting to suddenly be told it is not necessary. If we are _thread_in_vm and so unsafe, then no handshake/safepoint can have been processed, so there is nothing to check. Makes sense. But how long has that been true? Could we have simplified this years ago or it is a result of other changes?

I looked over the transitions this fall, I don't know but there are some of these oddities.
One problem is the some of the generic named transitions RAII are actually supposed to be used only at certain places and some optimizations have been added.
E.g. in this case someone have figured out that we can remove make_walkable(), but as a generic rule when going to a safe state (blocked/native)  you should call make_walkable. My guess is that calling make_walkable on every JNI/JVM TI call was expensive on Solaris, so someone optimized that.
Now I'm unsure if that is actually true (I my feeling is no, because we could accidentally call Java if we hit e.g. class loading or something). And the optimization is no longer valid, make_walkable() is light weight on current hardware AFAIK.

I updated to what I see as a correct transition to an safe state without any short cuts.
I also notice I missed an assert from the previous code.

Re-running all test, since these kinds of changes should not be taken lightly.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3875


More information about the serviceability-dev mailing list