RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

[Xue-Lei Andrew Fan]

On Thu, 4 Feb 2021 20:45:55 GMT, djelinski <github.com+30433125+djelinski at openjdk.org> wrote:

> Thanks for your review! Some comments below.
> 
> > A full handshake or OCSP status grabbing could counteract all the performance gain with the cache update.
> 
> Yes, but that's unlikely. Note that K=3 is before K=1 in the queue only because 3 wasn't used since 1 was last used. This means that either K=3 is used less frequently than K=1, or that all cached items are in active use. In the former case we don't lose much by dropping K=3 (granted, there's nothing to offset that). In the latter we are dealing with full cache at all times, which means that most `put()`s would scan the queue, and we will gain a lot by finishing faster.

I may think it differently.  It may be hard to know the future frequency of an cached item based on the past behaviors.  For the above case, I'm not sure that K=3 is used less frequently than K=1.  Maybe, next few seconds, K=1 could be more frequently.

I would like a solution to following the timeout specification: keep the newer items if possible.

> 
> > get() [..] without [..] change the order of the queue
> 
> If we do that, frequently used entries will be evicted at the same age as never used ones. This means we will have to recompute (full handshake/fresh OCSP) both the frequently used and the infrequently used entries. It's better to recompute only the infrequently used ones, and reuse the frequently used as long as possible - we will do less work that way.
> That's probably the reason why a `LinkedHashMap` with `accessOrder=true` was chosen as the backing store implementation originally.
> 

See above.  It may be true for some case to determine the frequency, but Cache is a general class and we may want to be more careful about if we are really be able to determine the frequency within the Cache implementation.

> > get() performance is more important [..] so we should try to keep the cache small if possible
> 
> I don't see the link; could you explain?
> 

link? Did you mean the link to get() method?  It is a method in the Cache class.

> > In the update, the SoftReference.clear() get removed. I'm not sure of the impact of the enqueued objects any longer. In theory, it could improve the memory use, which could counteract the performance gain in some situation.
> 
> That's the best part: no objects ever get enqueued! We only called `clear()` right before losing the last reference to `SoftCacheEntry` (which is the `SoftReference`). When GC collects the `SoftReference`, it does not enqueue anything. GC only enqueues the `SoftReference` when it collects the referenced object (session / OCSP response) without collecting the `SoftReference` (cache entry) itself.
> This is [documented behavior](https://docs.oracle.com/javase/7/docs/api/java/lang/ref/package-summary.html): _If a registered reference becomes unreachable itself, then it will never be enqueued._
> 

I need more time for this section.

> > Could you edit the bug
> 
> I'd need an account on the bug tracker first.

Okay.  No worries, I will help you if we could get an agreement about the update.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2255