On Wed, 24 Nov 2021 21:17:34 GMT, Valerie Peng <valer...@openjdk.org> 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. Consider this case, two threads are changing a value at the same time. Since the method is not synchonized, thread1 might finish the first part of the method (`super.replace`) earlier than thread2, but it finishes the second part (`parseLegacy`) later than thread2. At the end, the internal entrySet has thread2's value but the legacy map has thread1's value. ------------- PR: https://git.openjdk.java.net/jdk/pull/6513