On Thu, 22 Jan 2026 23:51:42 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." > > Koushik Muthukrishnan Thirupattur has updated the pull request incrementally > with one additional commit since the last revision: > > 8345954: Addressing review comments Looks fine to me, just one minor question src/java.base/share/classes/sun/security/provider/X509Factory.java line 188: > 186: enc = newC.getEncodedInternal(); > 187: } > 188: return addIfNotPresent(certCache, enc, newC); Nit: what do you think about moving this logic inside of the `addIfNotPresent` and just pass an option if it is an impl? X509CertImpl newC; byte[] enc; if (isImpl) { newC = (X509CertImpl) c; enc = encoding; } else { newC = new X509CertImpl(encoding); enc = newC.getEncodedInternal(); } return addIfNotPresent(certCache, enc, newC); I think this might be a bit easier to follow then. But I don't really mind if you leave it this way either ------------- PR Review: https://git.openjdk.org/jdk/pull/29181#pullrequestreview-3705573013 PR Review Comment: https://git.openjdk.org/jdk/pull/29181#discussion_r2727198948
