RFR: 8263709: Cleanup THREAD/TRAPS/CHECK usage in JRT_ENTRY routines

[Coleen Phillimore]

On Thu, 18 Mar 2021 02:48:05 GMT, David Holmes <dholmes at openjdk.org> wrote:

> All JRT_ENTRY routines are required to have a parameter, "JavaThread* thread", which must be the current thread. The JRT_ENTRY, and related macros, then use this parameter and also expose it as THREAD for use with exception macros. But the fact "thread" is the current thread is lost in some routines and we see strange calls to other code that pass both "thread" and "THREAD" as distinct parameters - primarily when a TRAPS method is involved.
> 
> This should be cleaned up along with a general check on misuse of TRAPS/THREAD. 
> 
> Testing: tiers 1-3
> 
> Thanks,
> David

Can you state the convention that you're trying to establish?
I would like it to be when there is TRAPS, use THREAD for Handles, otherwise use current if passed as JavaThread* current.

src/hotspot/share/runtime/sharedRuntime.cpp line 1217:

> 1215:     assert(fr.is_entry_frame(), "must be");
> 1216:     // fr is now pointing to the entry frame.
> 1217:     callee_method = methodHandle(thread, fr.entry_frame_call_wrapper()->callee_method());

Ok, now this just seems arbitrary to me.  I don't see why we should introduce a JavaThread* thread, when there's a perfectly good THREAD available here, and THREAD is fine here.  There's a lot of code that uses THREAD as a Handle parameter and there's nothing wrong  with it.  If it's lower case 'thread', one has to wonder and look around to see if it's the current thread (changing the convention to 'current' was good where you did that).  Don't change these ones.  It seems like a conflict of your preferences to what's in the code, and I vote for what's used in the code everywhere.  Removing TRAPS from functions that don't throw an exception is also good. But I don't see the point of this at all and I think it's worse.

src/hotspot/share/runtime/sharedRuntime.cpp line 1340:

> 1338: // and are patched with the real destination of the call.
> 1339: methodHandle SharedRuntime::resolve_sub_helper(bool is_virtual, bool is_optimized, TRAPS) {
> 1340:   JavaThread* thread = THREAD->as_Java_thread();

Same comment as above.

src/hotspot/share/runtime/sharedRuntime.cpp line 2124:

> 2122:     Atomic::inc(BiasedLocking::slow_path_entry_count_addr());
> 2123:   }
> 2124:   Handle h_obj(thread, obj);

Can you make this one 'current'?

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

Changes requested by coleenp (Reviewer).

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