RFR 8206423: Use ConcurrentHashTable for ResolvedMethodTable

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Mon Aug 20 18:59:16 UTC 2018


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