AlgorithmConstraints caching [ was Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice]

[Seán Coffey]

I think the work done with 8284694 will help alot in any case since I 
suspect the same SSLAlgorithmConstraints Object will be shared much more 
now (rather than spin off new copies)

Some recent JFRs I looked at show that alot of CPU cycles[1] get taken 
in the HandshakeContext methods of :
sun.security.ssl.HandshakeContext#getActiveCipherSuites
sun.security.ssl.HandshakeContext#getActiveProtocols

Both methods get called per Handshakecontext construction and I think 
each TLS handshake gets a new Handshakecontext.I was thinking that a 
cache of the last known variables used to deduce the 
getActiveCipherSuites and getActiveProtocols Lists could be created. 
That might have the potential to avoid alot of needless CPU cycles in 
this area since the parameters used to produce these Lists don't really 
change that often. I'm still looking into this potential and hope to 
share a patch shortly.

regards,
Sean.

[1]

Stack Trace    Count    Percentage
TreeMap$Entry java.util.TreeMap.getEntryUsingComparator(Object) 1035    
100 %
TreeMap$Entry java.util.TreeMap.getEntry(Object)    1035    100 %
boolean java.util.TreeMap.containsKey(Object)    1035    100 %
boolean java.util.TreeSet.contains(Object)    1035    100 %
boolean 
sun.security.util.AbstractAlgorithmConstraints.checkAlgorithm(Set, 
String, AlgorithmDecomposer)    1035    100 %
boolean sun.security.util.DisabledAlgorithmConstraints.permits(Set, 
String, AlgorithmParameters)    1035    100 %
boolean sun.security.ssl.SSLAlgorithmConstraints.permits(Set, String, 
AlgorithmParameters)    1035    100 %
boolean sun.security.ssl.SSLAlgorithmConstraints.permits(Set, String, 
AlgorithmParameters)    553    53.4 %
boolean sun.security.ssl.HandshakeContext.isActivatable(CipherSuite, 
AlgorithmConstraints, Map)    309    29.9 %
List sun.security.ssl.HandshakeContext.getActiveCipherSuites(List, List, 
AlgorithmConstraints)    302    29.2 %
void sun.security.ssl.HandshakeContext.<init>(SSLContextImpl, 
TransportContext)    302    29.2 %
void sun.security.ssl.ClientHandshakeContext.<init>(SSLContextImpl, 
TransportContext)    302    29.2 %
void sun.security.ssl.TransportContext.kickstart()    302 29.2 %
void sun.security.ssl.SSLSocketImpl.startHandshake(boolean) 302    29.2 %


On 13/04/2022 18:05, Anthony Scarpino wrote:
> Hi Sean,
>
> Caching is an interesting idea.  I've wondered for a while off and on 
> about how to speed it up, but hadn't come up with a solution I liked. 
> The complication with caching is while something like an algorithm 
> name only could be easy in a hashmap, it gets more complicated when 
> you get into key sizes. Such as, how to represent RSA 1k being 
> disallowed and but 2k allowed.. or certificate usage..
>
> Tony
>
> On 4/13/22 2:03 AM, Sean Coffey wrote:
>> On Tue, 12 Apr 2022 11:28:12 GMT, Daniel Jeliński 
>> <djelinski at openjdk.org> wrote:
>>
>>> During TLS handshake, hundreds of constraints are evaluated to 
>>> determine which cipher suites are usable. Most of the evaluations 
>>> are performed using `HandshakeContext#algorithmConstraints` object. 
>>> By default that object contains a `SSLAlgorithmConstraints` instance 
>>> wrapping another `SSLAlgorithmConstraints` instance. As a result the 
>>> constraints defined in `SSLAlgorithmConstraints` are evaluated twice.
>>>
>>> This PR improves the default case; if the user-specified constraints 
>>> are left at defaults, we use a single `SSLAlgorithmConstraints` 
>>> instance, and avoid duplicate checks.
>>
>> Nice work. I've been looking at this area myself in recent weeks also 
>> while debugging some JDK 11u performance issues. JFR shows this area 
>> as costly. Some other JDK fixes in this area have already improved 
>> performance. This will certainly help. Pasting a stacktrace[1] from 
>> an old Oracle JDK 11.0.12 report for history purposes.
>>
>> I think there are other improvements that can be made in the TLS 
>> constraints code. Caching the last known values from a constraints 
>> permits test is something I've begun looking into.
>>
>> [1]
>> Stack Trace    Count    Percentage
>> boolean java.lang.StringLatin1.regionMatchesCI(byte[], int, byte[], 
>> int, int)    1637    100 %
>> boolean java.lang.String.regionMatches(boolean, int, String, int, 
>> int)    1637    100 %
>> boolean java.lang.String.equalsIgnoreCase(String)    1637 100 %
>> boolean 
>> sun.security.util.AbstractAlgorithmConstraints.checkAlgorithm(List, 
>> String, AlgorithmDecomposer)    1631    99.6 %
>> boolean sun.security.util.DisabledAlgorithmConstraints.permits(Set, 
>> String, AlgorithmParameters)    1631    99.6 %
>> boolean sun.security.ssl.SSLAlgorithmConstraints.permits(Set, String, 
>> AlgorithmParameters)    1631    99.6 %
>> boolean sun.security.ssl.SSLAlgorithmConstraints.permits(Set, String, 
>> AlgorithmParameters)    836    51.1 %
>> boolean sun.security.ssl.HandshakeContext.isActivatable(CipherSuite, 
>> AlgorithmConstraints, Map)    428    26.1 %
>> List sun.security.ssl.HandshakeContext.getActiveCipherSuites(List, 
>> List, AlgorithmConstraints)    418    25.5 %
>> void sun.security.ssl.HandshakeContext.<init>(SSLContextImpl, 
>> TransportContext)    418    25.5 %
>> void sun.security.ssl.ClientHandshakeContext.<init>(SSLContextImpl, 
>> TransportContext)    418    25.5 %
>> void sun.security.ssl.TransportContext.kickstart()    418 25.5 %
>> void sun.security.ssl.SSLSocketImpl.startHandshake(boolean) 418    
>> 25.5 %
>> void sun.security.ssl.SSLSocketImpl.startHandshake()    418 25.5 %
>>
>> -------------
>>
>> Marked as reviewed by coffeys (Reviewer).
>>
>> PR: https://git.openjdk.java.net/jdk/pull/8199