RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v2]

[David Holmes]

On 8/09/2020 4:34 pm, Kim Barrett wrote:
>> On Sep 8, 2020, at 12:09 AM, David Holmes <dholmes at openjdk.java.net> wrote:
>> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Used static_cast instead of raw C-style cast
>>   Moved inline functions to thread.inline.hpp
>>   Updated #includes to deal with the move to thread.inline.hpp
>>   Updated copyright year where needed.
>>
>> -------------
>>
>> Changes:
>>   - all: https://git.openjdk.java.net/jdk/pull/37/files
>>   - new: https://git.openjdk.java.net/jdk/pull/37/files/b18faad5..da70f804
>>
>> Webrevs:
>> - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=37&range=01
>> - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=37&range=00-01
>>
>>   Stats: 102 lines in 33 files changed: 39 ins; 28 del; 35 mod
>>   Patch: https://git.openjdk.java.net/jdk/pull/37.diff
>>   Fetch: git fetch https://git.openjdk.java.net/jdk pull/37/head:pull/37
>>
>> PR: https://git.openjdk.java.net/jdk/pull/37
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/runtime/thread.hpp
>   507   const JavaThread* as_const_Java_thread() const;
> 
> This missed part of what I'd suggested. I don't think the "const" part
> of the name is needed or helpful.  Just overload as_Java_thread on the
> const-ness of the thread it's being applied to.

I saw that but thought it was a typo as I don't think about overloads in 
terms of const or other modifiers. To me overloads are same named 
methods that differ in arguments (and I'm going to guess that 'this' is 
considered an argument in this context and hence leads to the overload 
based on the const?).

But in addition I wanted to have this say as_const_JavaThread because I 
wanted to clearly distringuish it from as_JavaThread because there is 
only a couple of special cases in the JFR code where this const version 
is needed. If that code were to change and is_const_JavaThread because 
unused, it would be easy to see that.

> There are some situations where there is such an overload pair and it
> is useful to additionally have an explicit const function, but I don't
> think this is one of those.
> 
> ------------------------------------------------------------------------------
> 
> The fannout from putting the conversion functions in thread.inline.hpp seems
> pretty high in the number of files that get touched. Especially since it
> leads to JavaThread::current() and JavaThread::as_CompilerThread() being
> moved. I would have been at least fine with, and probably would prefer,
> having the definitions of as_JavaThread near the old definitions of those
> functions in thread.hpp.

I tried to do the right and obey the "use .inline.hpp for inline 
definitions" rule. But I must confess I was reaching a point where I 
thought this was going too far but it's a good exercise with using git 
and PRs so I don't mind if I back up and take a simpler approach - 
especially as I just discovered more build failures due to missing 
includes. :(

Thanks,
David

> ------------------------------------------------------------------------------
>