Re: RFR: Remove map synchronization from SignatureAndHashAlgorithm

[Sean Mullan]

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.mul...@oracle.com
<mailto:sean.mul...@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);
             }
         }