RFR JDK-7092821 "java.security.Provider.getService() is synchronized and became scalability bottleneck"

[Valerie Peng]

If anyone is still reviewing this and need more time, please let me know 
soon. Otherwise, I will proceed with integration this afternoon.

Thanks,
Valerie


On 12/6/2018 7:27 PM, Valerie Peng wrote:
> Hi Sean,
>
> Thanks for your review, I have removed the stale comment from the 
> VerificationProvider.
>
> As for your concern on race condition, legacyChanged and the 
> String/String legacy mapping are still synchronized on the provider 
> object. Only the service mapping is managed by the ConcurrentHashMap. 
> I have also changed serviceChanged to be volatile. For those methods 
> which no longer have synchronized keywords, they still have 
> synchronized blocks around the legacy mapping related code. In 
> general, the provider mappings are populated during construction time 
> and rarely updated afterwards. In addition, most if not all, they 
> either use all legacy or all service. The mixing of them makes the 
> code un-necessarily complicated, oh-well.
>
> Latest webrev: http://cr.openjdk.java.net/~valeriep/7092821/webrev.01
>
> Valerie
>
> On 12/4/2018 2:29 PM, Sean Mullan wrote:
>> I haven't reviewed all of this, but had a couple of comments so far:
>>
>> - VerificationProvider.java
>>
>> 77         // the provider. Otherwise, create a temporary map and use a
>>
>> This comment is now stale so it needs to be removed/updated.
>>
>> - Provider.java
>>
>> You removed synchronized from several of the methods, but some of 
>> those methods modified fields such as servicesChanged and 
>> legacyChanged. Are there any concurrency issues or race conditions 
>> now that those methods are not synchronized?
>>
>> --Sean
>>
>>
>> On 11/21/18 1:05 PM, Valerie Peng wrote:
>>> Hi,
>>>
>>> Can someone help reviewing this fix?
>>>
>>> Besides changing the Provider class to use ConcurrentHashMap in 
>>> order to reduce the lock contention on Provider.getService() calls, 
>>> I also changed the security providers in java.base module to 
>>> register through putService(...) calls. Performance is manually 
>>> verified and mach5 run is clean.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-7092821
>>> Webrev: http://cr.openjdk.java.net/~valeriep/7092821/webrev.00/
>>>
>>> Thanks,
>>> Valerie
>>>
>>>
>>>
>>>
>>>