<div dir="ltr">Hi Xuelei,<div>Thank you, that would be great.</div><div>Yes, I created a microbenchmark and got some numbers. The benchmark just calls MemoryCache.put() in a loop; baseline results for different size & timeout values:</div><div><pre style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:11.9px;margin-top:0px;margin-bottom:16px;padding:16px;overflow:auto;line-height:1.45;border-radius:6px;color:rgb(36,41,46)"><code style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:11.9px;padding:0px;margin:0px;background:transparent;border-radius:6px;word-break:normal;border:0px;display:inline;overflow:visible;line-height:inherit">Benchmark (size) (timeout) Mode Cnt Score Error Units
CacheBench.put 20480 86400 avgt 25 83.653 ? 6.269 us/op
CacheBench.put 20480 0 avgt 25 0.107 ? 0.001 us/op
CacheBench.put 204800 86400 avgt 25 2057.781 ? 35.942 us/op
CacheBench.put 204800 0 avgt 25 0.108 ? 0.001 us/op</code></pre></div><div>And after applying my proposed changes:</div><div><pre style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:11.9px;margin-top:0px;margin-bottom:16px;padding:16px;overflow:auto;line-height:1.45;border-radius:6px;color:rgb(36,41,46)"><code style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:11.9px;padding:0px;margin:0px;background:transparent;border-radius:6px;word-break:normal;border:0px;display:inline;overflow:visible;line-height:inherit">Benchmark (size) (timeout) Mode Cnt Score Error Units
CacheBench.put 20480 86400 avgt 25 0.146 ? 0.002 us/op
CacheBench.put 20480 0 avgt 25 0.108 ? 0.002 us/op
CacheBench.put 204800 86400 avgt 25 0.150 ? 0.001 us/op
CacheBench.put 204800 0 avgt 25 0.106 ? 0.001 us/op</code></pre></div><div>Given that the server-side handshake takes about 2 milliseconds (same machine, TLS 1.2/ECDHE/ECDSA/secp256r1), I don't expect 0.1 microsecond cache access to be a point of contention, but I haven't field-tested it yet.</div><div><br></div><div>I posted my code and benchmark results here: <a href="https://github.com/openjdk/jdk/pull/2255">https://github.com/openjdk/jdk/pull/2255</a>. I'm still struggling with the company's OCA (signed a few years and a few corporate changes ago, and not used in a long time), I hope to get this sorted soon. I went with minimal changes to the existing implementation, hoping that smaller changes will be more likely to be approved for backporting than complete rewrites.</div><div><br></div><div>I had a quick look at your SessionCache. It looks like your put() operation is constant-time already, so not much else for me to improve. However, please correct me if I'm wrong, but I think SoftCacheEntry.unlink() should be called with a lock held at all times, shouldn't it? SessionCache.get() may call remove() without lock. Also there's no protection against calling unlink() twice on the same entry (for example from concurrent get() calls), which could wreck the linked list; setting prev=next=null would have avoided this.</div><div><br></div><div>Thanks,<br></div><div>Daniel</div><div><br></div><div><br></div><div><div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">śr., 27 sty 2021 o 20:36 Xue-Lei Fan <<a href="mailto:xuelei.fan@oracle.com">xuelei.fan@oracle.com</a>> napisał(a):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="overflow-wrap: break-word;">
Hi Daniel,
<div><br>
</div>
<div>
<div>I would like to help you out. Did you have some numbers about the performance improvement of your evaluation? I had a <a href="https://github.com/XueleiFan/jdk/blob/jdk-8245576/src/java.base/share/classes/sun/security/ssl/SSLSessionContextImpl.java" target="_blank">draft
re-write cache</a> by using ConcurrentHashMap, if you have a chance, would you like to evaluate if it could be improved further with your ideas?</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Xuelei</div>
<div><br>
<blockquote type="cite">
<div>On Jan 27, 2021, at 1:28 AM, Daniel Jeliński <<a href="mailto:djelinski1@gmail.com" target="_blank">djelinski1@gmail.com</a>> wrote:</div>
<br>
<div>
<div dir="ltr">Hi all,
<div>I'd like to modify the MemoryCache class that is used for caching SSL sessions in Java 11; when the cache is overloaded (full cache with no expired entries), the computational complexity of put operation is linear in the cache size.</div>
<div><br>
</div>
<div>I am aware that the current Java versions are using stateless resumption; my company's policy is to use Java LTS, so stateless is not an option to us at the moment.</div>
<div><br>
</div>
<div>The change I propose would alter the behavior of MemoryCache; currently the cache guarantees that expired entries are removed before live entries when the cache capacity is reached. I'd like to remove that guarantee, and instead always remove
the least recently used cache entry. Additionally, to keep the memory use in check, I'd like to check the least recently used entries for expiration, and remove them as needed. This operation would be run on every put().</div>
<div><br>
</div>
<div>Evaluated alternatives:</div>
<div>- keep MemoryCache as is, increase cache size to avoid overloading. This would reduce the frequency of check for expired entries, but if the larger cache eventually gets overloaded, the put() operation would take even longer to complete.</div>
<div>- keep MemoryCache behavior as is, but improve performance of removing expired entries. This would require a new data structure - we would need to have cache entries sorted both by access time and creation time.</div>
<div>- always remove entries in insertion order, regardless if they are used or not.</div>
<div>- always remove the least recently used entries, even if they are still valid and there are recently used expired entries somewhere in the cache. This is my preferred option.</div>
<div><br>
</div>
<div>Additionally, we can choose how to remove stale entries:</div>
<div>- all reachable stale entries on every put (that is, iterate over the internal data structure removing stale entries until a non-stale one is found); keeps memory use low, but can occasionally scan the entire cache (computational complexity: linear
worst case, amortized constant); </div>
<div>- fixed number of reachable stale entries on every put (same as above, but stop iterating after a fixed number of removed entries even if more are available); execution time is bounded by a constant, but some entries may stay in cache a bit longer
than they need to.</div>
<div>- all stale entries on put that would otherwise exceed cache capacity (current implementation)<br>
</div>
<div>- never; once cache reaches full capacity, it stays full until the application is restarted. Fastest implementation, but at a cost of increased memory use.</div>
<div><br>
</div>
<div>My preference would be to remove either all or a fixed number of reachable stale entries.</div>
<div><br>
</div>
<div>I'm willing to prepare a patch for 17 and 11u, if I can find a sponsor.</div>
<div>Thanks,</div>
<div>Daniel</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote></div>