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

[Aleksey Shipilev]

On 1/8/19 12:27 PM, David Holmes wrote:
> On 8/01/2019 7:56 pm, Aleksey Shipilev wrote:
>> On 1/8/19 3:15 AM, David Holmes wrote:
>>> It seems somewhat awkward to me to have two different code paths for initializing the
>>> java.lang.Class name field. Can this be restructured a little more (change Class.getName()) so that
>>> the VM always initializes "name" and then JVM_GetClassName could just call java_lang_Class::name,
>>> rather than duplicating the logic?
>>
>> Mmm. I am afraid to do this eagerly because of more memory footprint and potential bootstrapping
>> issues. 
> 
> I said nothing about doing this eagerly. All I'm suggesting is that instead of getName() doing:
> 
> if (this.name == null)
>   getName0(); // calls JVM_GetClassName
> return this.name;
> 
> it just does:
> 
> if (this.name == null)
>   getName0(); // calls JVM_GetClassName
> return this.name;

Oh. I had this option on the table when doing the patch, but I disregarded it as dirty hack, because
calling the getter for side effects *only* is awkward. Handling concurrency on Java side also looks
simpler. There is a benign race on Class.name, and to remain benign, it should _not_ do the second
read. In other word, null-checking this.name, and then returning the second read of this.name is not
entirely correct.

The original code is checking and returning the local, not the second heap read:

    public String getName() {
        String name = this.name;
        if (name == null)
            this.name = name = getName0();
        return name;
    }

This, I think, leaves us with taking the return value from getName0.

If we wanted to handle JVM_GetClassName better, we could consider calling java_lang_Class::name from
JVM_GetClassName, thus going through the cached path. That would do the two stores, though: one
through javaClasses, and another on Java side. Seeing how there is only a single use of
JVM_GetClassName, and that use is already-cached Class.getName0, I see no reason to do the excessive
write.

-Aleksey