RFR (S): 8199567: [Nestmates] Cleanup instanceKlass.cpp

David Holmes david.holmes at oracle.com
Tue Oct 23 20:59:56 UTC 2018


Thanks for the review Coleen!

David

On 24/10/2018 4:09 AM, coleen.phillimore at oracle.com wrote:
> 
> This looks fine.  Thank you both for suggesting and making the 
> is_klass() change.
> Coleen
> 
> On 10/23/18 10:20 AM, Lois Foltan wrote:
>> On 10/23/2018 8:36 AM, David Holmes wrote:
>>
>>> Hi Lois,
>>>
>>> On 23/10/2018 10:14 PM, Lois Foltan wrote:
>>>> On 10/22/2018 11:43 PM, David Holmes wrote:
>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199567
>>>>> Webrev: http://cr.openjdk.java.net/~dholmes/8199567/webrev/
>>>>>
>>>>> This is a simple "cleanup" previously suggested by John Rose for 
>>>>> checking nest membership. Rather than checking for name equality 
>>>>> and then checking the loaded class, we check if there's a loaded 
>>>>> class first and if not fallback to the name check. This favours the 
>>>>> case where the nestmate classes are already resolved and loaded.
>>>>
>>>> Hi David,
>>>>
>>>> Looks good.  One comment:
>>>
>>> Thanks for looking at it!
>>>
>>>> oops/instanceKlass.cpp:
>>>> - line #173: Consider changing the if conditional to "if 
>>>> (_constants->tag_at(cp_index).is_klass())"
>>>>
>>>> I believe the "is_klass()" method is the preferred way to compare a 
>>>> tag to JVM_CONSTANT_Class.
>>>
>>> Sure - webrev updated:
>>>
>>> http://cr.openjdk.java.net/~dholmes/8199567/webrev.v2/
>>
>> Looks good, thank you for making that change!
>> Lois
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> Thanks,
>>>> Lois
>>>>
>>>>>
>>>>> There was no observable performance difference.
>>>>>
>>>>> Testing: mach5 tiers 1-3 including nestmate tests
>>>>>
>>>>> Thanks,
>>>>> David
>>>>
>>
> 


More information about the hotspot-runtime-dev mailing list