On Thu, 22 Jul 2021 19:01:02 GMT, Clive Verghese <cvergh...@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.
>
> Clive Verghese has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add blank space before and after the for loop
Marked as reviewed by djelin...@github.com (no known OpenJDK username).
src/java.base/share/classes/sun/security/ssl/CipherSuite.java line 971:
> 969:
> 970: boolean found = false;
> 971: for (CipherSuite cs : allowedCipherSuites) {
We can avoid this loop; look up the cipher by name first (`cs = nameOf(name)`),
then check for supported protocols.
Other than that, LGTM.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4783
