On Sat, 23 Apr 2022 14:57:01 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> Profiling the TLS handshakes using SSLHandshake benchmark shows that a large >> portion of time is spent in HandshakeContext initialization, specifically in >> DisabledAlgorithmConstraints class. >> >> There are only a few instances of that class, and they are immutable. >> Caching the results should be a low-risk operation. >> >> The cache is implemented as a softly reachable ConcurrentHashMap; this way >> it can be removed from memory after a period of inactivity. Under normal >> circumstances the cache holds no more than 100 algorithms. > > src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java > line 105: > >> 103: private final Set<String> disabledAlgorithms; >> 104: private final Constraints algorithmConstraints; >> 105: private volatile SoftReference<Map<String, Boolean>> cacheRef = > > It looks like a one-clear-for-all mechanism. Once the clearing happens, the > full map will be collected. For SoftReferences, it clears them fairly > eagerly. Maybe, the performance could be further improved in practice by > using soft reference for each map entry (See sun.security.util.Cache code for > an example). > > I will have another look next week. Right, soft references are likely to be cleaned if they are not used in an entire GC cycle. Using a soft reference for each map entry would not help here; note that all keys and all values in this map are GC roots (keys are enum names, values are `TRUE` and `FALSE`), so in practice they would never be collected. Even if we made the entries collectable, it would not help with the TLS case; SSLEngine & SSLSocket only check the constraints once at the beginning of the handshake, so they would all expire at the same time anyway. What's even worse, we would then have to clear the expired entries from the map. I also considered limiting the size of the map. That's a tricky subject; we would need to get the size limit exactly right. Given that the algorithms are always checked in the same order, if the cache were too small, we would get zero hits. With all the above in mind I decided not to use `sun.security.util.Cache` here. ------------- PR: https://git.openjdk.java.net/jdk/pull/8349
