Hi Tony & Sean, As it turns out, caching the availability of algorithms is sufficient to get a massive speedup here. Check out the results on https://github.com/openjdk/jdk/pull/8349 and let me know what you think.
Regards, Daniel śr., 20 kwi 2022 o 22:22 Seán Coffey <sean.cof...@oracle.com> napisał(a): > > 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 > >> <djelin...@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
