Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480

[Xuelei Fan]

Thanks for the review, Jmail & Sean.

New webrev:
     http://cr.openjdk.java.net/~xuelei/8210985/webrev.03/

I will update CSR when we come to an agreement.

On 11/16/2018 11:33 AM, Sean Mullan wrote:
>   122      * @apiNote Both the session timeout and cache size impact 
> performance
>   123      *          of future connections.  It is not recommended to 
> use too big
>   124      *          or too small timeout or cache size limit. 
> Applications should
>   125      *          carefully weigh the limits and performance for the 
> specific
>   126      *          circumstances.
> 
> I am not really sure if the @apiNote is that useful or appropriate,
I worry about the default value actually.  A new bug may be filed again 
and argue if the default value is not a proper one.  The default value 
of session timeout and cache size really depends on the real world 
circumstances.  I think we'd better make a clarification in the spec, 
and remind application tune them accordingly.

> but 
> it seems a bit odd to talk about the session timeout in this method. 
The performance impact is a combination of the session timeout limit and 
cache size.  I would like application consider them together if need to 
tune the values, but not individually.

> If 
> you still want to add this, I would add an @apiNote to each of the 
> setSessionCacheSize and setSessionTimeout methods and just discuss each 
> property separately.
> 
I added the update spec to both setSessionCacheSize and setSessionTimeout.

> Also, unless you say what "too big" or "too small" is, I would avoid 
> giving this advice.
> 
It makes sense to me.

Thanks,
Xuelei

> --Sean
> 
> 
> On 11/16/18 1:30 PM, Xuelei Fan wrote:
>> It's time to use the systemProperty tag as it is ready.
>>
>> As we are already there, I also update the setSessionCacheSize() for 
>> more clarification.
>>
>> Please review both CSR and webrev:
>>      https://bugs.openjdk.java.net/browse/JDK-8213577
>>      http://cr.openjdk.java.net/~xuelei/8210985/webrev.02/
>>
>> Thanks,
>> Xuelei
>>
>> On 11/16/2018 8:19 AM, Sean Mullan wrote:
>>> On 11/15/18 3:37 PM, Xuelei Fan wrote:
>>>> Hi Sean,
>>>>
>>>> Are you OK if we do it later?  I'm waiting for the @systemProperty 
>>>> tag, proposed within JDK-5076751.  I will file a bug to use the tag 
>>>> for more cleanup.
>>>
>>> JDK-5076751 is completed and pushed to JDK 12, so you can use the new 
>>> tag now.
>>>
>>> I think it would be easier to do it now, it seems pretty simple and 
>>> that way there is no need to worry about it later.
>>>
>>> --Sean
>>>
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>> On 11/15/2018 11:55 AM, Sean Mullan wrote:
>>>>> This is a good opportunity to document the 
>>>>> javax.net.ssl.sessionCacheSize system property in the 
>>>>> SSLSessionContext API (and use the new @systemProperty tag) in an 
>>>>> @implNote, for example:
>>>>>
>>>>>      /**
>>>>>       * Returns the size of the cache used for storing
>>>>>       * <code>SSLSession</code> objects grouped under this
>>>>>       * <code>SSLSessionContext</code>.
>>>>>       *
>>>>>       * @implNote The JDK implementation returns the cache size as 
>>>>> set by
>>>>>       * the {@code setSessionCacheSize method}, or if not set, the 
>>>>> value
>>>>>       * of the {@systemProperty javax.net.ssl.sessionCacheSize} system
>>>>>       * property. If neither is set, it returns a default value of 
>>>>> 20480.
>>>>>       *
>>>>>       * @return size of the session cache; zero means there is no 
>>>>> size limit.
>>>>>       * @see #setSessionCacheSize
>>>>>       */
>>>>>      public int getSessionCacheSize();
>>>>>
>>>>> On 11/14/18 11:59 AM, Xuelei Fan wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review this simple update:
>>>>>>      http://cr.openjdk.java.net/~xuelei/8210985/webrev.00/
>>>>>>
>>>>>> The default value for the maximum number of entries in the SSL 
>>>>>> session cache (SSLSessionContext.getSessionCacheSize()) is 
>>>>>> infinite now.  In the request, the default value is updated to 
>>>>>> 20480 for performance consideration.
>>>>>>
>>>>>> For the detailed behavior update, please refer to CSR:
>>>>>>      https://bugs.openjdk.java.net/browse/JDK-8213577
>>>>>>
>>>>>> Thanks,
>>>>>> Xuelei