On Thu, 29 Oct 2020 16:45:01 GMT, Christoph Langer <clan...@openjdk.org> wrote:
>>> > Did you have a benchmark with various cache sizes (for example, from 1 to >>> > 10K) and various connections (for example from 1 to 10K) for those >>> > components (including TLS implementation) that use Cache? >>> >>> Nope, we've just seen the memory regression in a certain customer use case >>> (lot's of meory retained by a finalizer) and confirmed it resolved after >>> setting javax.net.ssl.sessionCacheSize to 0. >>> >> Did you have this patch checked with the customer? I think the performance >> may be similar or improved comparing to set the cache size to 0. >> >>> But I guess this change merits certain further benchmarking to get it right. >>> >> It looks good to me, but we may be more confident with it if there is a >> benchmarking. > > We're currently rolling out a patch to our SAP JVM shipment (based on > Oracle's JDK 8 licensee repository) with exactly this content. We will then > check with the customer but I'd suspect his results will about the same as > with -Djavax.net.ssl.sessionCacheSize=0. > > If you require some benchmarking I guess it'll take me some more time. > > In the end I doubt that we'll find a better default value than 1 for the > cache size as it's hard to predict how full a cache will be in the average. > Maybe one could spend a property for the initial size - but I also doubt > that's worth the effort. > > I also think that for the use case in StatusResponseManager's responseCache > the influence of a different initial value is neglectable. I can confirm that JDK-8210985 can cause massive regressions in memory consumption. Also, [JDK-8253116 Performance regression observed post upgrade to 8u261](https://bugs.openjdk.java.net/browse/JDK-8253116) is clearly a duplicate for this. I'm fine with setting the initial cache size to 1 as this restores the original behavior for `javax.net.ssl.sessionCacheSize=0`. The other possibility would be to use the default size for `LinkedHashMap` but that's not easy if we want to set the `accessOrder` because the only constructor which takes the `accessOrder` also requires the specification of `initialCapacitiy` and `loadFactor`. Otherwise, `LinkedHashMap`s capacity defaults to `HashMap`s default capacity which is `16`. Setting the `initialCapacity` to `1` will initialize the hash map with one bucket (see initialization code in `HashMap`). /** * Returns a power of two size for the given target capacity. */ static final int tableSizeFor(int cap) { int n = cap - 1; n |= n >>> 1; n |= n >>> 2; n |= n >>> 4; n |= n >>> 8; n |= n >>> 16; return (n < 0) ? 1 : (n >= MAXIMUM_CAPACITY) ? MAXIMUM_CAPACITY : n + 1; } Benchmarking is probably hard because we don't know the average occupancy of the map. So in the end I think `1` is a good solution for now. ------------- PR: https://git.openjdk.java.net/jdk/pull/937
