Code Review Request JDK-8129988 JSSE should create a single instance of the cacerts KeyStore

[Sean Mullan]

Hi Xuelei,

First cut of a review at this. I still need to review TrustStoreManager, 
so will get back to you later on that. Looks very good so far.

* KeyStores.java

Do an 'hg rename' on this file, so you can see the diffs between this 
and the new name (TrustStoreUtil), and you preserve the history.

* TrustStoreUtil.java

   54         Set<X509Certificate> set = new HashSet<X509Certificate>();

Use <> operator.

   71         } catch (KeyStoreException e) {
   72             // ignore
   73         }

This should be rare, but maybe we want to log this.

* TrustStoreManager.java

   65     public static KeyStore getgetTrustedKeyStore() throws Exception {

s/getget/get/

* TrustManagerFactoryImpl.java

   85     abstract X509TrustManager getInstance(
   86             Collection<X509Certificate> trustedCerts) throws 
KeyStoreException;

It no longer makes sense for this method to throw KeyStoreException.

* X509TrustManagerImpl.java

   71     X509TrustManagerImpl(String validatorType,
   72             Collection<X509Certificate> trustedCerts) throws 
KeyStoreException {

No longer can throw KeyStoreException so it can be removed from throws 
clause.

  83         if (debug != null && Debug.isOn("trustmanager")) {
  84             showTrustedCerts();

Not sure why you made this change (and on line 99) because 
showTrustedCerts is still only called if debug is enabled.

--Sean

On 11/26/16 7:46 PM, Xuelei Fan wrote:
> Hi,
>
> Please review the performance enhancement update:
>
>    http://cr.openjdk.java.net/~xuelei/8129988/webrev.00/
>
> In SunJSSE provider, there are two ways to use the default trust store
> (lib/security/cacerts), using the default SSLContext instance or using
> the default trust manager.
>
> The default SSLContext holds a strong reference to a collection of
> trusted certificates in cacerts in static mode.  The default trust
> manager reads the cacerts file and creates a KeyStore and parses the
> certificates each time.
>
> With the growth of cacerts, the loading and cache of trusted certificate
> is not performance friendly.
>
> In this fix, I'm trying to find a balance between CPU and memory: reuse
> the loaded trusted certificates if possible and release the unused
> trusted certificates if necessary.
>
> Thanks,
> Xuelei
>