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

[Valerie Peng]

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