You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by sebb <se...@gmail.com> on 2012/08/20 18:13:21 UTC
Re: svn commit: r1374946 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml
On 20 August 2012 09:49, <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Mon Aug 20 08:49:59 2012
> New Revision: 1374946
>
> URL: http://svn.apache.org/viewvc?rev=1374946&view=rev
> Log:
> Bug 53743 - JavaSamplers.allSamplers static Set keeps references even after thread has ended
> Bugzilla Id: 53743
-1, please revert.
This does solve the memory issue, but it breaks the contract for
.JavaSamplerClient#teardownTest(JavaSamplerContext)
which says:
"Do any clean-up required by this test at the end of a test run."
teardownTest is now called at end of thread, not end of test.
This may break some implementations.
> Modified:
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> jmeter/trunk/xdocs/changes.xml
>
> Modified: jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java?rev=1374946&r1=1374945&r2=1374946&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java (original)
> +++ jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java Mon Aug 20 08:49:59 2012
> @@ -20,7 +20,6 @@ package org.apache.jmeter.protocol.java.
>
> import java.util.Arrays;
> import java.util.HashSet;
> -import java.util.Iterator;
> import java.util.Set;
>
> import org.apache.jmeter.config.Arguments;
> @@ -31,6 +30,7 @@ import org.apache.jmeter.samplers.Entry;
> import org.apache.jmeter.samplers.SampleResult;
> import org.apache.jmeter.testelement.TestElement;
> import org.apache.jmeter.testelement.TestListener;
> +import org.apache.jmeter.testelement.ThreadListener;
> import org.apache.jmeter.testelement.property.TestElementProperty;
> import org.apache.jorphan.logging.LoggingManager;
> import org.apache.log.Logger;
> @@ -41,7 +41,7 @@ import org.apache.log.Logger;
> * information on writing Java code to be executed by this sampler.
> *
> */
> -public class JavaSampler extends AbstractSampler implements TestListener {
> +public class JavaSampler extends AbstractSampler implements TestListener, ThreadListener {
>
> private static final Logger log = LoggingManager.getLoggerForClass();
>
> @@ -76,19 +76,10 @@ public class JavaSampler extends Abstrac
> private transient JavaSamplerContext context = null;
>
> /**
> - * Set used to register all active JavaSamplers. This is used so that the
> - * samplers can be notified when the test ends.
> - */
> - private static final Set<JavaSampler> allSamplers = new HashSet<JavaSampler>();
> -
> - /**
> * Create a JavaSampler.
> */
> public JavaSampler() {
> setArguments(new Arguments());
> - synchronized (allSamplers) {
> - allSamplers.add(this);
> - }
> }
>
> /**
> @@ -240,23 +231,9 @@ public class JavaSampler extends Abstrac
> log.debug(whoAmI() + "\ttestStarted(" + host + ")");
> }
>
> - /**
> - * Method called at the end of the test. This is called only on one instance
> - * of JavaSampler. This method will loop through all of the other
> - * JavaSamplers which have been registered (automatically in the
> - * constructor) and notify them that the test has ended, allowing the
> - * JavaSamplerClients to cleanup.
> - */
> + /* Implements TestListener.testEnded() */
> public void testEnded() {
> log.debug(whoAmI() + "\ttestEnded");
> - synchronized (allSamplers) {
> - Iterator<JavaSampler> i = allSamplers.iterator();
> - while (i.hasNext()) {
> - JavaSampler sampler = i.next();
> - sampler.releaseJavaClient();
> - i.remove();
> - }
> - }
> }
>
> /* Implements TestListener.testEnded(String) */
> @@ -300,4 +277,15 @@ public class JavaSampler extends Abstrac
> String guiClass = configElement.getProperty(TestElement.GUI_CLASS).getStringValue();
> return APPLIABLE_CONFIG_CLASSES.contains(guiClass);
> }
> +
> + public void threadStarted() {
> + // NOOP
> + }
> +
> + /**
> + * Cleanup java client
> + */
> + public void threadFinished() {
> + releaseJavaClient();
> + }
> }
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1374946&r1=1374945&r2=1374946&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Mon Aug 20 08:49:59 2012
> @@ -84,6 +84,7 @@ JSR223 Test Elements using Script file a
> <li><bugzilla>53357</bugzilla> - JMS Point to Point reports too high response times in Request Response Mode</li>
> <li><bugzilla>53440</bugzilla> - SSL connection leads to ArrayStoreException on JDK 6 with some KeyManagerFactory SPI</li>
> <li><bugzilla>53511</bugzilla> - access log sampler SessionFilter throws NullPointerException - cookie manager not initialized properly</li>
> +<li><bugzilla>53743</bugzilla> - JavaSamplers.allSamplers static Set keeps references even after thread has ended</li>
> </ul>
>
> <h3>Controllers</h3>
>
>
Re: svn commit: r1374946 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml
Posted by Philippe Mouawad <ph...@gmail.com>.
Then what if we implement testEnded the same way.
Looking at previous implementation I don't see why it was done this way.
In my opinion implemented as I did is the best for performances as it
cleaned up as soon as it becomes useless, I agree it breaks contract but we
could introduce a new parameter :
- java.samplerclient.teardown_at_thread_end=true => Would clean at end
of thread
- java.samplerclient.teardown_at_thread_end=false => Would clean at end
of test which would work as today but we would change impl to remove static
collection and just call releaseJavaClient()
What do you think ?
Regards
Philippe
On Mon, Aug 20, 2012 at 6:13 PM, sebb <se...@gmail.com> wrote:
> On 20 August 2012 09:49, <pm...@apache.org> wrote:
> > Author: pmouawad
> > Date: Mon Aug 20 08:49:59 2012
> > New Revision: 1374946
> >
> > URL: http://svn.apache.org/viewvc?rev=1374946&view=rev
> > Log:
> > Bug 53743 - JavaSamplers.allSamplers static Set keeps references even
> after thread has ended
> > Bugzilla Id: 53743
>
> -1, please revert.
>
> This does solve the memory issue, but it breaks the contract for
>
> .JavaSamplerClient#teardownTest(JavaSamplerContext)
>
> which says:
>
> "Do any clean-up required by this test at the end of a test run."
>
> teardownTest is now called at end of thread, not end of test.
>
> This may break some implementations.
>
> > Modified:
> >
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> > jmeter/trunk/xdocs/changes.xml
> >
> > Modified:
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java?rev=1374946&r1=1374945&r2=1374946&view=diff
> >
> ==============================================================================
> > ---
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> (original)
> > +++
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> Mon Aug 20 08:49:59 2012
> > @@ -20,7 +20,6 @@ package org.apache.jmeter.protocol.java.
> >
> > import java.util.Arrays;
> > import java.util.HashSet;
> > -import java.util.Iterator;
> > import java.util.Set;
> >
> > import org.apache.jmeter.config.Arguments;
> > @@ -31,6 +30,7 @@ import org.apache.jmeter.samplers.Entry;
> > import org.apache.jmeter.samplers.SampleResult;
> > import org.apache.jmeter.testelement.TestElement;
> > import org.apache.jmeter.testelement.TestListener;
> > +import org.apache.jmeter.testelement.ThreadListener;
> > import org.apache.jmeter.testelement.property.TestElementProperty;
> > import org.apache.jorphan.logging.LoggingManager;
> > import org.apache.log.Logger;
> > @@ -41,7 +41,7 @@ import org.apache.log.Logger;
> > * information on writing Java code to be executed by this sampler.
> > *
> > */
> > -public class JavaSampler extends AbstractSampler implements
> TestListener {
> > +public class JavaSampler extends AbstractSampler implements
> TestListener, ThreadListener {
> >
> > private static final Logger log =
> LoggingManager.getLoggerForClass();
> >
> > @@ -76,19 +76,10 @@ public class JavaSampler extends Abstrac
> > private transient JavaSamplerContext context = null;
> >
> > /**
> > - * Set used to register all active JavaSamplers. This is used so
> that the
> > - * samplers can be notified when the test ends.
> > - */
> > - private static final Set<JavaSampler> allSamplers = new
> HashSet<JavaSampler>();
> > -
> > - /**
> > * Create a JavaSampler.
> > */
> > public JavaSampler() {
> > setArguments(new Arguments());
> > - synchronized (allSamplers) {
> > - allSamplers.add(this);
> > - }
> > }
> >
> > /**
> > @@ -240,23 +231,9 @@ public class JavaSampler extends Abstrac
> > log.debug(whoAmI() + "\ttestStarted(" + host + ")");
> > }
> >
> > - /**
> > - * Method called at the end of the test. This is called only on one
> instance
> > - * of JavaSampler. This method will loop through all of the other
> > - * JavaSamplers which have been registered (automatically in the
> > - * constructor) and notify them that the test has ended, allowing
> the
> > - * JavaSamplerClients to cleanup.
> > - */
> > + /* Implements TestListener.testEnded() */
> > public void testEnded() {
> > log.debug(whoAmI() + "\ttestEnded");
> > - synchronized (allSamplers) {
> > - Iterator<JavaSampler> i = allSamplers.iterator();
> > - while (i.hasNext()) {
> > - JavaSampler sampler = i.next();
> > - sampler.releaseJavaClient();
> > - i.remove();
> > - }
> > - }
> > }
> >
> > /* Implements TestListener.testEnded(String) */
> > @@ -300,4 +277,15 @@ public class JavaSampler extends Abstrac
> > String guiClass =
> configElement.getProperty(TestElement.GUI_CLASS).getStringValue();
> > return APPLIABLE_CONFIG_CLASSES.contains(guiClass);
> > }
> > +
> > + public void threadStarted() {
> > + // NOOP
> > + }
> > +
> > + /**
> > + * Cleanup java client
> > + */
> > + public void threadFinished() {
> > + releaseJavaClient();
> > + }
> > }
> >
> > Modified: jmeter/trunk/xdocs/changes.xml
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1374946&r1=1374945&r2=1374946&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/xdocs/changes.xml (original)
> > +++ jmeter/trunk/xdocs/changes.xml Mon Aug 20 08:49:59 2012
> > @@ -84,6 +84,7 @@ JSR223 Test Elements using Script file a
> > <li><bugzilla>53357</bugzilla> - JMS Point to Point reports too high
> response times in Request Response Mode</li>
> > <li><bugzilla>53440</bugzilla> - SSL connection leads to
> ArrayStoreException on JDK 6 with some KeyManagerFactory SPI</li>
> > <li><bugzilla>53511</bugzilla> - access log sampler SessionFilter
> throws NullPointerException - cookie manager not initialized properly</li>
> > +<li><bugzilla>53743</bugzilla> - JavaSamplers.allSamplers static Set
> keeps references even after thread has ended</li>
> > </ul>
> >
> > <h3>Controllers</h3>
> >
> >
>
--
Cordialement.
Philippe Mouawad.