Large allocation in CipherSuites.

[Verghese, Clive]

Thank you for pointing out the JEP. We could still consider this enhancement while we wait for this JEP to land in JDK. 

With regards to performance, I was unable to benchmark the functions alone as the CipherSuite is not part of the public API. I was able to benchmark SSLHandshakes and the results were as below. 

Benchmark                           Mode  Cnt  Score   Error  Units
CipherSuiteBench.initiateHandshake  avgt   25  4.532 ? 0.134  ms/op     [Current version using CipherSuite.values]
CipherSuiteBench.initiateHandshake  avgt   25  4.366 ? 0.037  ms/op     [Proposed change Static Array]

I will proceed to create the JBS issue and work on getting additional benchmarks and numbers. 

Regards,
Clive Verghese   


From: Xuelei Fan <xuelei.fan at oracle.com>
Date: Friday, July 9, 2021 at 11:43 AM
To: "Verghese, Clive" <verghese at amazon.com>
Cc: "security-dev at openjdk.java.net" <security-dev at openjdk.java.net>
Subject: RE: Large allocation in CipherSuites. 

BTW, it may worthy to track the development of the Frozen Arrays JEP: 
    https://openjdk.java.net/jeps/8261007

Xuelei


On Jul 9, 2021, at 11:21 AM, Xuelei Fan <mailto:xuelei.fan at oracle.com> wrote:

Hi Clive,

It’s a good point to me!  Did you have the numbers about the performance impact?

Considering the size of CipherSuites, I think it is good to make an improvement.  As we are already here, may be we could consider if we could make further performance improvement for searching as well.

Thanks,
Xuelei


On Jul 9, 2021, at 10:52 AM, Verghese, Clive <mailto:verghese at amazon.com> wrote:

Hi 

We have identified large number of allocations in CipherSuites[1]. The root cause for the allocations is that in the `CipherSuite.values` call in `nameof` and `valueof` functions. These functions are called by the SSLAlgorithmDecomposer and in SSLEngineImpl. The enumeration values functions clones the array before returning. A previous discussion on the compiler-dev channel[2] describes why the values function returns a clone. We would like to propose that the CipherSuite.values be stored in a `private static final` field [3]. This would prevent the need to clone the array for each lookup across the enum. 

The proposed change stores the ciphers as an array itself. The other alternative would be to store the values as a HashMap. However, I feel that this would not be optimal due to the ordering of the Enumeration. 

Looking for your feedback and recommendations. If the proposal look good, I can go ahead and create a JBS issue and submit a PR for the same. 

Regards,
Clive Verghese 

1 : https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/ssl/CipherSuite.java
2 : http://mail.openjdk.java.net/pipermail/compiler-dev/2018-July/012242.html
3 : https://github.com/cliveverghese/jdk/commit/8b34c06d8305ef9cb6a790e4cc8ca169c2fc9d79