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

[Jamil Nimeh]

Hi Xuelei,

A little wordsmithing, nit picky stuff (sorry for not seeing this earlier):

  * @apiNote for setSessionCacheSize: The sentence beginning, "It is not
    recommended to use too big..." needs a slight grammatical change
      o Suggestion: "It is recommended that applications tune their
        timeout and cache size values so they are neither too small nor
        too large."
      o If you decide to change the language in the CSR then change it
        in SSLSessionContext.java, too.
  * @implNote for getSessionCacheSize: bring the closing brace in from
    "{@code setSessionCacheSize method}" to put the word method outside
    the braces.
      o This should change in the code review, SSLSessionContext.java:143

The rest is fine.

--Jamil

On 11/16/2018 10:30 AM, 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20181116/0f1054e3/attachment.htm>