RFR 8206424: Use locking for cleaning ProtectionDomainTable

[David Holmes]

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).

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
>>>
>