Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
Looks good. My only question is whether the apiNote should be an
implNote instead since it refers to what the JDK Implementation does.
But either way seems ok.
--Sean
On 11/26/18 1:14 PM, Xue-Lei Fan wrote:
I made the update accordingly:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.04/
Thanks,
Xuelei
On 11/19/2018 7:39 AM, Sean Mullan wrote:
On 11/16/18 3:19 PM, Xuelei Fan wrote:
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.
Then maybe the default is what you should be discussing in this
apiNote. Right now I don't think the apiNote adds much. To me, all you
are really saying is that these are methods that can be used to tune
performance, which I think should be obvious from their name and
description. Maybe the apiNote should say something like:
"Note that the JDK Implementation uses default values for both the
session cache size and timeout. See getSessionCacheSize and
getSessionTimeout for more information. Applications should consider
their performance requirements and override the defaults if necessary."
Also I think you should add a similar @implNote for getSessionTimeout
which describes the default value (86400 seconds or 24 hours), ex:
@implNote The JDK implementation returns the session timeout as set by
the {@code setSessionTimeout} method, or if not set, a default
value of 86400 seconds (24 hours).
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.
Ok, but the apiNote above says nothing about the default value.
--Sean
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
I made the update accordingly:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.04/
Thanks,
Xuelei
On 11/19/2018 7:39 AM, Sean Mullan wrote:
On 11/16/18 3:19 PM, Xuelei Fan wrote:
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.
Then maybe the default is what you should be discussing in this
apiNote. Right now I don't think the apiNote adds much. To me, all you
are really saying is that these are methods that can be used to tune
performance, which I think should be obvious from their name and
description. Maybe the apiNote should say something like:
"Note that the JDK Implementation uses default values for both the
session cache size and timeout. See getSessionCacheSize and
getSessionTimeout for more information. Applications should consider
their performance requirements and override the defaults if necessary."
Also I think you should add a similar @implNote for getSessionTimeout
which describes the default value (86400 seconds or 24 hours), ex:
@implNote The JDK implementation returns the session timeout as set by
the {@code setSessionTimeout} method, or if not set, a default
value of 86400 seconds (24 hours).
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.
Ok, but the apiNote above says nothing about the default value.
--Sean
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
On 11/16/18 3:19 PM, Xuelei Fan wrote:
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.
Then maybe the default is what you should be discussing in this apiNote.
Right now I don't think the apiNote adds much. To me, all you are really
saying is that these are methods that can be used to tune performance,
which I think should be obvious from their name and description. Maybe
the apiNote should say something like:
"Note that the JDK Implementation uses default values for both the
session cache size and timeout. See getSessionCacheSize and
getSessionTimeout for more information. Applications should consider
their performance requirements and override the defaults if necessary."
Also I think you should add a similar @implNote for getSessionTimeout
which describes the default value (86400 seconds or 24 hours), ex:
@implNote The JDK implementation returns the session timeout as set by
the {@code setSessionTimeout} method, or if not set, a default
value of 86400 seconds (24 hours).
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.
Ok, but the apiNote above says nothing about the default value.
--Sean
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
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, but
it seems a bit odd to talk about the session timeout in this method. 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.
Also, unless you say what "too big" or "too small" is, I would avoid
giving this advice.
--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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
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.
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
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
* SSLSession objects grouped under this
* SSLSessionContext.
*
* @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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
On 11/14/2018 9:16 AM, Jamil Nimeh wrote: Hi Xuelei, The fix looks fine to me. I think it would be good to have an else branch off of the check on line 205 so any < 0 value has a warning log entry stating that an invalid value was detected and the cache is getting set to DEFAULT_MAX_CACHE_SIZE. Good point! I updated with warnings of invalid property: http://cr.openjdk.java.net/~xuelei/8210985/webrev.01/ Xuelei --Jamil On 11/14/2018 8: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
Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480
Hi Xuelei, The fix looks fine to me. I think it would be good to have an else branch off of the check on line 205 so any < 0 value has a warning log entry stating that an invalid value was detected and the cache is getting set to DEFAULT_MAX_CACHE_SIZE. --Jamil On 11/14/2018 8: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
