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

[David Holmes]

On Thu, 18 Mar 2021 12:47:08 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> 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.
>
> The intent is that THREAD, ideally, is only used in relation to exception throwing/catching and you can't tell that if you use it just for convenience everywhere else. I haven't introduced a new local "thread" when there is only a single use, but if there are multiple uses then using "thread" or "current" is IMO much clearer because we can clearly isolate the the code that relates to exceptions and the code that does not. That is what I'm trying to achieve here. It wont be perfect and not every use of THREAD will be replaced, but it will be a great improvement over what we currently have.
> In this code I would love to change "thread" to "current" but the JRT macros require the use of "thread" and changing that would be too big a change. Note that in the code above I'm not introducing "thread" it is already there.

Correction, sorry I was misled by the mis-quoted code in the discussion view, I do introduce the local here because there are multiple none-exception-related uses.

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

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