On Wed, 5 May 2021 03:41:27 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:

>> Thank you for the update.
>> 
>> I also expect the code easy to read and maintain in the future.  But please 
>> go ahead for the integration if you don't want to make the update now.  We 
>> could file an enhancement later on.
>
>> @XueleiFan , @jnimeh, thank you for your suggestions. I have updated 
>> synchronization using ReentrantLock in the Cache object. The performance 
>> test showed no issues.
> 
> Thank you for considering use ReentrantLock, the "synchronized" keyword is 
> something we are trying to avoid in the JSSE implementation.  However, the 
> reentrant lock introduced is not the lock in the Cache implementation.  They 
> are overlapped and could be conflict.  The performance test looks good with 
> just a few threads, but I'm not very sure of the through put impact if the 
> threads goes up to 1M or more (there were complaints previously for large 
> volume traffics).
> 
> Maybe, you could keep it simple by adding a pull() method in the Cache class 
> (find the session and remove it from the cache if found), by using the Cache 
> class internal synchronize methods.

Unfortunately, simple pull() can not be used in this case. We have to check if 
the session found in the cache can be rejoined with parameters received in 
ClientHello and server context. Only rejoinable sessions should be removed from 
the session cache.
It is possible to use simple pull() and restore session in the cache if the 
session is not rejoinable, but I do not like this approach. Also, it will 
require extending Cache with get/setExpirationTime methods.

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

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

Reply via email to