On Tue, 13 Jan 2026 04:14:22 GMT, Koushik Muthukrishnan Thirupattur <[email protected]> wrote:
> Refactor sun.security.provider.X509Factory cache access to avoid > coarse-grained locking and reduce contention during certificate/CRL interning > and parsing. > > As per request in [the > PR](https://github.com/openjdk/jdk/pull/22616#issuecomment-2524971845), > re-visit "the initialisation and locking in this area, e.g. addToCache is a > static synchronized method so very coarse locking." Some comments. src/java.base/share/classes/sun/security/provider/X509Factory.java line 94: > 92: synchronized (certCache) { > 93: certCache.clear(); > 94: } This is just for debugging. Maybe not worth any protection. src/java.base/share/classes/sun/security/provider/X509Factory.java line 183: > 181: X509CertImpl newC = isImpl ? (X509CertImpl) c : new > X509CertImpl(encoding); > 182: byte[] enc = isImpl ? encoding : newC.getEncodedInternal(); > 183: return addToCache(certCache, enc, newC); Is this better than the old code? `isImpl` was only checked once. src/java.base/share/classes/sun/security/provider/X509Factory.java line 230: > 228: * Add the X509CertImpl or X509CRLImpl to the cache. > 229: */ > 230: private static <V> V addToCache(Cache<Object, V> cache, byte[] > encoding, V value) { Now that you added double check inside this method to ensure no cache, does it make sense to rename the method to be something like `addIfNotPresent`? src/java.base/share/classes/sun/security/provider/X509Factory.java line 245: > 243: } > 244: > 245: Newline? ------------- PR Review: https://git.openjdk.org/jdk/pull/29181#pullrequestreview-3692030795 PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716545800 PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716567416 PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716621763 PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2716550443
