RFR: 8270317: Large Allocation in CipherSuite

[Michael Bien]

On Wed, 14 Jul 2021 21:18:23 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> ### Benchmark results 
>> 
>> I have benchmarked 3 cases.
>> 
>> 1. The current situation. 
>> 
>> Benchmark                                                        (cipherSuite)  Mode  Cnt    Score   Error  Units
>> CipherSuiteBench.benchmarkCipherSuite                   TLS_AES_256_GCM_SHA384  avgt   25  124.783 ? 2.050  ns/op
>> CipherSuiteBench.benchmarkCipherSuite  TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384  avgt   25  125.403 ? 0.554  ns/op
>> CipherSuiteBench.benchmarkCipherSuite      TLS_DHE_DSS_WITH_AES_128_CBC_SHA256  avgt   25  127.117 ? 0.789  ns/op
>> CipherSuiteBench.benchmarkCipherSuite         TLS_DHE_RSA_WITH_AES_256_CBC_SHA  avgt   25  127.869 ? 1.112  ns/op
>> 
>> 
>> 2. Use `static final array` instead of calling `CipherSuite.values` each time. 
>> 
>> Benchmark                                                        (cipherSuite)  Mode  Cnt   Score   Error  Units
>> CipherSuiteBench.benchmarkCipherSuite                   TLS_AES_256_GCM_SHA384  avgt   25  10.146 ? 0.252  ns/op
>> CipherSuiteBench.benchmarkCipherSuite  TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384  avgt   25  30.501 ? 0.207  ns/op
>> CipherSuiteBench.benchmarkCipherSuite      TLS_DHE_DSS_WITH_AES_128_CBC_SHA256  avgt   25  47.375 ? 0.150  ns/op
>> CipherSuiteBench.benchmarkCipherSuite         TLS_DHE_RSA_WITH_AES_256_CBC_SHA  avgt   25  55.887 ? 3.786  ns/op
>> 
>> 
>> 3. Using Hashmap for lookup instead of iterating through the array each time. (Method in this PR)
>> 
>> Benchmark                                                        (cipherSuite)  Mode  Cnt   Score   Error  Units
>> CipherSuiteBench.benchmarkCipherSuite                   TLS_AES_256_GCM_SHA384  avgt   25  13.533 ? 0.148  ns/op
>> CipherSuiteBench.benchmarkCipherSuite  TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384  avgt   25  11.269 ? 0.147  ns/op
>> CipherSuiteBench.benchmarkCipherSuite      TLS_DHE_DSS_WITH_AES_128_CBC_SHA256  avgt   25  11.507 ? 0.107  ns/op
>> CipherSuiteBench.benchmarkCipherSuite         TLS_DHE_RSA_WITH_AES_256_CBC_SHA  avgt   25  10.932 ? 0.146  ns/op
>> 
>> 
>> I have picked 4 cipher suite from the start of the list and are roughly 10 positions apart. I have opted to go with HashMap for name and id lookup as they provide a more consistent times and benchmarks are similar for the first few cipher suits in the enum as well.
>
> src/java.base/share/classes/sun/security/ssl/CipherSuite.java line 865:
> 
>> 863:             maps_name.put(cs.name, cs);
>> 864:             for (String alias : cs.aliases) {
>> 865:                 maps_name.put(alias, cs);
> 
> A minor concern here. HashMap can't have duplicate keys.  what if there are duplicated names/aliases?
> 
> Even it's not the case now, CipherSuite is subject to change in the future and I think it is a good opportunity to detect key duplication here. Currently,  it's silently overwritten. This may introduce inconsistent behavior for nameOf.

this could be potentially stored in immutable collections which might be slightly more compact + they throw when they encounter duplicate keys
1) change base type to Map
2) copy ciphers array into Map.Entry array
3) maps_id = Map.ofEntries(entries) // + handle IAE

similar for the name map but with a list in between.

there might be a JDK internal shortcut to get to new ImmutableCollections.MapN<>(flatArray) right away without the Map.Entry step - I am sure a reviewer will chime in if there is.

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

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