RFR 8206423: Use ConcurrentHashTable for ResolvedMethodTable

Patricio Chilano patricio.chilano.mateo at oracle.com
Mon Aug 20 15:28:57 UTC 2018


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