RFR: 8259065: Optimize MessageDigest.getInstance [v3]

Claes Redestad redestad at openjdk.java.net
Thu Jan 7 04:09:00 UTC 2021


On Wed, 6 Jan 2021 20:52:19 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Claes Redestad has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add cloneInstance baseline micro
>
> src/java.base/share/classes/java/security/MessageDigest.java line 185:
> 
>> 183:         MessageDigest md;
>> 184: 
>> 185:         GetInstance.Instance instance = GetInstance.getInstance("MessageDigest",
> 
> There is another Security.getImpl call inside getInstance(String algorithm, String provider) method. For consistency sake, we should update it also?

Sure, why not. The Security.getImpl has a branch checking null provider which is pointless here since we guard against a null provider before that call, so we can reuse the same pattern here.

> src/java.base/share/classes/java/security/Provider.java line 1072:
> 
>> 1070:         }
>> 1071:         public int hashCode() {
>> 1072:             return type.hashCode() ^ algorithm.hashCode();
> 
> Is this change really necessary? It's faster to compute with ^, but does the generated hash values are as distinct as using Objects.hash()?

To avoid any suspicion we'd generate a worse hash here I've reverted this to inline what would be generated going through Objects.hash: `31*31 + type.hashCode()*31 + algorithm.hashCode()`

> src/java.base/share/classes/java/security/Provider.java line 1963:
> 
>> 1961:             Constructor<?> con = null;
>> 1962:             if (cache instanceof WeakReference<?> ref){
>> 1963:                 con = (ref == null) ? null : (Constructor<?>)ref.get();
> 
> ref should not be null if the instanceof check is true?

good catches - these 2 null checks can be removed

> src/java.base/share/classes/sun/security/jca/ProviderConfig.java line 170:
> 
>> 168:         }
>> 169:         // DCL
>> 170:         synchronized (ProviderConfig.class) {
> 
> synchronized (this) instead since 'provider' is an instance field?

yes!

-------------

PR: https://git.openjdk.java.net/jdk/pull/1933



More information about the security-dev mailing list