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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Jan 8 18:20:36 UTC 2019



On 1/8/19 11:55 AM, Aleksey Shipilev wrote:
> On 1/8/19 5:40 PM, coleen.phillimore at oracle.com wrote:
>> http://cr.openjdk.java.net/~shade/8216302/webrev.02/src/hotspot/share/classfile/javaClasses.cpp.udiff.html
>>
>> + oop class_oop = holder->java_mirror();
>>
>> This is a naked oop.  You should use Handle and HandleMark in this function.
> Mmm. But it is the same oop we are already handling in the old code... What's the rule here?

The StringTable::intern() could take out a lock.  It might not anymore 
but putting things like this relieves the need to look into the function 
to see whether it does or not.
>
>> 1347 oop java_lang_Class::name(oop java_class, TRAPS) {
>>
>> So is this.  You should pass a Handle to this.
> I don't understand the "should" part. Why should it be handle-ized? javaClasses are expected to work
> properly with naked oops, because we are not expected to get to safepoint in the middle of it, no?

We could though if StringTable::intern takes out a lock or stops for a 
safepoint (while resizing with the new concurrent hashtable).
> And we are storing oops to the heap itself, not in any VM structure.
>
> In the same StackTraceElement::fill_in, we do e.g. this without any handles:
>
>    oop loader = holder->class_loader();
>    if (loader != NULL) {
>      oop loader_name = java_lang_ClassLoader::name(loader);
>      if (loader_name != NULL)
>        java_lang_StackTraceElement::set_classLoaderName(element(), loader_name);
>    }

Having the oops on the stack and getting a safepoint is the definition 
of "unhandled oops"..    If the oop isn't used below any calls involving 
it, it is safe.  If there are intervening calls (especially ones with 
TRAPS), they should be put in Handle so that they are known safe without 
having to know the implementation details of all the functions that are 
called.  There is little overhead if there is a HandleMark near the Handles.

Thanks,
Coleen
> -Aleksey
>



More information about the hotspot-dev mailing list