On Wed, 14 Jul 2021 21:18:23 GMT, Xin Liu <[email protected]> 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
