RFR (S) 8216302: StackTraceElement::fill_in can use cached Class.name

[coleen.phillimore at oracle.com]

+1 for the XX version.
Coleen

On 1/8/19 8:12 PM, Mandy Chung wrote:
>
>
> On 1/8/19 4:59 PM, David Holmes wrote:
>> On 9/01/2019 10:08 am, Aleksey Shipilev wrote:
>>> On 1/9/19 12:24 AM, Mandy Chung wrote:
>>>>> I really don't like the fact the VM is now setting the name field 
>>>>> and there's nothing in the Java
>>>>> code to give any indication that this is happening. At a minimum a 
>>>>> comment should be added, as is
>>>>> done with other class members that get accessed directly by the VM.
>>>>>
>>>>> I also think core-libs folk should be having a say here.
>>>>
>>>> Catching up on this thread...
>>>>
>>>> Two ways setting the Class::name field isn't pleasant.  What about:
>>>>
>>>> public String getName() {
>>>>     String name = this.name;
>>>>     return name != null ? name : initClassName();
>>>> }
>>>>
>>>> where JVM_InitClassName will call java_lang_Class::name().
>>>
>>> Mmm. Should we really change the jvm.h here? Does that involve CSR? 
>>
>> jvm.h is a private interface between the OpenJDK core libraries and 
>> the OpenJDK VM (Hotspot), and does not require a CSR request when 
>> changed.
>
> Yup.  CSR is not required for changes in this private interface.
>>
>>> It would have ripple effects on
>>> Graal, potential backports (You'd want this in 11, right? I would. 
>>> This is a visible perf regression
>>> since 8), etc. Note that after handelizing java_lang_Class::name, we 
>>> cannot simply call it from
>>> JVM_GetClassName.
>>>
>>> Do we really think this is worth the hassle like this:
>>> http://cr.openjdk.java.net/~shade/8216302/webrev.XX/
>>
>> That version works for me.
>
> Thanks for making this change.  I prefer this version which makes the 
> code very clear what it does.
>
> Thanks
> Mandy