RFR (S) 8216302: StackTraceElement::fill_in can use cached Class.name
Mandy Chung
mandy.chung at oracle.com
Tue Jan 8 23:24:01 UTC 2019
On 1/8/19 2:40 PM, David Holmes wrote:
> On 9/01/2019 5:54 am, coleen.phillimore at oracle.com wrote:
>> On 1/8/19 1:55 PM, Aleksey Shipilev wrote:
>>> On 1/8/19 5:48 PM, coleen.phillimore at oracle.com wrote:
>>>> I agree we shouldn't initialize the name field eagerly when
>>>> creating a class. I also looked at this
>>>> code path:
>>>>
>>>> public String getName() {
>>>> String name = this.name;
>>>> if (name == null)
>>>> this.name = name = getName0();
>>>> return name;
>>>> }
>>>>
>>>> It looks like when we call JVM_GetClassName, we're initializing the
>>>> Class.name field by the caller.
>>>>
>>>> Maybe could rewrite the java/lang/Class version to be:
>>>>
>>>> public String getName() {
>>>> String name = this.name;
>>>> if (name == null)
>>>> name = getName0(); // this initializes this.name yuck.
>>>> return name;
>>>> }
>>>>
>>>> and have JVM_GetClassName call java_lang_Class::name() to do the
>>>> initialization. Seems not worth
>>>> it just to avoid duplicating these lines in both
>>>> java_lang_Class::name() and JVM_GetClassName.
>>> Right. I also think it does not worth it. My reason is given in
>>> another reply in this thread:
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2019-January/036152.html
>>>
>>
>> Right, saw that. I agreed.
>
> 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().
Mandy
More information about the hotspot-dev
mailing list