[External] : Re: Large allocation in CipherSuites.

[Xuelei Fan]

Thank you!

Xuelei

> On Jul 9, 2021, at 1:53 PM, Verghese, Clive <verghese at amazon.com> wrote:
> 
> 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://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/ssl/CipherSuite.java__;!!ACWV5N9M2RV99hQ!c5q6Xvm78wjyt6nXSP7VgXH0HQ2fpiAcTuHRkn8pcTH5aOixqThhw_QfcC3rRhBd$ 
> 2 : http://mail.openjdk.java.net/pipermail/compiler-dev/2018-July/012242.html
> 3 : https://urldefense.com/v3/__https://github.com/cliveverghese/jdk/commit/8b34c06d8305ef9cb6a790e4cc8ca169c2fc9d79__;!!ACWV5N9M2RV99hQ!c5q6Xvm78wjyt6nXSP7VgXH0HQ2fpiAcTuHRkn8pcTH5aOixqThhw_QfcCmEpVan$ 
> 
> 
> 
>