You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by fh...@apache.org on 2012/08/27 16:20:55 UTC
svn commit: r1377689 -
/tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Author: fhanik
Date: Mon Aug 27 14:20:55 2012
New Revision: 1377689
URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
Log:
Per http://markmail.org/message/nqnogctvfuyzhtol
1. Already encountered two users that would like to set this value. There is
never any need to hard code any value, regardless of its use
2. This turns it into a property on the listener
Modified:
tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Modified: tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java?rev=1377689&r1=1377688&r2=1377689&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java Mon Aug 27 14:20:55 2012
@@ -218,6 +218,17 @@ public class JreMemoryLeakPreventionList
this.classesToInitialize = classesToInitialize;
}
+ /**
+ * Sets the time that this listener will request for garbage-collection latency
+ * @see {@link sun.misc.GC#requestLatency(long)}
+ */
+ private long gcDaemonPeriod = Long.MAX_VALUE - 1;
+ public long getGcDaemonPeriod() {
+ return gcDaemonPeriod;
+ }
+ public void setGcDaemonPeriod(long gcDaemonPeriod) {
+ this.gcDaemonPeriod = gcDaemonPeriod;
+ }
@Override
public void lifecycleEvent(LifecycleEvent event) {
@@ -297,7 +308,7 @@ public class JreMemoryLeakPreventionList
Method method = clazz.getDeclaredMethod(
"requestLatency",
new Class[] {long.class});
- method.invoke(null, Long.getLong("org.apache.catalina.core.jreMemoryLeakPreventionGCDaemonPeriod", Long.valueOf(Long.MAX_VALUE-1)));
+ method.invoke(null, getGcDaemonPeriod());
} catch (ClassNotFoundException e) {
if (System.getProperty("java.vendor").startsWith(
"Sun")) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by "Filip Hanik (mailing lists)" <de...@hanik.com>.
> -----Original Message-----
> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
> Sent: Monday, August 27, 2012 3:41 PM
> To: Tomcat Developers List
> Subject: Re: svn commit: r1377689 -
> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
> ner.java
>
> 2012/8/28 Filip Hanik (mailing lists) <de...@hanik.com>:
> >>
> >> There are documentation glitches yet to be fixed:
> >> a. systemprops.xml change in trunk was not reverted by this commit.
> >> It was reverted in 7.0.x only.
> > [Filip Hanik]
> > I don't see the property in trunk, do you?
>
> I took care of that an hour ago.
> http://svn.apache.org/viewvc?rev=1377831&view=rev
[Filip Hanik]
Got it, what's the point of the following code change?
- method.invoke(null, getGcDaemonPeriod());
+ method.invoke(null,
Long.valueOf(getGcDaemonPeriod()));
>
> >
> >> b. The new property is yet to be documented in listeners.xml.
> > [Filip Hanik]
> > Done
> >
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/8/28 Filip Hanik (mailing lists) <de...@hanik.com>:
>>
>> There are documentation glitches yet to be fixed:
>> a. systemprops.xml change in trunk was not reverted by this commit.
>> It was reverted in 7.0.x only.
> [Filip Hanik]
> I don't see the property in trunk, do you?
I took care of that an hour ago.
http://svn.apache.org/viewvc?rev=1377831&view=rev
>
>> b. The new property is yet to be documented in listeners.xml.
> [Filip Hanik]
> Done
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Mark Thomas <ma...@apache.org>.
On 27/08/2012 23:08, Konstantin Kolinko wrote:
> 2012/8/28 Mark Thomas <ma...@apache.org>:
>> On 27/08/2012 22:48, Filip Hanik (mailing lists) wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mark Thomas [mailto:markt@apache.org]
>>>> Sent: Monday, August 27, 2012 3:44 PM
>>>> To: Tomcat Developers List
>>>> Subject: Re: svn commit: r1377689 -
>>>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>> ner.java
>>>>
>>>> On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
>>>>>> -----Original Message-----
>>>>>> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
>>>>>> Sent: Monday, August 27, 2012 2:09 PM
>>>>>> To: Tomcat Developers List
>>>>>> Subject: Re: svn commit: r1377689 -
>>>>>>
>>>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>>>> ner.java
>>>>>>
>>>>>> 2012/8/27 Mark Thomas <ma...@apache.org>:
>>>>>>> On 27/08/2012 15:20, fhanik@apache.org wrote:
>>>>>>>> Author: fhanik
>>>>>>>> Date: Mon Aug 27 14:20:55 2012
>>>>>>>> New Revision: 1377689
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
>>>>>>>> Log:
>>>>>>>> Per http://markmail.org/message/nqnogctvfuyzhtol
>>>>>>>>
>>>>>>>> 1. Already encountered two users that would like to set this value.
>>>>>> There is
>>>>>>>> never any need to hard code any value, regardless of its use
>>>>>>>
>>>>>>> What is the use case for wanting to set this value? I can understand
>>>>>>> users not liking the previous value that triggered a full GC every
>>>>>> hour
>>>>>>> and wanting to change that but I fail to see why anyone would want
>>>> to
>>>>>>> change this now it is set to trigger a full GC every 290 million
>>>> years
>>>>>>> or so.
>>>>
>>>>>> Maybe somebody wants their full GC once an hour, or once a day?
>>>>
>>>> That is not what this listener is for. The listener's purpose is to
>>>> prevent memory leaks, not provide options that allow users to tinker
>>>> with internal JVM GC settings.
>>>>
>>>> I have yet to see a valid use case for this new attribute.
>>> [Filip Hanik]
>>> The use case is very much valid, as if they had previously called that
>>> method, your code will override it.
>>> So in effect, you're hard coding the GC interval, but not letting a user
>>> control it.
>>
>> Nope. You should have looked at the implementation of
>> sun.misc.GC#requestLatency(long) rather than assuming how it worked.
>>
>>> It's not tomcat's role to configure GC intervals. It may be that tomcat
>>> somehow initiated the GC interval, and if that is the case, it must expose
>>> the actual interval to the user. Tomcat should not change JVM settings
>>> without letting the user configure them,
>>
>> Tomcat setting this value has zero impact on any user code or JRE code
>> that sets a lower value either before Tomcat sets it or after.
>>
>> I still see no valid use case for this attribute and without a valid use
>> case my veto remains.
>>
>
> Agreed.
>
> When a user wants to configure this value by themselves, they should
> just disable this feature in Tomcat with gcDaemonProtection="false".
They don't even need to do that. In the unlikely event of user code
setting this or the more likely event that JRE code sets this, then the
shortest period requested is used. Cancelling the request is also
supported at which point the next shortest period is used and so on.
On that note, I suppose that technically we should cancel the request we
make but I don't think any JVM will be up long enough for it to matter.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/8/28 Mark Thomas <ma...@apache.org>:
> On 27/08/2012 22:48, Filip Hanik (mailing lists) wrote:
>>
>>
>>> -----Original Message-----
>>> From: Mark Thomas [mailto:markt@apache.org]
>>> Sent: Monday, August 27, 2012 3:44 PM
>>> To: Tomcat Developers List
>>> Subject: Re: svn commit: r1377689 -
>>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>> ner.java
>>>
>>> On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
>>>>> -----Original Message-----
>>>>> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
>>>>> Sent: Monday, August 27, 2012 2:09 PM
>>>>> To: Tomcat Developers List
>>>>> Subject: Re: svn commit: r1377689 -
>>>>>
>>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>>> ner.java
>>>>>
>>>>> 2012/8/27 Mark Thomas <ma...@apache.org>:
>>>>>> On 27/08/2012 15:20, fhanik@apache.org wrote:
>>>>>>> Author: fhanik
>>>>>>> Date: Mon Aug 27 14:20:55 2012
>>>>>>> New Revision: 1377689
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
>>>>>>> Log:
>>>>>>> Per http://markmail.org/message/nqnogctvfuyzhtol
>>>>>>>
>>>>>>> 1. Already encountered two users that would like to set this value.
>>>>> There is
>>>>>>> never any need to hard code any value, regardless of its use
>>>>>>
>>>>>> What is the use case for wanting to set this value? I can understand
>>>>>> users not liking the previous value that triggered a full GC every
>>>>> hour
>>>>>> and wanting to change that but I fail to see why anyone would want
>>> to
>>>>>> change this now it is set to trigger a full GC every 290 million
>>> years
>>>>>> or so.
>>>
>>>>> Maybe somebody wants their full GC once an hour, or once a day?
>>>
>>> That is not what this listener is for. The listener's purpose is to
>>> prevent memory leaks, not provide options that allow users to tinker
>>> with internal JVM GC settings.
>>>
>>> I have yet to see a valid use case for this new attribute.
>> [Filip Hanik]
>> The use case is very much valid, as if they had previously called that
>> method, your code will override it.
>> So in effect, you're hard coding the GC interval, but not letting a user
>> control it.
>
> Nope. You should have looked at the implementation of
> sun.misc.GC#requestLatency(long) rather than assuming how it worked.
>
>> It's not tomcat's role to configure GC intervals. It may be that tomcat
>> somehow initiated the GC interval, and if that is the case, it must expose
>> the actual interval to the user. Tomcat should not change JVM settings
>> without letting the user configure them,
>
> Tomcat setting this value has zero impact on any user code or JRE code
> that sets a lower value either before Tomcat sets it or after.
>
> I still see no valid use case for this attribute and without a valid use
> case my veto remains.
>
Agreed.
When a user wants to configure this value by themselves, they should
just disable this feature in Tomcat with gcDaemonProtection="false".
(
>> I took care of that an hour ago.
>> http://svn.apache.org/viewvc?rev=1377831&view=rev
> [Filip Hanik]
> Got it, what's the point of the following code change?
> - method.invoke(null, getGcDaemonPeriod());
> + method.invoke(null,
> Long.valueOf(getGcDaemonPeriod()));
There was implicit boxing operation, which gives a warning with our
Eclipse settings
(the ones in res/ide-support/eclipse/java-compiler-errors-warnings.txt )
)
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Rainer Jung <ra...@kippdata.de>.
On 28.08.2012 01:16, Filip Hanik (mailing lists) wrote:
>
>
>> -----Original Message-----
>> From: Mark Thomas [mailto:markt@apache.org]
>> Sent: Monday, August 27, 2012 3:55 PM
>> To: Tomcat Developers List
>> Subject: Re: svn commit: r1377689 -
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>> ner.java
>>
>> On 27/08/2012 22:48, Filip Hanik (mailing lists) wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mark Thomas [mailto:markt@apache.org]
>>>> Sent: Monday, August 27, 2012 3:44 PM
>>>> To: Tomcat Developers List
>>>> Subject: Re: svn commit: r1377689 -
>>>>
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>> ner.java
>>>>
>>>> On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
>>>>>> -----Original Message-----
>>>>>> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
>>>>>> Sent: Monday, August 27, 2012 2:09 PM
>>>>>> To: Tomcat Developers List
>>>>>> Subject: Re: svn commit: r1377689 -
>>>>>>
>>>>
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>>>> ner.java
>>>>>>
>>>>>> 2012/8/27 Mark Thomas <ma...@apache.org>:
>>>>>>> On 27/08/2012 15:20, fhanik@apache.org wrote:
>>>>>>>> Author: fhanik
>>>>>>>> Date: Mon Aug 27 14:20:55 2012
>>>>>>>> New Revision: 1377689
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
>>>>>>>> Log:
>>>>>>>> Per http://markmail.org/message/nqnogctvfuyzhtol
>>>>>>>>
>>>>>>>> 1. Already encountered two users that would like to set this
>> value.
>>>>>> There is
>>>>>>>> never any need to hard code any value, regardless of its use
>>>>>>>
>>>>>>> What is the use case for wanting to set this value? I can
>> understand
>>>>>>> users not liking the previous value that triggered a full GC every
>>>>>> hour
>>>>>>> and wanting to change that but I fail to see why anyone would want
>>>> to
>>>>>>> change this now it is set to trigger a full GC every 290 million
>>>> years
>>>>>>> or so.
>>>>
>>>>>> Maybe somebody wants their full GC once an hour, or once a day?
>>>>
>>>> That is not what this listener is for. The listener's purpose is to
>>>> prevent memory leaks, not provide options that allow users to tinker
>>>> with internal JVM GC settings.
>>>>
>>>> I have yet to see a valid use case for this new attribute.
>>> [Filip Hanik]
>>> The use case is very much valid, as if they had previously called that
>>> method, your code will override it.
>>> So in effect, you're hard coding the GC interval, but not letting a
>> user
>>> control it.
>>
>> Nope. You should have looked at the implementation of
>> sun.misc.GC#requestLatency(long) rather than assuming how it worked.
>>
>>> It's not tomcat's role to configure GC intervals. It may be that
>> tomcat
>>> somehow initiated the GC interval, and if that is the case, it must
>> expose
>>> the actual interval to the user. Tomcat should not change JVM settings
>>> without letting the user configure them,
>>
>> Tomcat setting this value has zero impact on any user code or JRE code
>> that sets a lower value either before Tomcat sets it or after.
>>
>> I still see no valid use case for this attribute and without a valid use
>> case my veto remains.
> [Filip Hanik]
> Now you're just being stubborn. It would be like me going back and vetoing
> the hard coded value, and we'd run around in circles like little chickens.
> The reason I think the veto is unreasonable is that there is no
> functionality removed with this. There is nothing to be lost.
>
> IIRC any call changes the value, since there is only one daemon thread
> created. And since gcDaemonProtection is true by default means that 99.9% of
> tomcat instances will have this daemon thread running. Since we have this
> thread running, then we might as well hand out the ability to the users.
> Since you are turning this thread on, give them the ability to change the
> interval at which it is running.
>
> 141 } else {
> 142 /* Notify the existing daemon thread
> 143 * that the lateency target has changed
> 144 */
> 145 lock.notify();
> 146 }
I also checked the GC class and I agree with Mark. The GC daemon
protection just adds a LatencyRequest to a sorted set of these. They are
sorted by the latency, so our huge latency will result in the request
always be added to the end of the list and since it is so huge never
actually trigger. We don't interfere with any LatencyRequest created
before or after our own actions. The only difference is, that the thread
is created and we keep it running, but AFAIK that's the whole purpose of
that kind of leak prevention.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Mark Thomas <ma...@apache.org>.
On 28/08/2012 00:16, Filip Hanik (mailing lists) wrote:
>> -----Original Message-----
>> From: Mark Thomas [mailto:markt@apache.org]
>> Sent: Monday, August 27, 2012 3:55 PM
>> To: Tomcat Developers List
>> Subject: Re: svn commit: r1377689 -
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>> ner.java
>>
>> On 27/08/2012 22:48, Filip Hanik (mailing lists) wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mark Thomas [mailto:markt@apache.org]
>>>> Sent: Monday, August 27, 2012 3:44 PM
>>>> To: Tomcat Developers List
>>>> Subject: Re: svn commit: r1377689 -
>>>>
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>> ner.java
>>>>
>>>> On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
>>>>>> -----Original Message-----
>>>>>> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
>>>>>> Sent: Monday, August 27, 2012 2:09 PM
>>>>>> To: Tomcat Developers List
>>>>>> Subject: Re: svn commit: r1377689 -
>>>>>>
>>>>
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>>>> ner.java
>>>>>>
>>>>>> 2012/8/27 Mark Thomas <ma...@apache.org>:
>>>>>>> On 27/08/2012 15:20, fhanik@apache.org wrote:
>>>>>>>> Author: fhanik
>>>>>>>> Date: Mon Aug 27 14:20:55 2012
>>>>>>>> New Revision: 1377689
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
>>>>>>>> Log:
>>>>>>>> Per http://markmail.org/message/nqnogctvfuyzhtol
>>>>>>>>
>>>>>>>> 1. Already encountered two users that would like to set this
>> value.
>>>>>> There is
>>>>>>>> never any need to hard code any value, regardless of its use
>>>>>>>
>>>>>>> What is the use case for wanting to set this value? I can
>> understand
>>>>>>> users not liking the previous value that triggered a full GC every
>>>>>> hour
>>>>>>> and wanting to change that but I fail to see why anyone would want
>>>> to
>>>>>>> change this now it is set to trigger a full GC every 290 million
>>>> years
>>>>>>> or so.
>>>>
>>>>>> Maybe somebody wants their full GC once an hour, or once a day?
>>>>
>>>> That is not what this listener is for. The listener's purpose is to
>>>> prevent memory leaks, not provide options that allow users to tinker
>>>> with internal JVM GC settings.
>>>>
>>>> I have yet to see a valid use case for this new attribute.
>>> [Filip Hanik]
>>> The use case is very much valid, as if they had previously called that
>>> method, your code will override it.
>>> So in effect, you're hard coding the GC interval, but not letting a
>> user
>>> control it.
>>
>> Nope. You should have looked at the implementation of
>> sun.misc.GC#requestLatency(long) rather than assuming how it worked.
>>
>>> It's not tomcat's role to configure GC intervals. It may be that
>> tomcat
>>> somehow initiated the GC interval, and if that is the case, it must
>> expose
>>> the actual interval to the user. Tomcat should not change JVM settings
>>> without letting the user configure them,
>>
>> Tomcat setting this value has zero impact on any user code or JRE code
>> that sets a lower value either before Tomcat sets it or after.
>>
>> I still see no valid use case for this attribute and without a valid use
>> case my veto remains.
> [Filip Hanik]
> Now you're just being stubborn.
No, I am being consistent. I am against unnecessary bloat.
> It would be like me going back and vetoing
> the hard coded value, and we'd run around in circles like little chickens.
> The reason I think the veto is unreasonable is that there is no
> functionality removed with this. There is nothing to be lost.
It doesn't add anything either. It is pointless bloat.
> IIRC any call changes the value, since there is only one daemon thread
> created.
Then again, I suggest you actually go and look at the source code and
you'd see that you are wrong.
And since gcDaemonProtection is true by default means that 99.9% of
> tomcat instances will have this daemon thread running. Since we have this
> thread running, then we might as well hand out the ability to the users.
> Since you are turning this thread on, give them the ability to change the
> interval at which it is running.
Again, provide a valid use case for this option and I'll support the
change. You have yet to do so. My veto stands.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by "Filip Hanik (mailing lists)" <de...@hanik.com>.
> -----Original Message-----
> From: Mark Thomas [mailto:markt@apache.org]
> Sent: Monday, August 27, 2012 3:55 PM
> To: Tomcat Developers List
> Subject: Re: svn commit: r1377689 -
> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
> ner.java
>
> On 27/08/2012 22:48, Filip Hanik (mailing lists) wrote:
> >
> >
> >> -----Original Message-----
> >> From: Mark Thomas [mailto:markt@apache.org]
> >> Sent: Monday, August 27, 2012 3:44 PM
> >> To: Tomcat Developers List
> >> Subject: Re: svn commit: r1377689 -
> >>
> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
> >> ner.java
> >>
> >> On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
> >>>> -----Original Message-----
> >>>> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
> >>>> Sent: Monday, August 27, 2012 2:09 PM
> >>>> To: Tomcat Developers List
> >>>> Subject: Re: svn commit: r1377689 -
> >>>>
> >>
> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
> >>>> ner.java
> >>>>
> >>>> 2012/8/27 Mark Thomas <ma...@apache.org>:
> >>>>> On 27/08/2012 15:20, fhanik@apache.org wrote:
> >>>>>> Author: fhanik
> >>>>>> Date: Mon Aug 27 14:20:55 2012
> >>>>>> New Revision: 1377689
> >>>>>>
> >>>>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
> >>>>>> Log:
> >>>>>> Per http://markmail.org/message/nqnogctvfuyzhtol
> >>>>>>
> >>>>>> 1. Already encountered two users that would like to set this
> value.
> >>>> There is
> >>>>>> never any need to hard code any value, regardless of its use
> >>>>>
> >>>>> What is the use case for wanting to set this value? I can
> understand
> >>>>> users not liking the previous value that triggered a full GC every
> >>>> hour
> >>>>> and wanting to change that but I fail to see why anyone would want
> >> to
> >>>>> change this now it is set to trigger a full GC every 290 million
> >> years
> >>>>> or so.
> >>
> >>>> Maybe somebody wants their full GC once an hour, or once a day?
> >>
> >> That is not what this listener is for. The listener's purpose is to
> >> prevent memory leaks, not provide options that allow users to tinker
> >> with internal JVM GC settings.
> >>
> >> I have yet to see a valid use case for this new attribute.
> > [Filip Hanik]
> > The use case is very much valid, as if they had previously called that
> > method, your code will override it.
> > So in effect, you're hard coding the GC interval, but not letting a
> user
> > control it.
>
> Nope. You should have looked at the implementation of
> sun.misc.GC#requestLatency(long) rather than assuming how it worked.
>
> > It's not tomcat's role to configure GC intervals. It may be that
> tomcat
> > somehow initiated the GC interval, and if that is the case, it must
> expose
> > the actual interval to the user. Tomcat should not change JVM settings
> > without letting the user configure them,
>
> Tomcat setting this value has zero impact on any user code or JRE code
> that sets a lower value either before Tomcat sets it or after.
>
> I still see no valid use case for this attribute and without a valid use
> case my veto remains.
[Filip Hanik]
Now you're just being stubborn. It would be like me going back and vetoing
the hard coded value, and we'd run around in circles like little chickens.
The reason I think the veto is unreasonable is that there is no
functionality removed with this. There is nothing to be lost.
IIRC any call changes the value, since there is only one daemon thread
created. And since gcDaemonProtection is true by default means that 99.9% of
tomcat instances will have this daemon thread running. Since we have this
thread running, then we might as well hand out the ability to the users.
Since you are turning this thread on, give them the ability to change the
interval at which it is running.
141 } else {
142 /* Notify the existing daemon thread
143 * that the lateency target has changed
144 */
145 lock.notify();
146 }
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Mark Thomas <ma...@apache.org>.
On 27/08/2012 22:48, Filip Hanik (mailing lists) wrote:
>
>
>> -----Original Message-----
>> From: Mark Thomas [mailto:markt@apache.org]
>> Sent: Monday, August 27, 2012 3:44 PM
>> To: Tomcat Developers List
>> Subject: Re: svn commit: r1377689 -
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>> ner.java
>>
>> On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
>>>> -----Original Message-----
>>>> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
>>>> Sent: Monday, August 27, 2012 2:09 PM
>>>> To: Tomcat Developers List
>>>> Subject: Re: svn commit: r1377689 -
>>>>
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>>>> ner.java
>>>>
>>>> 2012/8/27 Mark Thomas <ma...@apache.org>:
>>>>> On 27/08/2012 15:20, fhanik@apache.org wrote:
>>>>>> Author: fhanik
>>>>>> Date: Mon Aug 27 14:20:55 2012
>>>>>> New Revision: 1377689
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
>>>>>> Log:
>>>>>> Per http://markmail.org/message/nqnogctvfuyzhtol
>>>>>>
>>>>>> 1. Already encountered two users that would like to set this value.
>>>> There is
>>>>>> never any need to hard code any value, regardless of its use
>>>>>
>>>>> What is the use case for wanting to set this value? I can understand
>>>>> users not liking the previous value that triggered a full GC every
>>>> hour
>>>>> and wanting to change that but I fail to see why anyone would want
>> to
>>>>> change this now it is set to trigger a full GC every 290 million
>> years
>>>>> or so.
>>
>>>> Maybe somebody wants their full GC once an hour, or once a day?
>>
>> That is not what this listener is for. The listener's purpose is to
>> prevent memory leaks, not provide options that allow users to tinker
>> with internal JVM GC settings.
>>
>> I have yet to see a valid use case for this new attribute.
> [Filip Hanik]
> The use case is very much valid, as if they had previously called that
> method, your code will override it.
> So in effect, you're hard coding the GC interval, but not letting a user
> control it.
Nope. You should have looked at the implementation of
sun.misc.GC#requestLatency(long) rather than assuming how it worked.
> It's not tomcat's role to configure GC intervals. It may be that tomcat
> somehow initiated the GC interval, and if that is the case, it must expose
> the actual interval to the user. Tomcat should not change JVM settings
> without letting the user configure them,
Tomcat setting this value has zero impact on any user code or JRE code
that sets a lower value either before Tomcat sets it or after.
I still see no valid use case for this attribute and without a valid use
case my veto remains.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by "Filip Hanik (mailing lists)" <de...@hanik.com>.
> -----Original Message-----
> From: Mark Thomas [mailto:markt@apache.org]
> Sent: Monday, August 27, 2012 3:44 PM
> To: Tomcat Developers List
> Subject: Re: svn commit: r1377689 -
> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
> ner.java
>
> On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
> >> -----Original Message-----
> >> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
> >> Sent: Monday, August 27, 2012 2:09 PM
> >> To: Tomcat Developers List
> >> Subject: Re: svn commit: r1377689 -
> >>
> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
> >> ner.java
> >>
> >> 2012/8/27 Mark Thomas <ma...@apache.org>:
> >>> On 27/08/2012 15:20, fhanik@apache.org wrote:
> >>>> Author: fhanik
> >>>> Date: Mon Aug 27 14:20:55 2012
> >>>> New Revision: 1377689
> >>>>
> >>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
> >>>> Log:
> >>>> Per http://markmail.org/message/nqnogctvfuyzhtol
> >>>>
> >>>> 1. Already encountered two users that would like to set this value.
> >> There is
> >>>> never any need to hard code any value, regardless of its use
> >>>
> >>> What is the use case for wanting to set this value? I can understand
> >>> users not liking the previous value that triggered a full GC every
> >> hour
> >>> and wanting to change that but I fail to see why anyone would want
> to
> >>> change this now it is set to trigger a full GC every 290 million
> years
> >>> or so.
>
> >> Maybe somebody wants their full GC once an hour, or once a day?
>
> That is not what this listener is for. The listener's purpose is to
> prevent memory leaks, not provide options that allow users to tinker
> with internal JVM GC settings.
>
> I have yet to see a valid use case for this new attribute.
[Filip Hanik]
The use case is very much valid, as if they had previously called that
method, your code will override it.
So in effect, you're hard coding the GC interval, but not letting a user
control it.
It's not tomcat's role to configure GC intervals. It may be that tomcat
somehow initiated the GC interval, and if that is the case, it must expose
the actual interval to the user. Tomcat should not change JVM settings
without letting the user configure them,
Filip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Mark Thomas <ma...@apache.org>.
On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
>> -----Original Message-----
>> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
>> Sent: Monday, August 27, 2012 2:09 PM
>> To: Tomcat Developers List
>> Subject: Re: svn commit: r1377689 -
>> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
>> ner.java
>>
>> 2012/8/27 Mark Thomas <ma...@apache.org>:
>>> On 27/08/2012 15:20, fhanik@apache.org wrote:
>>>> Author: fhanik
>>>> Date: Mon Aug 27 14:20:55 2012
>>>> New Revision: 1377689
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
>>>> Log:
>>>> Per http://markmail.org/message/nqnogctvfuyzhtol
>>>>
>>>> 1. Already encountered two users that would like to set this value.
>> There is
>>>> never any need to hard code any value, regardless of its use
>>>
>>> What is the use case for wanting to set this value? I can understand
>>> users not liking the previous value that triggered a full GC every
>> hour
>>> and wanting to change that but I fail to see why anyone would want to
>>> change this now it is set to trigger a full GC every 290 million years
>>> or so.
>> Maybe somebody wants their full GC once an hour, or once a day?
That is not what this listener is for. The listener's purpose is to
prevent memory leaks, not provide options that allow users to tinker
with internal JVM GC settings.
I have yet to see a valid use case for this new attribute.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by "Filip Hanik (mailing lists)" <de...@hanik.com>.
> -----Original Message-----
> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
> Sent: Monday, August 27, 2012 2:09 PM
> To: Tomcat Developers List
> Subject: Re: svn commit: r1377689 -
> /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListe
> ner.java
>
> 2012/8/27 Mark Thomas <ma...@apache.org>:
> > On 27/08/2012 15:20, fhanik@apache.org wrote:
> >> Author: fhanik
> >> Date: Mon Aug 27 14:20:55 2012
> >> New Revision: 1377689
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
> >> Log:
> >> Per http://markmail.org/message/nqnogctvfuyzhtol
> >>
> >> 1. Already encountered two users that would like to set this value.
> There is
> >> never any need to hard code any value, regardless of its use
> >
> > What is the use case for wanting to set this value? I can understand
> > users not liking the previous value that triggered a full GC every
> hour
> > and wanting to change that but I fail to see why anyone would want to
> > change this now it is set to trigger a full GC every 290 million years
> > or so.
> >
> >> 2. This turns it into a property on the listener
> >
> > Thanks. If the feature is retained, that is a much better
> implementation.
> >
>
> Re: 1:
> Maybe somebody wants their full GC once an hour, or once a day?
>
> There are documentation glitches yet to be fixed:
> a. systemprops.xml change in trunk was not reverted by this commit.
> It was reverted in 7.0.x only.
[Filip Hanik]
I don't see the property in trunk, do you?
> b. The new property is yet to be documented in listeners.xml.
[Filip Hanik]
Done
Filip
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/8/27 Mark Thomas <ma...@apache.org>:
> On 27/08/2012 15:20, fhanik@apache.org wrote:
>> Author: fhanik
>> Date: Mon Aug 27 14:20:55 2012
>> New Revision: 1377689
>>
>> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
>> Log:
>> Per http://markmail.org/message/nqnogctvfuyzhtol
>>
>> 1. Already encountered two users that would like to set this value. There is
>> never any need to hard code any value, regardless of its use
>
> What is the use case for wanting to set this value? I can understand
> users not liking the previous value that triggered a full GC every hour
> and wanting to change that but I fail to see why anyone would want to
> change this now it is set to trigger a full GC every 290 million years
> or so.
>
>> 2. This turns it into a property on the listener
>
> Thanks. If the feature is retained, that is a much better implementation.
>
Re: 1:
Maybe somebody wants their full GC once an hour, or once a day?
There are documentation glitches yet to be fixed:
a. systemprops.xml change in trunk was not reverted by this commit.
It was reverted in 7.0.x only.
b. The new property is yet to be documented in listeners.xml.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
Posted by Mark Thomas <ma...@apache.org>.
On 27/08/2012 15:20, fhanik@apache.org wrote:
> Author: fhanik
> Date: Mon Aug 27 14:20:55 2012
> New Revision: 1377689
>
> URL: http://svn.apache.org/viewvc?rev=1377689&view=rev
> Log:
> Per http://markmail.org/message/nqnogctvfuyzhtol
>
> 1. Already encountered two users that would like to set this value. There is
> never any need to hard code any value, regardless of its use
What is the use case for wanting to set this value? I can understand
users not liking the previous value that triggered a full GC every hour
and wanting to change that but I fail to see why anyone would want to
change this now it is set to trigger a full GC every 290 million years
or so.
> 2. This turns it into a property on the listener
Thanks. If the feature is retained, that is a much better implementation.
Mark
>
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java?rev=1377689&r1=1377688&r2=1377689&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java Mon Aug 27 14:20:55 2012
> @@ -218,6 +218,17 @@ public class JreMemoryLeakPreventionList
> this.classesToInitialize = classesToInitialize;
> }
>
> + /**
> + * Sets the time that this listener will request for garbage-collection latency
> + * @see {@link sun.misc.GC#requestLatency(long)}
> + */
> + private long gcDaemonPeriod = Long.MAX_VALUE - 1;
> + public long getGcDaemonPeriod() {
> + return gcDaemonPeriod;
> + }
> + public void setGcDaemonPeriod(long gcDaemonPeriod) {
> + this.gcDaemonPeriod = gcDaemonPeriod;
> + }
>
> @Override
> public void lifecycleEvent(LifecycleEvent event) {
> @@ -297,7 +308,7 @@ public class JreMemoryLeakPreventionList
> Method method = clazz.getDeclaredMethod(
> "requestLatency",
> new Class[] {long.class});
> - method.invoke(null, Long.getLong("org.apache.catalina.core.jreMemoryLeakPreventionGCDaemonPeriod", Long.valueOf(Long.MAX_VALUE-1)));
> + method.invoke(null, getGcDaemonPeriod());
> } catch (ClassNotFoundException e) {
> if (System.getProperty("java.vendor").startsWith(
> "Sun")) {
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org