Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-28 Thread Rainer Jung 

On 28.08.2012 01:16, Filip Hanik (mailing lists) wrote:




-Original Message-
From: Mark Thomas [mailto:ma...@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:ma...@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.koli...@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, fha...@apache.org wrote:

Author: fhanik
Date: Mon Aug 27 14:20:55 2012
New Revision: 1377689

URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Mark Thomas 
On 27/08/2012 15:20, fha...@apache.org wrote:
 Author: fhanik
 Date: Mon Aug 27 14:20:55 2012
 New Revision: 1377689
 
 URL: http://svn.apache.org/viewvc?rev=1377689view=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=1377689r1=1377688r2=1377689view=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-unsubscr...@tomcat.apache.org
 For additional commands, e-mail: dev-h...@tomcat.apache.org
 


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Konstantin Kolinko 
2012/8/27 Mark Thomas ma...@apache.org:
 On 27/08/2012 15:20, fha...@apache.org wrote:
 Author: fhanik
 Date: Mon Aug 27 14:20:55 2012
 New Revision: 1377689

 URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Filip Hanik (mailing lists) 


 -Original Message-
 From: Konstantin Kolinko [mailto:knst.koli...@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, fha...@apache.org wrote:
  Author: fhanik
  Date: Mon Aug 27 14:20:55 2012
  New Revision: 1377689
 
  URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
 For additional commands, e-mail: dev-h...@tomcat.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Konstantin Kolinko 
2012/8/28 Filip Hanik (mailing lists) devli...@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=1377831view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Mark Thomas 
On 27/08/2012 22:37, Filip Hanik (mailing lists) wrote:
 -Original Message-
 From: Konstantin Kolinko [mailto:knst.koli...@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, fha...@apache.org wrote:
 Author: fhanik
 Date: Mon Aug 27 14:20:55 2012
 New Revision: 1377689

 URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Filip Hanik (mailing lists) 


 -Original Message-
 From: Konstantin Kolinko [mailto:knst.koli...@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) devli...@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=1377831view=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-unsubscr...@tomcat.apache.org
 For additional commands, e-mail: dev-h...@tomcat.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Filip Hanik (mailing lists) 


 -Original Message-
 From: Mark Thomas [mailto:ma...@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.koli...@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, fha...@apache.org wrote:
  Author: fhanik
  Date: Mon Aug 27 14:20:55 2012
  New Revision: 1377689
 
  URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Mark Thomas 
On 27/08/2012 22:48, Filip Hanik (mailing lists) wrote:
 
 
 -Original Message-
 From: Mark Thomas [mailto:ma...@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.koli...@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, fha...@apache.org wrote:
 Author: fhanik
 Date: Mon Aug 27 14:20:55 2012
 New Revision: 1377689

 URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Konstantin Kolinko 
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:ma...@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.koli...@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, fha...@apache.org wrote:
 Author: fhanik
 Date: Mon Aug 27 14:20:55 2012
 New Revision: 1377689

 URL: http://svn.apache.org/viewvc?rev=1377689view=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=1377831view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Mark Thomas 
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:ma...@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.koli...@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, fha...@apache.org wrote:
 Author: fhanik
 Date: Mon Aug 27 14:20:55 2012
 New Revision: 1377689

 URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

RE: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Filip Hanik (mailing lists) 


 -Original Message-
 From: Mark Thomas [mailto:ma...@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:ma...@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.koli...@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, fha...@apache.org wrote:
  Author: fhanik
  Date: Mon Aug 27 14:20:55 2012
  New Revision: 1377689
 
  URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
 For additional commands, e-mail: dev-h...@tomcat.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1377689 - /tomcat/trunk/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java

2012-08-27 Thread Mark Thomas 
On 28/08/2012 00:16, Filip Hanik (mailing lists) wrote:
 -Original Message-
 From: Mark Thomas [mailto:ma...@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:ma...@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.koli...@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, fha...@apache.org wrote:
 Author: fhanik
 Date: Mon Aug 27 14:20:55 2012
 New Revision: 1377689

 URL: http://svn.apache.org/viewvc?rev=1377689view=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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org