RFR 8206423: Use ConcurrentHashTable for ResolvedMethodTable

Patricio Chilano patricio.chilano.mateo at oracle.com
Mon Aug 20 19:06:11 UTC 2018


Thanks Coleen!


Patricio

On 8/20/18 2:59 PM, coleen.phillimore at oracle.com wrote:
>
> This comment looks good to me!
> Coleen
>
> On 8/20/18 11:28 AM, Patricio Chilano wrote:
>> Hi David,
>>
>> Thanks for the review! Do you think this new comment works?
>>
>> -// This is done late during GC.
>> +// This is done by the ServiceThread after being notified on class 
>> unloading
>>
>> Webrev URL: http://cr.openjdk.java.net/~pchilanomate/8206423.03/webrev/
>>
>> Thanks,
>> Patricio
>>
>> On 8/17/18 6:30 PM, David Holmes wrote:
>>> Hi Patricio,
>>>
>>> On 18/08/2018 4:42 AM, Patricio Chilano wrote:
>>>> Hi Gerard,
>>>>
>>>> Thanks for the review! Here is the new webrev with the suggested 
>>>> changes:
>>>>
>>>> Webrev URL: 
>>>> http://cr.openjdk.java.net/~coleenp/8206423.02/webrev/index.html
>>>
>>> This seems fine to me. One minor nit - this comment:
>>>
>>>  159 // This is done late during GC.
>>>  160 void ResolvedMethodTable::unlink() {
>>>
>>> is no longer accurate in resolvedMethodTable.cpp
>>>
>>> Thanks,
>>> David
>>>
>>>> Thanks,
>>>> Patricio
>>>>
>>>> On 8/17/18 12:40 PM, Gerard Ziemski wrote:
>>>>> hi Patricio,
>>>>>
>>>>> Looks good. The only feedback I have is that, I’d prefer to see 
>>>>> the names in src/hotspot/share/runtime/serviceThread.cpp use 
>>>>> similar naming convention, so maybe something like:
>>>>>
>>>>>       bool string_table_work = false;
>>>>>       bool symbol_table_work = false;
>>>>>       bool resolved_method_table_work = false;
>>>>> ...
>>>>>                 !(string_table_work = StringTable::has_work()) &&
>>>>>                 !(symbol_table_work = SymbolTable::has_work()) &&
>>>>>                 !(resolved_method_table_work = 
>>>>> ResolvedMethodTable::has_work())) {
>>>>>
>>>>>
>>>>> cheers
>>>>>
>>>>>
>>>>>> On Aug 17, 2018, at 11:08 AM, Patricio 
>>>>>> Chilano<patricio.chilano.mateo at oracle.com> wrote:
>>>>>>
>>>>>> Hi all,
>>>>>> Could you review this fix that moves the cleanup up of dead 
>>>>>> entries in the resolved method table from the VMThread to the 
>>>>>> ServiceThread.
>>>>>> The goal was to remove the need to clean up dead entries in the 
>>>>>> resolved method table during safepoints, thus reducing pause 
>>>>>> times. That task was moved to the ServiceThread, which will be 
>>>>>> notified by the VMThread when detecting the need to cleanup after 
>>>>>> class unloading. Benchmarks where run(javac, sanity, SPECjbb) 
>>>>>> showing non-significant results.
>>>>>> The fix was tested with Mach5 on tiers 1, 2, 3 on all platforms. 
>>>>>> (Note: test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java 
>>>>>> already exists and tests the creation and deletion of entries in 
>>>>>> the resolved method table).
>>>>>> Bug URL:https://bugs.openjdk.java.net/browse/JDK-8206423
>>>>>> Webrev 
>>>>>> URL:http://cr.openjdk.java.net/~coleenp/8206423.01/webrev/index.html 
>>>>>> <http://cr.openjdk.java.net/%7Ecoleenp/8206470.01/webrev/index.html>
>>>>>> Thanks,
>>>>>> Patricio
>>>>>>
>>>>>>
>>>>
>>
>



More information about the hotspot-runtime-dev mailing list