RFR: JDK-8255603: Memory/Performance regression after JDK-8210985

[Volker Simonis]

On Thu, 29 Oct 2020 16:45:01 GMT, Christoph Langer <clanger at 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