RFR 8206423: Use ConcurrentHashTable for ResolvedMethodTable

[David Holmes]

Hi Patricio,

This change seems to have induced a test failure:

runtime/MemberName/MemberNameLeak.java

---------System.err:(42/3986)----------
Java HotSpot(TM) 64-Bit Server VM warning: Option UseConcMarkSweepGC was 
deprecated in version 9.0 and will likely be removed in a future release.
  stdout: [[0.697s][debug][membername,table] ResolvedMethod entry added 
for MemberNameLeak$Leak.callMe()V index 615
[0.702s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.MethodHandle.linkToSpecial(Ljava/lang/Object;Ljava/lang/invoke/MemberName;)V 
index 186
[0.705s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.DirectMethodHandle.internalMemberName(Ljava/lang/Object;)Ljava/lang/Object; 
index 879
[0.719s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.LambdaForm$DMH/0x0000000800060840.invokeSpecial(Ljava/lang/Object;Ljava/lang/Object;)V 
index 897
[0.721s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.Invokers.checkExactType(Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)V 
index 811
[0.722s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.Invokers.checkCustomized(Ljava/lang/invoke/MethodHandle;)V 
index 257
[0.722s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.MethodHandle.invokeBasic(Ljava/lang/Object;)V index 4
[0.722s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.MethodHandle.linkToSpecial(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/invoke/MemberName;)V 
index 648
[0.722s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.DirectMethodHandle$Holder.invokeSpecial(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V 
index 825
[0.723s][debug][membername,table] ResolvedMethod entry added for 
java.lang.invoke.LambdaForm$MH/0x0000000800060c40.invokeExact_MT(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)V 
index 115
[0.726s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.726s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.726s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.726s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.726s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.727s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.727s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.727s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
[0.727s][debug][membername,table] ResolvedMethod entry found for 
MemberNameLeak$Leak.callMe()V index 615
];
  stderr: [Java HotSpot(TM) 64-Bit Server VM warning: Option 
UseConcMarkSweepGC was deprecated in version 9.0 and will likely be 
removed in a future release.
]
  exitValue = 0

java.lang.RuntimeException: 'ResolvedMethod entry removed' missing from 
stdout/stderr

	at 
jdk.test.lib.process.OutputAnalyzer.shouldContain(OutputAnalyzer.java:150)
	at MemberNameLeak.test(MemberNameLeak.java:68)
	at MemberNameLeak.main(MemberNameLeak.java:77)

No bug filed yet.

David

On 21/08/2018 10:23 AM, David Holmes wrote:
> On 21/08/2018 1: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
> 
> Yes that's fine - thanks.
> 
> David
> 
>> 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
>>>>>>
>>>>>>
>>>>
>>