On Tue, 27 Apr 2021 23:45:30 GMT, Alexey Bakhtin <abakh...@openjdk.org> wrote:
>> src/java.base/share/classes/sun/security/ssl/PreSharedKeyExtension.java line
>> 377:
>>
>>> 375: // If we are keeping state, see if the identity is
>>> in the cache
>>> 376: if (requestedId.identity.length ==
>>> SessionId.MAX_LENGTH) {
>>> 377: synchronized (sessionCache) {
>>
>> Did you have a test if there is a performance regression by introducing the
>> synchronization here?
>>
>> I have to check the engineGetServerSessionContext() specification and
>> implementation (if the sessionCache is a reference?), and the session cache
>> implementation to make sure the synchronization works (if the
>> synchronization on sessionCache is the same as the synchronization on the
>> cache internal implementation) . Maybe, the improvement could in the cache
>> implementation for readability?
>
> Yes, I’ve made a test that calculates total time spent by server to receive
> "N" connections. Every server handshake is performed in a separate thread
> The client starts "T" threads. Every thread sends one initial connection and
> "R-1" renegotiated connections. So, the total number of connections is
> "N"="T"*"R"
>
> Results for the original and JDK-8241248 code are almost identical:
> "T"=10 "R"=100
> Original OpenJDK: 1140 ms
> JDK-8241248 code: 1090 ms
>
> "T"=50 "R"=100
> Original OpenJDK: 1164 ms
> JDK-8241248 code: 1108 ms
>
> "T"=60 "R"=100
> Original OpenJDK: 1461 ms
> JDK-8241248 code: 1579 ms
>
> "T"=70 "R"=100
> Original OpenJDK: 2058 ms
> JDK-8241248 code: 1999 ms
>
> "T"=80 "R"=100
> Original OpenJDK: 2148 ms
> JDK-8241248 code: 2125 ms
>
> "T"=90 "R"=100
> Original OpenJDK: 2540 ms
> JDK-8241248 code: 2514 ms
>
> "T"=90 "R"=100
> Original OpenJDK: 3011 ms
> JDK-8241248 code: 3010 ms
>
> I can confirm that the synchronization code works. Without it, I still catch
> exception from different threads trying to resume the same session from the
> cache
Thank you for the update.
I would like to have the synchronization in the cache class for easier
maintainence. But please go ahead for the integration if you don't want to
make the update now. We could file an enhancement later on.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3664
