On Wed, 24 Nov 2021 21:17:34 GMT, Valerie Peng <[email protected]> wrote:
>> It is observed that when running crypto benchmark with large number of
>> threads, a lot of time is spent on the synchronized block inside the
>> Provider.getService() method. The cause for this is that
>> Provider.getService() method first uses the 'serviceMap' field to find the
>> requested service. However, when the requested service is not supported by
>> this provider, e.g. requesting Cipher.RSA from SUN provider, the impl
>> continues to try searching the legacy registrations whose processing is
>> guarded by the "synchronized" keyword. When apps use getInstance() calls
>> without the provider argument, Provider class has to iterate through
>> existing providers trying to find one that supports the requested service.
>>
>> Now that the parent class of Provider no longer synchronizes all of its
>> methods, Provider class should follow suit and de-synchronize its methods.
>> Parsing of the legacy registration is done eagerly (at the time of put(...)
>> calls) instead of lazily (at the time of getService(...) calls). This also
>> makes "legacyStrings" redundant as the registration is parsed and stored
>> directly into "legacyMap".
>>
>> The bug reporter has confirmed that the changes resolve the performance
>> bottleneck and all regression tests pass.
>>
>> Please review and thanks in advance,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updated to use pattern matching with instanceof operator.
src/java.base/share/classes/java/security/Provider.java line 1292:
> 1290: }
> 1291:
> 1292: if (!legacyMap.isEmpty()) {
Looks like this check is redundant. get() will return `null` anyway.
src/java.base/share/classes/java/security/Provider.java line 1295:
> 1293: Service s = legacyMap.get(key);
> 1294: if (s != null && !s.isValid()) {
> 1295: legacyMap.remove(key);
I wonder if it's better to use `legacyMap.remove(key, value);` here. What if
another thread put some other value by the same key?
-------------
PR: https://git.openjdk.java.net/jdk/pull/6513
