RFR: Remove map synchronization from SignatureAndHashAlgorithm

[Steven Davidovitz]

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?

On Tue, Apr 25, 2017 at 7:05 AM, Sean Mullan <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
>
> Also, have you signed the OCA? See http://www.oracle.com/technetw
> ork/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
>>
>> diff --git a/src/java.base/share/classes/sun/security/ssl/SignatureAndH
>> ashAlgorithm.java
>> b/src/java.base/share/classes/sun/security/ssl/SignatureAndH
>> ashAlgorithm.java
>> --- a/src/java.base/share/classes/sun/security/ssl/SignatureAndH
>> ashAlgorithm.java
>> +++ b/src/java.base/share/classes/sun/security/ssl/SignatureAndH
>> ashAlgorithm.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);
>>      }
>>  }
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20170426/851f6c56/attachment.htm>