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

[David Holmes]

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.

> 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.

> I'd rather prefer to document this:
> 
> diff -r 7c99f0c51412 src/java.base/share/classes/java/lang/Class.java
> --- a/src/java.base/share/classes/java/lang/Class.java	Tue Jan 08 20:27:23 2019 +0100
> +++ b/src/java.base/share/classes/java/lang/Class.java	Wed Jan 09 00:20:24 2019 +0100
> @@ -801,5 +801,6 @@
>       }
> 
> -    // cache the name to reduce the number of calls into the VM
> +    // Cache the name to reduce the number of calls into the VM.
> +    // This field can be set by VM itself without the call to getName0.
>       private transient String name;
>       private native String getName0();
> 
> ...accept that cache can be set on different paths, and move on.

That would meet my minimum level for acceptance.

Cheers,
David

> -Aleksey
>