RFR 8206424: Use locking for cleaning ProtectionDomainTable

[Patricio Chilano]

Hi David,

Yes, as Coleen mentioned both locks (ResolvedMethodTable_lock, 
SystemDictionary_lock) required to do the cleanup will check for safepoints.
The idea was originally to use a concurrent hash table like the one 
implemented for the string and symbol tables. The benchmarks with this 
simple locking mechanism show non-significant changes, which might have 
to do with the size and usage of these tables, specially for the 
protection domain table. Eventually we could move to a more complex 
synchronization scheme to try to achieve speedups.

Thanks,
Patricio

8/22/18 8:54 AM, coleen.phillimore at oracle.com wrote:
>
> On 8/21/18 10:23 PM, David Holmes wrote:
>> Hi Patricio,
>>
>> Actual changes seem fine - thanks.
>>
>> I have a meta question about what we are doing and why. IIUC we're 
>> trying to offload work from the VMThread at a safepoint, to the 
>> ServiceThread not-at-a-safepoint - is that correct? Have we looked at 
>> the amount of work this can generate in the service thread and the 
>> potentially latency introduced for the cleanup actions? Also if the 
>> service thread gets too much work to do it will delay the next 
>> safepoint (as I'm not expecting these cleanup tasks to themselves 
>> encounter safepoint checks).
>
> This is correct.  These table are small and the locks that clean up 
> these tables check for safepoints like good citizens.
>
> Thanks,
> Coleen
>>
>> And having just asked a question I have to apologise that I'll be on 
>> vacation by the time you can respond to it. :) But the discussion can 
>> keep till later.
>>
>> Thanks,
>> David
>>
>> On 22/08/2018 7:31 AM, Patricio Chilano wrote:
>>> Hi Ioi and Coleen,
>>>
>>> Thanks for reviewing this change! I moved the test to 
>>> test/hotspot/jtreg/runtime/Dictionary and fixed the copyright. Here 
>>> is the new webrev:
>>>
>>> Webrev URL: http://cr.openjdk.java.net/~pchilanomate/8206424.02/webrev
>>>
>>> Thanks,
>>> Patricio
>>>
>>> On 8/21/18 5:11 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>> Patricio,
>>>>
>>>> This looks really good.  I agree with Ioi's comment.  Maybe the 
>>>> test could go into test/hotspot/jtreg/runtime/Dictionary (there are 
>>>> similar tests there?)
>>>>
>>>> Also the copyright should just have 2018 followed by a comma (even 
>>>> though grammatically incorrect).
>>>>
>>>> thanks,
>>>> Coleen
>>> On 8/21/18 3:27 PM, Ioi Lam wrote:
>>>> Hi Patricio,
>>>>
>>>> I think the test should be under a different directory since it is 
>>>> not specific to AppCDS
>>>>
>>>> Thanks
>>>> Ioi
>>>>
>>>>> On Aug 21, 2018, at 11:47 AM, Patricio Chilano 
>>>>> <patricio.chilano.mateo at oracle.com> wrote:
>>>>>
>>>>> Hi all,
>>>>> Could you review this fix that moves the cleanup of dead entries 
>>>>> in the protection domain table from the VMThread to the 
>>>>> ServiceThread.
>>>>> The goal is the same as JDK-8206423. The task of cleaning up dead 
>>>>> entries in the protection domain table was moved to the 
>>>>> ServiceThread.
>>>>> The fix was tested with Mach5 on tiers 1, 2, 3 on all platforms. A 
>>>>> new test 
>>>>> (test/hotspot/jtreg/runtime/appcds/CleanProtectionDomain.java) was 
>>>>> added to test the creation and removal of entries in the 
>>>>> Protection Domain table.
>>>>> Bug URL: https://bugs.openjdk.java.net/browse/JDK-820642 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8206423>4
>>>>> Webrev URL: 
>>>>> http://cr.openjdk.java.net/~pchilanomate/8206424.01/webrev/index.html
>>>>> Thanks,
>>>>> Patricio
>>>>>
>>>
>