RFR: Remove map synchronization from SignatureAndHashAlgorithm

Sean Mullan sean.mullan at oracle.com
Wed Apr 26 20:31:19 UTC 2017


On 4/26/17 1:52 PM, Steven Davidovitz wrote:
> Thanks! I'm working on the OCA right now, I made a mistake when I
> submitted it before so I'll follow up when it's done.
>
> Anything else I need to do? Could this also be backported to version 8?

We are already at RDP2 [1] for JDK 9. Only P1 and P2 bugs that are 
critical issues should be fixed now and in my opinion, this is not a 
critical issue for JDK 9. So we will need to fix it in JDK 10 first. 
Backports to a 9u/8u release can be considered later.

--Sean

[1] http://openjdk.java.net/projects/jdk9/rdp-2

>
> On Tue, Apr 25, 2017 at 7:05 AM, Sean Mullan <sean.mullan at oracle.com
> <mailto:sean.mullan at oracle.com>> wrote:
>
>     Hi Steven,
>
>     Thanks, I filed an issue on your behalf for tracking purposes:
>     https://bugs.openjdk.java.net/browse/JDK-8179275
>     <https://bugs.openjdk.java.net/browse/JDK-8179275>
>
>     Also, have you signed the OCA? See
>     http://www.oracle.com/technetwork/community/oca-486395.html
>     <http://www.oracle.com/technetwork/community/oca-486395.html> for
>     more information.
>
>     --Sean
>
>
>     On 4/15/17 3:25 PM, Steven Davidovitz wrote:
>
>         With the removal of the synchronization on priorityMap inside
>         'SignatureAndHashAlgorithm.getSupportedAlgorithms' in rev
>         daaace32c979
>         [1], it seems unnecessary to use a synchronizedSortedMap.
>         Benchmarking, I see a 2x performance increase by using the bare
>         TreeMap.
>
>         measureModified   sample  11336330 11949.506 ± 1775.776  ns/op
>         measureOriginal    sample  10855026 23003.654 ± 2286.571  ns/op
>
>         Thanks,
>         Steven Davidovitz
>
>         1: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/daaace32c979
>         <http://hg.openjdk.java.net/jdk9/dev/jdk/rev/daaace32c979>
>
>         diff --git
>         a/src/java.base/share/classes/sun/security/ssl/SignatureAndHashAlgorithm.java
>         b/src/java.base/share/classes/sun/security/ssl/SignatureAndHashAlgorithm.java
>         ---
>         a/src/java.base/share/classes/sun/security/ssl/SignatureAndHashAlgorithm.java
>         +++
>         b/src/java.base/share/classes/sun/security/ssl/SignatureAndHashAlgorithm.java
>         @@ -396,46 +396,42 @@
>              }
>
>              static {
>         -        supportedMap = Collections.synchronizedSortedMap(
>         -            new TreeMap<Integer, SignatureAndHashAlgorithm>());
>         -        priorityMap = Collections.synchronizedSortedMap(
>         -            new TreeMap<Integer, SignatureAndHashAlgorithm>());
>         -
>         -        synchronized (supportedMap) {
>         -            int p = SUPPORTED_ALG_PRIORITY_MAX_NUM;
>         -            supports(HashAlgorithm.MD5,
>          SignatureAlgorithm.RSA,
>         -                    "MD5withRSA",           --p);
>         -            supports(HashAlgorithm.SHA1,
>         SignatureAlgorithm.DSA,
>         -                    "SHA1withDSA",          --p);
>         -            supports(HashAlgorithm.SHA1,
>         SignatureAlgorithm.RSA,
>         -                    "SHA1withRSA",          --p);
>         -            supports(HashAlgorithm.SHA1,
>         SignatureAlgorithm.ECDSA,
>         -                    "SHA1withECDSA",        --p);
>         +        supportedMap = new TreeMap<Integer,
>         SignatureAndHashAlgorithm>();
>         +        priorityMap = new TreeMap<Integer,
>         SignatureAndHashAlgorithm>();
>
>         -            if (Security.getProvider("SunMSCAPI") == null) {
>         -                supports(HashAlgorithm.SHA224,
>         SignatureAlgorithm.DSA,
>         -                        "SHA224withDSA",        --p);
>         -                supports(HashAlgorithm.SHA224,
>         SignatureAlgorithm.RSA,
>         -                        "SHA224withRSA",        --p);
>         -                supports(HashAlgorithm.SHA224,
>         SignatureAlgorithm.ECDSA,
>         -                        "SHA224withECDSA",      --p);
>         -            }
>         +        int p = SUPPORTED_ALG_PRIORITY_MAX_NUM;
>         +        supports(HashAlgorithm.MD5,         SignatureAlgorithm.RSA,
>         +                "MD5withRSA",           --p);
>         +        supports(HashAlgorithm.SHA1,        SignatureAlgorithm.DSA,
>         +                "SHA1withDSA",          --p);
>         +        supports(HashAlgorithm.SHA1,        SignatureAlgorithm.RSA,
>         +                "SHA1withRSA",          --p);
>         +        supports(HashAlgorithm.SHA1,
>         SignatureAlgorithm.ECDSA,
>         +                "SHA1withECDSA",        --p);
>
>         -            supports(HashAlgorithm.SHA256,
>         SignatureAlgorithm.DSA,
>         -                    "SHA256withDSA",        --p);
>         -            supports(HashAlgorithm.SHA256,
>         SignatureAlgorithm.RSA,
>         -                    "SHA256withRSA",        --p);
>         -            supports(HashAlgorithm.SHA256,
>         SignatureAlgorithm.ECDSA,
>         -                    "SHA256withECDSA",      --p);
>         -            supports(HashAlgorithm.SHA384,
>         SignatureAlgorithm.RSA,
>         -                    "SHA384withRSA",        --p);
>         -            supports(HashAlgorithm.SHA384,
>         SignatureAlgorithm.ECDSA,
>         -                    "SHA384withECDSA",      --p);
>         -            supports(HashAlgorithm.SHA512,
>         SignatureAlgorithm.RSA,
>         -                    "SHA512withRSA",        --p);
>         -            supports(HashAlgorithm.SHA512,
>         SignatureAlgorithm.ECDSA,
>         -                    "SHA512withECDSA",      --p);
>         +        if (Security.getProvider("SunMSCAPI") == null) {
>         +            supports(HashAlgorithm.SHA224,
>         SignatureAlgorithm.DSA,
>         +                    "SHA224withDSA",        --p);
>         +            supports(HashAlgorithm.SHA224,
>         SignatureAlgorithm.RSA,
>         +                    "SHA224withRSA",        --p);
>         +            supports(HashAlgorithm.SHA224,
>         SignatureAlgorithm.ECDSA,
>         +                    "SHA224withECDSA",      --p);
>                  }
>         +
>         +        supports(HashAlgorithm.SHA256,      SignatureAlgorithm.DSA,
>         +                "SHA256withDSA",        --p);
>         +        supports(HashAlgorithm.SHA256,      SignatureAlgorithm.RSA,
>         +                "SHA256withRSA",        --p);
>         +        supports(HashAlgorithm.SHA256,
>         SignatureAlgorithm.ECDSA,
>         +                "SHA256withECDSA",      --p);
>         +        supports(HashAlgorithm.SHA384,      SignatureAlgorithm.RSA,
>         +                "SHA384withRSA",        --p);
>         +        supports(HashAlgorithm.SHA384,
>         SignatureAlgorithm.ECDSA,
>         +                "SHA384withECDSA",      --p);
>         +        supports(HashAlgorithm.SHA512,      SignatureAlgorithm.RSA,
>         +                "SHA512withRSA",        --p);
>         +        supports(HashAlgorithm.SHA512,
>         SignatureAlgorithm.ECDSA,
>         +                "SHA512withECDSA",      --p);
>              }
>          }
>
>


More information about the security-dev mailing list