You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2012/08/25 15:17:20 UTC
svn commit: r1377291 - in /jmeter/trunk:
src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Author: pmouawad
Date: Sat Aug 25 13:17:19 2012
New Revision: 1377291
URL: http://svn.apache.org/viewvc?rev=1377291&view=rev
Log:
Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup to use less memory
Only register instance of JavaSamplerClient that have overriden or implemented teardownTest
Bugzilla Id: 53782
Modified:
jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
jmeter/trunk/xdocs/changes.xml
jmeter/trunk/xdocs/usermanual/component_reference.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=1377291&r1=1377290&r2=1377291&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 Sat Aug 25 13:17:19 2012
@@ -18,10 +18,14 @@
package org.apache.jmeter.protocol.java.sampler;
+import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.jmeter.config.Arguments;
import org.apache.jmeter.config.ConfigTestElement;
import org.apache.jmeter.samplers.AbstractSampler;
@@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
private transient JavaSamplerContext context = null;
/**
+ * Cache of classname, boolean that holds information about a class and wether or not it should
+ * be registered for cleanup.
+ * This is done to avoid using reflection on each registration
+ */
+ private transient Map<String, Boolean> isToBeRegisteredCache = new ConcurrentHashMap<String, Boolean>();
+
+ /**
* Set used to register all JavaSamplerClient and JavaSamplerContext.
* This is used so that the JavaSamplerClient can be notified when the test ends.
*/
@@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
* @param entry
* the Entry for this sample
* @return test SampleResult
+ * @throws NoSuchMethodException
+ * @throws SecurityException
*/
- public SampleResult sample(Entry entry) {
- Arguments args = getArguments();
- args.addArgument(TestElement.NAME, getName()); // Allow Sampler access
- // to test element name
- context = new JavaSamplerContext(args);
- if (javaClient == null) {
- log.debug(whoAmI() + "\tCreating Java Client");
- createJavaClient();
- javaClientAndContextSet.add(new Object[]{javaClient, context});
- javaClient.setupTest(context);
+ public SampleResult sample(Entry entry) {
+ try {
+ Arguments args = getArguments();
+ args.addArgument(TestElement.NAME, getName()); // Allow Sampler access
+ // to test element name
+ context = new JavaSamplerContext(args);
+ if (javaClient == null) {
+ log.debug(whoAmI() + "\tCreating Java Client");
+ createJavaClient();
+ registerForCleanup(javaClient, context);
+ javaClient.setupTest(context);
+ }
+
+ SampleResult result = javaClient.runTest(context);
+
+ // Only set the default label if it has not been set
+ if (result != null && result.getSampleLabel().length() == 0) {
+ result.setSampleLabel(getName());
+ }
+
+ return result;
+ } catch(Exception ex) {
+ SampleResult sampleResultIfError = new SampleResult();
+ sampleResultIfError.setSampleLabel(getName());
+ sampleResultIfError.setSuccessful(false);
+ sampleResultIfError.setResponseCode("500"); // $NON-NLS-1$
+ sampleResultIfError.setResponseMessage(ExceptionUtils.getRootCauseMessage(ex));
+ sampleResultIfError.setResponseData(ExceptionUtils.getStackTrace(ex), "UTF-8");
+ return sampleResultIfError;
}
+ }
- SampleResult result = javaClient.runTest(context);
-
- // Only set the default label if it has not been set
- if (result != null && result.getSampleLabel().length() == 0) {
- result.setSampleLabel(getName());
+ /**
+ * Only register jsClient if it contains a custom teardownTest method
+ * @param jsClient JavaSamplerClient
+ * @param jsContext JavaSamplerContext
+ * @throws NoSuchMethodException
+ * @throws SecurityException
+ */
+ private final void registerForCleanup(JavaSamplerClient jsClient,
+ JavaSamplerContext jsContext) throws SecurityException, NoSuchMethodException {
+ if(isToBeRegistered(jsClient.getClass())) {
+ javaClientAndContextSet.add(new Object[]{jsClient, jsContext});
}
+ }
- return result;
+ /**
+ * Tests clazz to see if a custom teardown method has been written and caches the test result.
+ * If classes uses {@link AbstractJavaSamplerClient#teardownTest(JavaSamplerContext)} then it won't
+ * be registered for cleanup as it does nothing.
+ * @param clazz Class to be verified
+ * @return true if clazz should be registered for cleanup
+ * @throws SecurityException
+ * @throws NoSuchMethodException
+ */
+ private boolean isToBeRegistered(Class<? extends JavaSamplerClient> clazz) throws SecurityException, NoSuchMethodException {
+ Boolean isToBeRegistered = isToBeRegisteredCache.get(clazz.getName());
+ if(isToBeRegistered == null) {
+ Method method = clazz.getMethod("teardownTest", new Class[]{JavaSamplerContext.class});
+ isToBeRegistered = Boolean.valueOf(!method.getDeclaringClass().equals(AbstractJavaSamplerClient.class));
+ isToBeRegisteredCache.put(clazz.getName(), isToBeRegistered);
+ }
+ return isToBeRegistered.booleanValue();
}
/**
@@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
}
javaClientAndContextSet.clear();
}
+ isToBeRegisteredCache.clear();
}
/* Implements TestStateListener.testEnded(String) */
Modified: jmeter/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
==============================================================================
--- jmeter/trunk/xdocs/changes.xml (original)
+++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
@@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
<ul>
<li><bugzilla>55310</bugzilla> - TestAction should implement Interruptible</li>
<li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP Request Defaults Control </li>
+<li><bugzilla>53782</bugzilla> - Enhance JavaSampler handling of JavaSamplerClient cleanup to use less memory</li>
</ul>
<h3>Controllers</h3>
Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml
URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
==============================================================================
--- jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
+++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sat Aug 25 13:17:19 2012
@@ -566,6 +566,9 @@ The fields allow variables to be used, s
</p>
</description>
+<note>Since JMeter 2.8, if method teardownTest is not overriden by subclasses of AbstractJavaSamplerClient, then the method will not be called to optimise JMeter memory behaviour.
+This will not have any impact on existing Test plans.
+</note>
<note>The Add/Delete buttons don't serve any purpose at present.</note>
<properties>
Re: svn commit: r1377291 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Posted by Philippe Mouawad <ph...@gmail.com>.
Thanks Milamber.
Fixed.
Regards
Philippe
On Sun, Aug 26, 2012 at 6:17 PM, Milamber <mi...@apache.org> wrote:
>
>
> Le 25/08/2012 14:17, pmouawad@apache.org a ecrit :
>
> Author: pmouawad
>> Date: Sat Aug 25 13:17:19 2012
>> New Revision: 1377291
>>
>> URL: http://svn.apache.org/viewvc?**rev=1377291&view=rev<http://svn.apache.org/viewvc?rev=1377291&view=rev>
>> Log:
>> Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup to
>> use less memory
>> Only register instance of JavaSamplerClient that have overriden or
>> implemented teardownTest
>> Bugzilla Id: 53782
>>
>> Modified:
>> jmeter/trunk/src/protocol/**java/org/apache/jmeter/**
>> protocol/java/sampler/**JavaSampler.java
>>
>
> Seems introduce regression in junit tests.
>
>
> [client] ... end of run
> [echo] BatchTestLocal output files compared OK
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**4430d82d java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**30384065 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**4cf7c31d java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**1e2acc65 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**2c79809 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**3794d372 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**bc5fde0 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**56c163f java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**2bb5340c java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**212bcd4b java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**60de1b8a java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**15e232b5 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**7dc05ffd java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**7f92c8d9 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**72e6f7d2 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**303bc257 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**2ec791b9 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**538f1d7e java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**2353f67e java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**40589e56 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**5fe0f2f6 java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
> [concat] 2012/08/26 16:15:48 WARN - jmeter.engine.**StandardJMeterEngine:
> Error encountered during shutdown of org.apache.jmeter.protocol.**
> java.sampler.JavaSampler@**5d3ad33d java.lang.NullPointerException
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**303)
> [concat] at org.apache.jmeter.protocol.**java.sampler.JavaSampler.*
> *testEnded(JavaSampler.java:**308)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.**
> notifyTestListenersOfEnd(**StandardJMeterEngine.java:220)
> [concat] at org.apache.jmeter.engine.**StandardJMeterEngine.run(**
> StandardJMeterEngine.java:423)
> [concat] at java.lang.Thread.run(Thread.**java:595)
>
> BUILD FAILED
>
>
>
>
>
> jmeter/trunk/xdocs/changes.xml
>> jmeter/trunk/xdocs/usermanual/**component_reference.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=1377291&**r1=1377290&r2=1377291&view=**diff<http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java?rev=1377291&r1=1377290&r2=1377291&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 Sat Aug 25 13:17:19 2012
>> @@ -18,10 +18,14 @@
>>
>> package org.apache.jmeter.protocol.**java.sampler;
>>
>> +import java.lang.reflect.Method;
>> import java.util.Arrays;
>> import java.util.HashSet;
>> +import java.util.Map;
>> import java.util.Set;
>> +import java.util.concurrent.**ConcurrentHashMap;
>>
>> +import org.apache.commons.lang3.**exception.ExceptionUtils;
>> import org.apache.jmeter.config.**Arguments;
>> import org.apache.jmeter.config.**ConfigTestElement;
>> import org.apache.jmeter.samplers.**AbstractSampler;
>> @@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
>> private transient JavaSamplerContext context = null;
>>
>> /**
>> + * Cache of classname, boolean that holds information about a class
>> and wether or not it should
>> + * be registered for cleanup.
>> + * This is done to avoid using reflection on each registration
>> + */
>> + private transient Map<String, Boolean> isToBeRegisteredCache = new
>> ConcurrentHashMap<String, Boolean>();
>> +
>> + /**
>> * Set used to register all JavaSamplerClient and
>> JavaSamplerContext.
>> * This is used so that the JavaSamplerClient can be notified when
>> the test ends.
>> */
>> @@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
>> * @param entry
>> * the Entry for this sample
>> * @return test SampleResult
>> + * @throws NoSuchMethodException
>> + * @throws SecurityException
>> */
>> - public SampleResult sample(Entry entry) {
>> - Arguments args = getArguments();
>> - args.addArgument(TestElement.**NAME, getName()); // Allow
>> Sampler access
>> - // to test
>> element name
>> - context = new JavaSamplerContext(args);
>> - if (javaClient == null) {
>> - log.debug(whoAmI() + "\tCreating Java Client");
>> - createJavaClient();
>> - javaClientAndContextSet.add(**new Object[]{javaClient,
>> context});
>> - javaClient.setupTest(context);
>> + public SampleResult sample(Entry entry) {
>> + try {
>> + Arguments args = getArguments();
>> + args.addArgument(TestElement.**NAME, getName()); // Allow
>> Sampler access
>> + // to test
>> element name
>> + context = new JavaSamplerContext(args);
>> + if (javaClient == null) {
>> + log.debug(whoAmI() + "\tCreating Java Client");
>> + createJavaClient();
>> + registerForCleanup(javaClient, context);
>> + javaClient.setupTest(context);
>> + }
>> +
>> + SampleResult result = javaClient.runTest(context);
>> +
>> + // Only set the default label if it has not been set
>> + if (result != null&& result.getSampleLabel().**length() ==
>> 0) {
>>
>> + result.setSampleLabel(getName(**));
>> + }
>> +
>> + return result;
>> + } catch(Exception ex) {
>> + SampleResult sampleResultIfError = new SampleResult();
>> + sampleResultIfError.**setSampleLabel(getName());
>> + sampleResultIfError.**setSuccessful(false);
>> + sampleResultIfError.**setResponseCode("500"); // $NON-NLS-1$
>> + sampleResultIfError.**setResponseMessage(**ExceptionUtils.**
>> getRootCauseMessage(ex));
>> + sampleResultIfError.**setResponseData(**
>> ExceptionUtils.getStackTrace(**ex), "UTF-8");
>> + return sampleResultIfError;
>> }
>> + }
>>
>> - SampleResult result = javaClient.runTest(context);
>> -
>> - // Only set the default label if it has not been set
>> - if (result != null&& result.getSampleLabel().**length() == 0) {
>>
>> - result.setSampleLabel(getName(**));
>> + /**
>> + * Only register jsClient if it contains a custom teardownTest method
>> + * @param jsClient JavaSamplerClient
>> + * @param jsContext JavaSamplerContext
>> + * @throws NoSuchMethodException
>> + * @throws SecurityException
>> + */
>> + private final void registerForCleanup(**JavaSamplerClient jsClient,
>> + JavaSamplerContext jsContext) throws SecurityException,
>> NoSuchMethodException {
>> + if(isToBeRegistered(jsClient.**getClass())) {
>> + javaClientAndContextSet.add(**new Object[]{jsClient,
>> jsContext});
>> }
>> + }
>>
>> - return result;
>> + /**
>> + * Tests clazz to see if a custom teardown method has been written
>> and caches the test result.
>> + * If classes uses {@link AbstractJavaSamplerClient#**teardownTest(*
>> *JavaSamplerContext)} then it won't
>> + * be registered for cleanup as it does nothing.
>> + * @param clazz Class to be verified
>> + * @return true if clazz should be registered for cleanup
>> + * @throws SecurityException
>> + * @throws NoSuchMethodException
>> + */
>> + private boolean isToBeRegistered(Class<? extends JavaSamplerClient>
>> clazz) throws SecurityException, NoSuchMethodException {
>> + Boolean isToBeRegistered = isToBeRegisteredCache.get(**
>> clazz.getName());
>> + if(isToBeRegistered == null) {
>> + Method method = clazz.getMethod("teardownTest"**, new
>> Class[]{JavaSamplerContext.**class});
>> + isToBeRegistered = Boolean.valueOf(!method.**
>> getDeclaringClass().equals(**AbstractJavaSamplerClient.**class));
>> + isToBeRegisteredCache.put(**clazz.getName(),
>> isToBeRegistered);
>> + }
>> + return isToBeRegistered.booleanValue(**);
>> }
>>
>> /**
>> @@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
>> }
>> javaClientAndContextSet.clear(**);
>> }
>> + isToBeRegisteredCache.clear();
>> }
>>
>> /* Implements TestStateListener.testEnded(**String) */
>>
>> Modified: jmeter/trunk/xdocs/changes.xml
>> URL: http://svn.apache.org/viewvc/**jmeter/trunk/xdocs/changes.**
>> xml?rev=1377291&r1=1377290&r2=**1377291&view=diff<http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff>
>> ==============================**==============================**
>> ==================
>> --- jmeter/trunk/xdocs/changes.xml (original)
>> +++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
>> @@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
>> <ul>
>> <li><bugzilla>55310</bugzilla> - TestAction should implement
>> Interruptible</li>
>> <li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP
>> Request Defaults Control</li>
>> +<li><bugzilla>53782</**bugzilla> - Enhance JavaSampler handling of
>> JavaSamplerClient cleanup to use less memory</li>
>> </ul>
>>
>> <h3>Controllers</h3>
>>
>> Modified: jmeter/trunk/xdocs/usermanual/**component_reference.xml
>> URL: http://svn.apache.org/viewvc/**jmeter/trunk/xdocs/usermanual/**
>> component_reference.xml?rev=**1377291&r1=1377290&r2=1377291&**view=diff<http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff>
>> ==============================**==============================**
>> ==================
>> --- jmeter/trunk/xdocs/usermanual/**component_reference.xml (original)
>> +++ jmeter/trunk/xdocs/usermanual/**component_reference.xml Sat Aug 25
>> 13:17:19 2012
>> @@ -566,6 +566,9 @@ The fields allow variables to be used, s
>> </p>
>> </description>
>>
>> +<note>Since JMeter 2.8, if method teardownTest is not overriden by
>> subclasses of AbstractJavaSamplerClient, then the method will not be called
>> to optimise JMeter memory behaviour.
>> +This will not have any impact on existing Test plans.
>> +</note>
>> <note>The Add/Delete buttons don't serve any purpose at present.</note>
>>
>> <properties>
>>
>>
>>
>>
>
--
Cordialement.
Philippe Mouawad.
Re: svn commit: r1377291 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Posted by Milamber <mi...@apache.org>.
Le 25/08/2012 14:17, pmouawad@apache.org a ecrit :
> Author: pmouawad
> Date: Sat Aug 25 13:17:19 2012
> New Revision: 1377291
>
> URL: http://svn.apache.org/viewvc?rev=1377291&view=rev
> Log:
> Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup to use less memory
> Only register instance of JavaSamplerClient that have overriden or implemented teardownTest
> Bugzilla Id: 53782
>
> Modified:
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
Seems introduce regression in junit tests.
[client] ... end of run
[echo] BatchTestLocal output files compared OK
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@4430d82d
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@30384065
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@4cf7c31d
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@1e2acc65
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@2c79809
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@3794d372
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@bc5fde0
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@56c163f
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@2bb5340c
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@212bcd4b
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@60de1b8a
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@15e232b5
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@7dc05ffd
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@7f92c8d9
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@72e6f7d2
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@303bc257
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@2ec791b9
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@538f1d7e
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@2353f67e
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@40589e56
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@5fe0f2f6
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
[concat] 2012/08/26 16:15:48 WARN -
jmeter.engine.StandardJMeterEngine: Error encountered during shutdown of
org.apache.jmeter.protocol.java.sampler.JavaSampler@5d3ad33d
java.lang.NullPointerException
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:303)
[concat] at
org.apache.jmeter.protocol.java.sampler.JavaSampler.testEnded(JavaSampler.java:308)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfEnd(StandardJMeterEngine.java:220)
[concat] at
org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:423)
[concat] at java.lang.Thread.run(Thread.java:595)
BUILD FAILED
> jmeter/trunk/xdocs/changes.xml
> jmeter/trunk/xdocs/usermanual/component_reference.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=1377291&r1=1377290&r2=1377291&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 Sat Aug 25 13:17:19 2012
> @@ -18,10 +18,14 @@
>
> package org.apache.jmeter.protocol.java.sampler;
>
> +import java.lang.reflect.Method;
> import java.util.Arrays;
> import java.util.HashSet;
> +import java.util.Map;
> import java.util.Set;
> +import java.util.concurrent.ConcurrentHashMap;
>
> +import org.apache.commons.lang3.exception.ExceptionUtils;
> import org.apache.jmeter.config.Arguments;
> import org.apache.jmeter.config.ConfigTestElement;
> import org.apache.jmeter.samplers.AbstractSampler;
> @@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
> private transient JavaSamplerContext context = null;
>
> /**
> + * Cache of classname, boolean that holds information about a class and wether or not it should
> + * be registered for cleanup.
> + * This is done to avoid using reflection on each registration
> + */
> + private transient Map<String, Boolean> isToBeRegisteredCache = new ConcurrentHashMap<String, Boolean>();
> +
> + /**
> * Set used to register all JavaSamplerClient and JavaSamplerContext.
> * This is used so that the JavaSamplerClient can be notified when the test ends.
> */
> @@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
> * @param entry
> * the Entry for this sample
> * @return test SampleResult
> + * @throws NoSuchMethodException
> + * @throws SecurityException
> */
> - public SampleResult sample(Entry entry) {
> - Arguments args = getArguments();
> - args.addArgument(TestElement.NAME, getName()); // Allow Sampler access
> - // to test element name
> - context = new JavaSamplerContext(args);
> - if (javaClient == null) {
> - log.debug(whoAmI() + "\tCreating Java Client");
> - createJavaClient();
> - javaClientAndContextSet.add(new Object[]{javaClient, context});
> - javaClient.setupTest(context);
> + public SampleResult sample(Entry entry) {
> + try {
> + Arguments args = getArguments();
> + args.addArgument(TestElement.NAME, getName()); // Allow Sampler access
> + // to test element name
> + context = new JavaSamplerContext(args);
> + if (javaClient == null) {
> + log.debug(whoAmI() + "\tCreating Java Client");
> + createJavaClient();
> + registerForCleanup(javaClient, context);
> + javaClient.setupTest(context);
> + }
> +
> + SampleResult result = javaClient.runTest(context);
> +
> + // Only set the default label if it has not been set
> + if (result != null&& result.getSampleLabel().length() == 0) {
> + result.setSampleLabel(getName());
> + }
> +
> + return result;
> + } catch(Exception ex) {
> + SampleResult sampleResultIfError = new SampleResult();
> + sampleResultIfError.setSampleLabel(getName());
> + sampleResultIfError.setSuccessful(false);
> + sampleResultIfError.setResponseCode("500"); // $NON-NLS-1$
> + sampleResultIfError.setResponseMessage(ExceptionUtils.getRootCauseMessage(ex));
> + sampleResultIfError.setResponseData(ExceptionUtils.getStackTrace(ex), "UTF-8");
> + return sampleResultIfError;
> }
> + }
>
> - SampleResult result = javaClient.runTest(context);
> -
> - // Only set the default label if it has not been set
> - if (result != null&& result.getSampleLabel().length() == 0) {
> - result.setSampleLabel(getName());
> + /**
> + * Only register jsClient if it contains a custom teardownTest method
> + * @param jsClient JavaSamplerClient
> + * @param jsContext JavaSamplerContext
> + * @throws NoSuchMethodException
> + * @throws SecurityException
> + */
> + private final void registerForCleanup(JavaSamplerClient jsClient,
> + JavaSamplerContext jsContext) throws SecurityException, NoSuchMethodException {
> + if(isToBeRegistered(jsClient.getClass())) {
> + javaClientAndContextSet.add(new Object[]{jsClient, jsContext});
> }
> + }
>
> - return result;
> + /**
> + * Tests clazz to see if a custom teardown method has been written and caches the test result.
> + * If classes uses {@link AbstractJavaSamplerClient#teardownTest(JavaSamplerContext)} then it won't
> + * be registered for cleanup as it does nothing.
> + * @param clazz Class to be verified
> + * @return true if clazz should be registered for cleanup
> + * @throws SecurityException
> + * @throws NoSuchMethodException
> + */
> + private boolean isToBeRegistered(Class<? extends JavaSamplerClient> clazz) throws SecurityException, NoSuchMethodException {
> + Boolean isToBeRegistered = isToBeRegisteredCache.get(clazz.getName());
> + if(isToBeRegistered == null) {
> + Method method = clazz.getMethod("teardownTest", new Class[]{JavaSamplerContext.class});
> + isToBeRegistered = Boolean.valueOf(!method.getDeclaringClass().equals(AbstractJavaSamplerClient.class));
> + isToBeRegisteredCache.put(clazz.getName(), isToBeRegistered);
> + }
> + return isToBeRegistered.booleanValue();
> }
>
> /**
> @@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
> }
> javaClientAndContextSet.clear();
> }
> + isToBeRegisteredCache.clear();
> }
>
> /* Implements TestStateListener.testEnded(String) */
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
> @@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
> <ul>
> <li><bugzilla>55310</bugzilla> - TestAction should implement Interruptible</li>
> <li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP Request Defaults Control</li>
> +<li><bugzilla>53782</bugzilla> - Enhance JavaSampler handling of JavaSamplerClient cleanup to use less memory</li>
> </ul>
>
> <h3>Controllers</h3>
>
> Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml
> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
> +++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sat Aug 25 13:17:19 2012
> @@ -566,6 +566,9 @@ The fields allow variables to be used, s
> </p>
> </description>
>
> +<note>Since JMeter 2.8, if method teardownTest is not overriden by subclasses of AbstractJavaSamplerClient, then the method will not be called to optimise JMeter memory behaviour.
> +This will not have any impact on existing Test plans.
> +</note>
> <note>The Add/Delete buttons don't serve any purpose at present.</note>
>
> <properties>
>
>
>
Re: svn commit: r1377291 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Posted by sebb <se...@gmail.com>.
On 28 August 2012 20:59, Philippe Mouawad <ph...@gmail.com> wrote:
> Hello sebb,
> I fixed issues you mentioned so you can figure out what I mean.
OK.
> I didn't rollback as in my opinion there is really little chance of
> breaking something and if it happens we should get some bug report to fix
> it.
>
> If you want to, feel free to do it but in my opinion this change is worth
> regarding the benefit it brings in terms of memory.
>
> The second change I would make is to remove implementations of teardownTest
> methods in JavaTest and SleepTest as they are useless and the way they are
> coded now make them register for cleanup.
Done.
I think this is now complete.
> Regards
> Philippe
>
> On Tue, Aug 28, 2012 at 9:10 PM, Philippe Mouawad <
> philippe.mouawad@gmail.com> wrote:
>
>> Hello sebb,
>> My answers below.
>>
>> Regards
>> Philippe
>>
>> On Tue, Aug 28, 2012 at 7:58 PM, sebb <se...@gmail.com> wrote:
>>
>>> On 25 August 2012 14:17, <pm...@apache.org> wrote:
>>> > Author: pmouawad
>>> > Date: Sat Aug 25 13:17:19 2012
>>> > New Revision: 1377291
>>> >
>>> > URL: http://svn.apache.org/viewvc?rev=1377291&view=rev
>>> > Log:
>>> > Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup
>>> to use less memory
>>> > Only register instance of JavaSamplerClient that have overriden or
>>> implemented teardownTest
>>> > Bugzilla Id: 53782
>>>
>>> -1
>>>
>>> Sorry, but I don't think the fixes entirely solve the problem.
>>> See below for details.
>>>
>>> I think the first thing to do is to capture the existing behaviour in
>>> some unit test cases. For this the current changes need to be
>>> reverted.
>>>
>>> Once the test cases are set up correctly, changes can be made to try
>>> and implement the performance improvements.
>>>
>>> > Modified:
>>> >
>>> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
>>> > jmeter/trunk/xdocs/changes.xml
>>> > jmeter/trunk/xdocs/usermanual/component_reference.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=1377291&r1=1377290&r2=1377291&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
>>> Sat Aug 25 13:17:19 2012
>>> > @@ -18,10 +18,14 @@
>>> >
>>> > package org.apache.jmeter.protocol.java.sampler;
>>> >
>>> > +import java.lang.reflect.Method;
>>> > import java.util.Arrays;
>>> > import java.util.HashSet;
>>> > +import java.util.Map;
>>> > import java.util.Set;
>>> > +import java.util.concurrent.ConcurrentHashMap;
>>> >
>>> > +import org.apache.commons.lang3.exception.ExceptionUtils;
>>> > import org.apache.jmeter.config.Arguments;
>>> > import org.apache.jmeter.config.ConfigTestElement;
>>> > import org.apache.jmeter.samplers.AbstractSampler;
>>> > @@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
>>> > private transient JavaSamplerContext context = null;
>>> >
>>> > /**
>>> > + * Cache of classname, boolean that holds information about a
>>> class and wether or not it should
>>> > + * be registered for cleanup.
>>> > + * This is done to avoid using reflection on each registration
>>> > + */
>>> > + private transient Map<String, Boolean> isToBeRegisteredCache = new
>>> ConcurrentHashMap<String, Boolean>();
>>>
>>> This is not needed.
>>>
>>> There is only ever one class involved, which is the javaClient, so the
>>> value can be deternined when the class is instantiated.
>>>
>>> Sorry this should have been static.
>> In this case it is needed as it will avoid testing classes many times.
>>
>>> > +
>>> > + /**
>>> > * Set used to register all JavaSamplerClient and
>>> JavaSamplerContext.
>>> > * This is used so that the JavaSamplerClient can be notified when
>>> the test ends.
>>> > */
>>> > @@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
>>> > * @param entry
>>> > * the Entry for this sample
>>> > * @return test SampleResult
>>> > + * @throws NoSuchMethodException
>>> > + * @throws SecurityException
>>>
>>> The new code should not cause additional exceptions.
>>>
>> OK I will fix it.
>>
>>>
>>> > */
>>> > - public SampleResult sample(Entry entry) {
>>> > - Arguments args = getArguments();
>>> > - args.addArgument(TestElement.NAME, getName()); // Allow
>>> Sampler access
>>> > - // to test
>>> element name
>>> > - context = new JavaSamplerContext(args);
>>> > - if (javaClient == null) {
>>> > - log.debug(whoAmI() + "\tCreating Java Client");
>>> > - createJavaClient();
>>> > - javaClientAndContextSet.add(new Object[]{javaClient,
>>> context});
>>> > - javaClient.setupTest(context);
>>> > + public SampleResult sample(Entry entry) {
>>> > + try {
>>> > + Arguments args = getArguments();
>>> > + args.addArgument(TestElement.NAME, getName()); // Allow
>>> Sampler access
>>> > + // to test
>>> element name
>>> > + context = new JavaSamplerContext(args);
>>> > + if (javaClient == null) {
>>> > + log.debug(whoAmI() + "\tCreating Java Client");
>>> > + createJavaClient();
>>> > + registerForCleanup(javaClient, context);
>>> > + javaClient.setupTest(context);
>>> > + }
>>> > +
>>> > + SampleResult result = javaClient.runTest(context);
>>> > +
>>> > + // Only set the default label if it has not been set
>>> > + if (result != null && result.getSampleLabel().length() ==
>>> 0) {
>>> > + result.setSampleLabel(getName());
>>> > + }
>>> > +
>>> > + return result;
>>> > + } catch(Exception ex) {
>>> > + SampleResult sampleResultIfError = new SampleResult();
>>> > + sampleResultIfError.setSampleLabel(getName());
>>> > + sampleResultIfError.setSuccessful(false);
>>> > + sampleResultIfError.setResponseCode("500"); // $NON-NLS-1$
>>> > +
>>> sampleResultIfError.setResponseMessage(ExceptionUtils.getRootCauseMessage(ex));
>>> > +
>>> sampleResultIfError.setResponseData(ExceptionUtils.getStackTrace(ex),
>>> "UTF-8");
>>> > + return sampleResultIfError;
>>>
>>> There is already a special ErrorSamplerClient for such cases.
>>>
>>> Ok will use it.
>>
>>> > }
>>> > + }
>>> >
>>> > - SampleResult result = javaClient.runTest(context);
>>> > -
>>> > - // Only set the default label if it has not been set
>>> > - if (result != null && result.getSampleLabel().length() == 0) {
>>> > - result.setSampleLabel(getName());
>>> > + /**
>>> > + * Only register jsClient if it contains a custom teardownTest
>>> method
>>> > + * @param jsClient JavaSamplerClient
>>> > + * @param jsContext JavaSamplerContext
>>> > + * @throws NoSuchMethodException
>>> > + * @throws SecurityException
>>> > + */
>>> > + private final void registerForCleanup(JavaSamplerClient jsClient,
>>> > + JavaSamplerContext jsContext) throws SecurityException,
>>> NoSuchMethodException {
>>> > + if(isToBeRegistered(jsClient.getClass())) {
>>> > + javaClientAndContextSet.add(new Object[]{jsClient,
>>> jsContext});
>>>
>>> This is only called if sample() is called.
>>>
>>
>> This changes the behaviour in some edge cases.
>>>
>>> This is called only if javaClient is created , what would be this case ?
>> In case instance is cloned javaClient will also be null.
>>
>>> > }
>>> > + }
>>> >
>>> > - return result;
>>> > + /**
>>> > + * Tests clazz to see if a custom teardown method has been written
>>> and caches the test result.
>>> > + * If classes uses {@link
>>> AbstractJavaSamplerClient#teardownTest(JavaSamplerContext)} then it won't
>>> > + * be registered for cleanup as it does nothing.
>>> > + * @param clazz Class to be verified
>>> > + * @return true if clazz should be registered for cleanup
>>> > + * @throws SecurityException
>>> > + * @throws NoSuchMethodException
>>>
>>> Should not throw NoSuchMethodException.
>>>
>>> Not sure it should throw SecurityException either.
>>>
>> It should but as I will fix previous SampleResult as you propose it will
>> be OK.
>>
>>>
>>> > + */
>>> > + private boolean isToBeRegistered(Class<? extends
>>> JavaSamplerClient> clazz) throws SecurityException, NoSuchMethodException {
>>> > + Boolean isToBeRegistered =
>>> isToBeRegisteredCache.get(clazz.getName());
>>> > + if(isToBeRegistered == null) {
>>> > + Method method = clazz.getMethod("teardownTest", new
>>> Class[]{JavaSamplerContext.class});
>>>
>>> This does not deal with the case where a JavaSampler implementation
>>> does not sub-class AbstractJavaSamplerClient and does not define
>>> tearDownTest.
>>> We need a test for that.
>>>
>>
>> I am not sure to understand, as method must implement JavaSamplerClient,
>> it must define tearDownTest.
>> Am I missing something ?
>>
>>>
>>> If the method is not found, clearly there is no need to register the
>>> class.
>>>
>>> > + isToBeRegistered =
>>> Boolean.valueOf(!method.getDeclaringClass().equals(AbstractJavaSamplerClient.class));
>>> > + isToBeRegisteredCache.put(clazz.getName(),
>>> isToBeRegistered);
>>> > + }
>>> > + return isToBeRegistered.booleanValue();
>>> > }
>>> >
>>> > /**
>>> > @@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
>>> > }
>>> > javaClientAndContextSet.clear();
>>> > }
>>> > + isToBeRegisteredCache.clear();
>>> > }
>>> >
>>> > /* Implements TestStateListener.testEnded(String) */
>>> >
>>> > Modified: jmeter/trunk/xdocs/changes.xml
>>> > URL:
>>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
>>> >
>>> ==============================================================================
>>> > --- jmeter/trunk/xdocs/changes.xml (original)
>>> > +++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
>>> > @@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
>>> > <ul>
>>> > <li><bugzilla>55310</bugzilla> - TestAction should implement
>>> Interruptible</li>
>>> > <li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP
>>> Request Defaults Control </li>
>>> > +<li><bugzilla>53782</bugzilla> - Enhance JavaSampler handling of
>>> JavaSamplerClient cleanup to use less memory</li>
>>> > </ul>
>>> >
>>> > <h3>Controllers</h3>
>>> >
>>> > Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml
>>> > URL:
>>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
>>> >
>>> ==============================================================================
>>> > --- jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
>>> > +++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sat Aug 25
>>> 13:17:19 2012
>>> > @@ -566,6 +566,9 @@ The fields allow variables to be used, s
>>> > </p>
>>> > </description>
>>> >
>>> > +<note>Since JMeter 2.8, if method teardownTest is not overriden by
>>> subclasses of AbstractJavaSamplerClient, then the method will not be called
>>> to optimise JMeter memory behaviour.
>>> > +This will not have any impact on existing Test plans.
>>> > +</note>
>>>
>>> I don't think the note should be added to component reference (it will
>>> just be confusing to readers); it belongs in changes.
>>>
>>> Ok I can remove it.
>>
>>> > <note>The Add/Delete buttons don't serve any purpose at
>>> present.</note>
>>> >
>>> > <properties>
>>> >
>>> >
>>>
>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.
>>
>>
>>
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.
Re: svn commit: r1377291 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Posted by Philippe Mouawad <ph...@gmail.com>.
Hello sebb,
I fixed issues you mentioned so you can figure out what I mean.
I didn't rollback as in my opinion there is really little chance of
breaking something and if it happens we should get some bug report to fix
it.
If you want to, feel free to do it but in my opinion this change is worth
regarding the benefit it brings in terms of memory.
The second change I would make is to remove implementations of teardownTest
methods in JavaTest and SleepTest as they are useless and the way they are
coded now make them register for cleanup.
Regards
Philippe
On Tue, Aug 28, 2012 at 9:10 PM, Philippe Mouawad <
philippe.mouawad@gmail.com> wrote:
> Hello sebb,
> My answers below.
>
> Regards
> Philippe
>
> On Tue, Aug 28, 2012 at 7:58 PM, sebb <se...@gmail.com> wrote:
>
>> On 25 August 2012 14:17, <pm...@apache.org> wrote:
>> > Author: pmouawad
>> > Date: Sat Aug 25 13:17:19 2012
>> > New Revision: 1377291
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1377291&view=rev
>> > Log:
>> > Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup
>> to use less memory
>> > Only register instance of JavaSamplerClient that have overriden or
>> implemented teardownTest
>> > Bugzilla Id: 53782
>>
>> -1
>>
>> Sorry, but I don't think the fixes entirely solve the problem.
>> See below for details.
>>
>> I think the first thing to do is to capture the existing behaviour in
>> some unit test cases. For this the current changes need to be
>> reverted.
>>
>> Once the test cases are set up correctly, changes can be made to try
>> and implement the performance improvements.
>>
>> > Modified:
>> >
>> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
>> > jmeter/trunk/xdocs/changes.xml
>> > jmeter/trunk/xdocs/usermanual/component_reference.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=1377291&r1=1377290&r2=1377291&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
>> Sat Aug 25 13:17:19 2012
>> > @@ -18,10 +18,14 @@
>> >
>> > package org.apache.jmeter.protocol.java.sampler;
>> >
>> > +import java.lang.reflect.Method;
>> > import java.util.Arrays;
>> > import java.util.HashSet;
>> > +import java.util.Map;
>> > import java.util.Set;
>> > +import java.util.concurrent.ConcurrentHashMap;
>> >
>> > +import org.apache.commons.lang3.exception.ExceptionUtils;
>> > import org.apache.jmeter.config.Arguments;
>> > import org.apache.jmeter.config.ConfigTestElement;
>> > import org.apache.jmeter.samplers.AbstractSampler;
>> > @@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
>> > private transient JavaSamplerContext context = null;
>> >
>> > /**
>> > + * Cache of classname, boolean that holds information about a
>> class and wether or not it should
>> > + * be registered for cleanup.
>> > + * This is done to avoid using reflection on each registration
>> > + */
>> > + private transient Map<String, Boolean> isToBeRegisteredCache = new
>> ConcurrentHashMap<String, Boolean>();
>>
>> This is not needed.
>>
>> There is only ever one class involved, which is the javaClient, so the
>> value can be deternined when the class is instantiated.
>>
>> Sorry this should have been static.
> In this case it is needed as it will avoid testing classes many times.
>
>> > +
>> > + /**
>> > * Set used to register all JavaSamplerClient and
>> JavaSamplerContext.
>> > * This is used so that the JavaSamplerClient can be notified when
>> the test ends.
>> > */
>> > @@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
>> > * @param entry
>> > * the Entry for this sample
>> > * @return test SampleResult
>> > + * @throws NoSuchMethodException
>> > + * @throws SecurityException
>>
>> The new code should not cause additional exceptions.
>>
> OK I will fix it.
>
>>
>> > */
>> > - public SampleResult sample(Entry entry) {
>> > - Arguments args = getArguments();
>> > - args.addArgument(TestElement.NAME, getName()); // Allow
>> Sampler access
>> > - // to test
>> element name
>> > - context = new JavaSamplerContext(args);
>> > - if (javaClient == null) {
>> > - log.debug(whoAmI() + "\tCreating Java Client");
>> > - createJavaClient();
>> > - javaClientAndContextSet.add(new Object[]{javaClient,
>> context});
>> > - javaClient.setupTest(context);
>> > + public SampleResult sample(Entry entry) {
>> > + try {
>> > + Arguments args = getArguments();
>> > + args.addArgument(TestElement.NAME, getName()); // Allow
>> Sampler access
>> > + // to test
>> element name
>> > + context = new JavaSamplerContext(args);
>> > + if (javaClient == null) {
>> > + log.debug(whoAmI() + "\tCreating Java Client");
>> > + createJavaClient();
>> > + registerForCleanup(javaClient, context);
>> > + javaClient.setupTest(context);
>> > + }
>> > +
>> > + SampleResult result = javaClient.runTest(context);
>> > +
>> > + // Only set the default label if it has not been set
>> > + if (result != null && result.getSampleLabel().length() ==
>> 0) {
>> > + result.setSampleLabel(getName());
>> > + }
>> > +
>> > + return result;
>> > + } catch(Exception ex) {
>> > + SampleResult sampleResultIfError = new SampleResult();
>> > + sampleResultIfError.setSampleLabel(getName());
>> > + sampleResultIfError.setSuccessful(false);
>> > + sampleResultIfError.setResponseCode("500"); // $NON-NLS-1$
>> > +
>> sampleResultIfError.setResponseMessage(ExceptionUtils.getRootCauseMessage(ex));
>> > +
>> sampleResultIfError.setResponseData(ExceptionUtils.getStackTrace(ex),
>> "UTF-8");
>> > + return sampleResultIfError;
>>
>> There is already a special ErrorSamplerClient for such cases.
>>
>> Ok will use it.
>
>> > }
>> > + }
>> >
>> > - SampleResult result = javaClient.runTest(context);
>> > -
>> > - // Only set the default label if it has not been set
>> > - if (result != null && result.getSampleLabel().length() == 0) {
>> > - result.setSampleLabel(getName());
>> > + /**
>> > + * Only register jsClient if it contains a custom teardownTest
>> method
>> > + * @param jsClient JavaSamplerClient
>> > + * @param jsContext JavaSamplerContext
>> > + * @throws NoSuchMethodException
>> > + * @throws SecurityException
>> > + */
>> > + private final void registerForCleanup(JavaSamplerClient jsClient,
>> > + JavaSamplerContext jsContext) throws SecurityException,
>> NoSuchMethodException {
>> > + if(isToBeRegistered(jsClient.getClass())) {
>> > + javaClientAndContextSet.add(new Object[]{jsClient,
>> jsContext});
>>
>> This is only called if sample() is called.
>>
>
> This changes the behaviour in some edge cases.
>>
>> This is called only if javaClient is created , what would be this case ?
> In case instance is cloned javaClient will also be null.
>
>> > }
>> > + }
>> >
>> > - return result;
>> > + /**
>> > + * Tests clazz to see if a custom teardown method has been written
>> and caches the test result.
>> > + * If classes uses {@link
>> AbstractJavaSamplerClient#teardownTest(JavaSamplerContext)} then it won't
>> > + * be registered for cleanup as it does nothing.
>> > + * @param clazz Class to be verified
>> > + * @return true if clazz should be registered for cleanup
>> > + * @throws SecurityException
>> > + * @throws NoSuchMethodException
>>
>> Should not throw NoSuchMethodException.
>>
>> Not sure it should throw SecurityException either.
>>
> It should but as I will fix previous SampleResult as you propose it will
> be OK.
>
>>
>> > + */
>> > + private boolean isToBeRegistered(Class<? extends
>> JavaSamplerClient> clazz) throws SecurityException, NoSuchMethodException {
>> > + Boolean isToBeRegistered =
>> isToBeRegisteredCache.get(clazz.getName());
>> > + if(isToBeRegistered == null) {
>> > + Method method = clazz.getMethod("teardownTest", new
>> Class[]{JavaSamplerContext.class});
>>
>> This does not deal with the case where a JavaSampler implementation
>> does not sub-class AbstractJavaSamplerClient and does not define
>> tearDownTest.
>> We need a test for that.
>>
>
> I am not sure to understand, as method must implement JavaSamplerClient,
> it must define tearDownTest.
> Am I missing something ?
>
>>
>> If the method is not found, clearly there is no need to register the
>> class.
>>
>> > + isToBeRegistered =
>> Boolean.valueOf(!method.getDeclaringClass().equals(AbstractJavaSamplerClient.class));
>> > + isToBeRegisteredCache.put(clazz.getName(),
>> isToBeRegistered);
>> > + }
>> > + return isToBeRegistered.booleanValue();
>> > }
>> >
>> > /**
>> > @@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
>> > }
>> > javaClientAndContextSet.clear();
>> > }
>> > + isToBeRegisteredCache.clear();
>> > }
>> >
>> > /* Implements TestStateListener.testEnded(String) */
>> >
>> > Modified: jmeter/trunk/xdocs/changes.xml
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/xdocs/changes.xml (original)
>> > +++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
>> > @@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
>> > <ul>
>> > <li><bugzilla>55310</bugzilla> - TestAction should implement
>> Interruptible</li>
>> > <li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP
>> Request Defaults Control </li>
>> > +<li><bugzilla>53782</bugzilla> - Enhance JavaSampler handling of
>> JavaSamplerClient cleanup to use less memory</li>
>> > </ul>
>> >
>> > <h3>Controllers</h3>
>> >
>> > Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
>> > +++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sat Aug 25
>> 13:17:19 2012
>> > @@ -566,6 +566,9 @@ The fields allow variables to be used, s
>> > </p>
>> > </description>
>> >
>> > +<note>Since JMeter 2.8, if method teardownTest is not overriden by
>> subclasses of AbstractJavaSamplerClient, then the method will not be called
>> to optimise JMeter memory behaviour.
>> > +This will not have any impact on existing Test plans.
>> > +</note>
>>
>> I don't think the note should be added to component reference (it will
>> just be confusing to readers); it belongs in changes.
>>
>> Ok I can remove it.
>
>> > <note>The Add/Delete buttons don't serve any purpose at
>> present.</note>
>> >
>> > <properties>
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
>
>
>
--
Cordialement.
Philippe Mouawad.
Re: svn commit: r1377291 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Posted by sebb <se...@gmail.com>.
On 28 August 2012 20:10, Philippe Mouawad <ph...@gmail.com> wrote:
> Hello sebb,
> My answers below.
>
> Regards
> Philippe
>
> On Tue, Aug 28, 2012 at 7:58 PM, sebb <se...@gmail.com> wrote:
>
>> On 25 August 2012 14:17, <pm...@apache.org> wrote:
>> > Author: pmouawad
>> > Date: Sat Aug 25 13:17:19 2012
>> > New Revision: 1377291
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1377291&view=rev
>> > Log:
>> > Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup to
>> use less memory
>> > Only register instance of JavaSamplerClient that have overriden or
>> implemented teardownTest
>> > Bugzilla Id: 53782
>>
>> -1
>>
>> Sorry, but I don't think the fixes entirely solve the problem.
>> See below for details.
>>
>> I think the first thing to do is to capture the existing behaviour in
>> some unit test cases. For this the current changes need to be
>> reverted.
>>
>> Once the test cases are set up correctly, changes can be made to try
>> and implement the performance improvements.
>>
>> > Modified:
>> >
>> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
>> > jmeter/trunk/xdocs/changes.xml
>> > jmeter/trunk/xdocs/usermanual/component_reference.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=1377291&r1=1377290&r2=1377291&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
>> Sat Aug 25 13:17:19 2012
>> > @@ -18,10 +18,14 @@
>> >
>> > package org.apache.jmeter.protocol.java.sampler;
>> >
>> > +import java.lang.reflect.Method;
>> > import java.util.Arrays;
>> > import java.util.HashSet;
>> > +import java.util.Map;
>> > import java.util.Set;
>> > +import java.util.concurrent.ConcurrentHashMap;
>> >
>> > +import org.apache.commons.lang3.exception.ExceptionUtils;
>> > import org.apache.jmeter.config.Arguments;
>> > import org.apache.jmeter.config.ConfigTestElement;
>> > import org.apache.jmeter.samplers.AbstractSampler;
>> > @@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
The Set javaClientAndContextSet could contain JavaSampler instances as
before (but not all instances).
The instance contains javaClient and context, which are all that is
needed to call the teardownTest method.
That would save extra references, and simplify the code.
>> > private transient JavaSamplerContext context = null;
>> >
>> > /**
>> > + * Cache of classname, boolean that holds information about a class
>> and wether or not it should
>> > + * be registered for cleanup.
>> > + * This is done to avoid using reflection on each registration
>> > + */
>> > + private transient Map<String, Boolean> isToBeRegisteredCache = new
>> ConcurrentHashMap<String, Boolean>();
>>
>> This is not needed.
>>
>> There is only ever one class involved, which is the javaClient, so the
>> value can be deternined when the class is instantiated.
>>
>>
> Sorry this should have been static.
> In this case it is needed as it will avoid testing classes many times.
It could be avoided by setting it up in the testStarted() method as
that is done before the class is cloned.
The testStarted() method could create the class (but not instantiate
it) and check for the method implementation.
The check would still be repeated for each instance of the class in a
test plan, but caching that seems unnecessary.
>> > +
>> > + /**
>> > * Set used to register all JavaSamplerClient and
>> JavaSamplerContext.
>> > * This is used so that the JavaSamplerClient can be notified when
>> the test ends.
>> > */
>> > @@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
>> > * @param entry
>> > * the Entry for this sample
>> > * @return test SampleResult
>> > + * @throws NoSuchMethodException
>> > + * @throws SecurityException
>>
>> The new code should not cause additional exceptions.
>>
> OK I will fix it.
>
>>
>> > */
>> > - public SampleResult sample(Entry entry) {
>> > - Arguments args = getArguments();
>> > - args.addArgument(TestElement.NAME, getName()); // Allow Sampler
>> access
>> > - // to test
>> element name
>> > - context = new JavaSamplerContext(args);
>> > - if (javaClient == null) {
>> > - log.debug(whoAmI() + "\tCreating Java Client");
>> > - createJavaClient();
>> > - javaClientAndContextSet.add(new Object[]{javaClient,
>> context});
>> > - javaClient.setupTest(context);
>> > + public SampleResult sample(Entry entry) {
>> > + try {
>> > + Arguments args = getArguments();
>> > + args.addArgument(TestElement.NAME, getName()); // Allow
>> Sampler access
>> > + // to test
>> element name
>> > + context = new JavaSamplerContext(args);
>> > + if (javaClient == null) {
>> > + log.debug(whoAmI() + "\tCreating Java Client");
>> > + createJavaClient();
>> > + registerForCleanup(javaClient, context);
>> > + javaClient.setupTest(context);
>> > + }
>> > +
>> > + SampleResult result = javaClient.runTest(context);
>> > +
>> > + // Only set the default label if it has not been set
>> > + if (result != null && result.getSampleLabel().length() ==
>> 0) {
>> > + result.setSampleLabel(getName());
>> > + }
>> > +
>> > + return result;
>> > + } catch(Exception ex) {
>> > + SampleResult sampleResultIfError = new SampleResult();
>> > + sampleResultIfError.setSampleLabel(getName());
>> > + sampleResultIfError.setSuccessful(false);
>> > + sampleResultIfError.setResponseCode("500"); // $NON-NLS-1$
>> > +
>> sampleResultIfError.setResponseMessage(ExceptionUtils.getRootCauseMessage(ex));
>> > +
>> sampleResultIfError.setResponseData(ExceptionUtils.getStackTrace(ex),
>> "UTF-8");
>> > + return sampleResultIfError;
>>
>> There is already a special ErrorSamplerClient for such cases.
>>
>> Ok will use it.
>> > }
>> > + }
>> >
>> > - SampleResult result = javaClient.runTest(context);
>> > -
>> > - // Only set the default label if it has not been set
>> > - if (result != null && result.getSampleLabel().length() == 0) {
>> > - result.setSampleLabel(getName());
>> > + /**
>> > + * Only register jsClient if it contains a custom teardownTest
>> method
>> > + * @param jsClient JavaSamplerClient
>> > + * @param jsContext JavaSamplerContext
>> > + * @throws NoSuchMethodException
>> > + * @throws SecurityException
>> > + */
>> > + private final void registerForCleanup(JavaSamplerClient jsClient,
>> > + JavaSamplerContext jsContext) throws SecurityException,
>> NoSuchMethodException {
>> > + if(isToBeRegistered(jsClient.getClass())) {
>> > + javaClientAndContextSet.add(new Object[]{jsClient,
>> jsContext});
>>
>> This is only called if sample() is called.
>>
>> This changes the behaviour in some edge cases.
>>
> This is called only if javaClient is created , what would be this case ?
> In case instance is cloned javaClient will also be null.
Sorry, I'd overlooked the fact that the method is not called if the
client has not been created yet.
>> > }
>> > + }
>> >
>> > - return result;
>> > + /**
>> > + * Tests clazz to see if a custom teardown method has been written
>> and caches the test result.
>> > + * If classes uses {@link
>> AbstractJavaSamplerClient#teardownTest(JavaSamplerContext)} then it won't
>> > + * be registered for cleanup as it does nothing.
>> > + * @param clazz Class to be verified
>> > + * @return true if clazz should be registered for cleanup
>> > + * @throws SecurityException
>> > + * @throws NoSuchMethodException
>>
>> Should not throw NoSuchMethodException.
>>
>> Not sure it should throw SecurityException either.
>>
> It should but as I will fix previous SampleResult as you propose it will
> be OK.
>
>>
>> > + */
>> > + private boolean isToBeRegistered(Class<? extends JavaSamplerClient>
>> clazz) throws SecurityException, NoSuchMethodException {
>> > + Boolean isToBeRegistered =
>> isToBeRegisteredCache.get(clazz.getName());
>> > + if(isToBeRegistered == null) {
>> > + Method method = clazz.getMethod("teardownTest", new
>> Class[]{JavaSamplerContext.class});
>>
>> This does not deal with the case where a JavaSampler implementation
>> does not sub-class AbstractJavaSamplerClient and does not define
>> tearDownTest.
>> We need a test for that.
>>
>
> I am not sure to understand, as method must implement JavaSamplerClient,
> it must define tearDownTest.
> Am I missing something ?
Sorry, I'd overlooked that.
>>
>> If the method is not found, clearly there is no need to register the class.
>>
>> > + isToBeRegistered =
>> Boolean.valueOf(!method.getDeclaringClass().equals(AbstractJavaSamplerClient.class));
>> > + isToBeRegisteredCache.put(clazz.getName(),
>> isToBeRegistered);
>> > + }
>> > + return isToBeRegistered.booleanValue();
>> > }
>> >
>> > /**
>> > @@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
>> > }
>> > javaClientAndContextSet.clear();
>> > }
>> > + isToBeRegisteredCache.clear();
>> > }
>> >
>> > /* Implements TestStateListener.testEnded(String) */
>> >
>> > Modified: jmeter/trunk/xdocs/changes.xml
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/xdocs/changes.xml (original)
>> > +++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
>> > @@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
>> > <ul>
>> > <li><bugzilla>55310</bugzilla> - TestAction should implement
>> Interruptible</li>
>> > <li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP
>> Request Defaults Control </li>
>> > +<li><bugzilla>53782</bugzilla> - Enhance JavaSampler handling of
>> JavaSamplerClient cleanup to use less memory</li>
>> > </ul>
>> >
>> > <h3>Controllers</h3>
>> >
>> > Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
>> > +++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sat Aug 25
>> 13:17:19 2012
>> > @@ -566,6 +566,9 @@ The fields allow variables to be used, s
>> > </p>
>> > </description>
>> >
>> > +<note>Since JMeter 2.8, if method teardownTest is not overriden by
>> subclasses of AbstractJavaSamplerClient, then the method will not be called
>> to optimise JMeter memory behaviour.
>> > +This will not have any impact on existing Test plans.
>> > +</note>
>>
>> I don't think the note should be added to component reference (it will
>> just be confusing to readers); it belongs in changes.
>>
>> Ok I can remove it.
>
>> > <note>The Add/Delete buttons don't serve any purpose at present.</note>
>> >
>> > <properties>
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
Re: svn commit: r1377291 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Posted by Philippe Mouawad <ph...@gmail.com>.
Hello sebb,
My answers below.
Regards
Philippe
On Tue, Aug 28, 2012 at 7:58 PM, sebb <se...@gmail.com> wrote:
> On 25 August 2012 14:17, <pm...@apache.org> wrote:
> > Author: pmouawad
> > Date: Sat Aug 25 13:17:19 2012
> > New Revision: 1377291
> >
> > URL: http://svn.apache.org/viewvc?rev=1377291&view=rev
> > Log:
> > Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup to
> use less memory
> > Only register instance of JavaSamplerClient that have overriden or
> implemented teardownTest
> > Bugzilla Id: 53782
>
> -1
>
> Sorry, but I don't think the fixes entirely solve the problem.
> See below for details.
>
> I think the first thing to do is to capture the existing behaviour in
> some unit test cases. For this the current changes need to be
> reverted.
>
> Once the test cases are set up correctly, changes can be made to try
> and implement the performance improvements.
>
> > Modified:
> >
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> > jmeter/trunk/xdocs/changes.xml
> > jmeter/trunk/xdocs/usermanual/component_reference.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=1377291&r1=1377290&r2=1377291&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
> Sat Aug 25 13:17:19 2012
> > @@ -18,10 +18,14 @@
> >
> > package org.apache.jmeter.protocol.java.sampler;
> >
> > +import java.lang.reflect.Method;
> > import java.util.Arrays;
> > import java.util.HashSet;
> > +import java.util.Map;
> > import java.util.Set;
> > +import java.util.concurrent.ConcurrentHashMap;
> >
> > +import org.apache.commons.lang3.exception.ExceptionUtils;
> > import org.apache.jmeter.config.Arguments;
> > import org.apache.jmeter.config.ConfigTestElement;
> > import org.apache.jmeter.samplers.AbstractSampler;
> > @@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
> > private transient JavaSamplerContext context = null;
> >
> > /**
> > + * Cache of classname, boolean that holds information about a class
> and wether or not it should
> > + * be registered for cleanup.
> > + * This is done to avoid using reflection on each registration
> > + */
> > + private transient Map<String, Boolean> isToBeRegisteredCache = new
> ConcurrentHashMap<String, Boolean>();
>
> This is not needed.
>
> There is only ever one class involved, which is the javaClient, so the
> value can be deternined when the class is instantiated.
>
> Sorry this should have been static.
In this case it is needed as it will avoid testing classes many times.
> > +
> > + /**
> > * Set used to register all JavaSamplerClient and
> JavaSamplerContext.
> > * This is used so that the JavaSamplerClient can be notified when
> the test ends.
> > */
> > @@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
> > * @param entry
> > * the Entry for this sample
> > * @return test SampleResult
> > + * @throws NoSuchMethodException
> > + * @throws SecurityException
>
> The new code should not cause additional exceptions.
>
OK I will fix it.
>
> > */
> > - public SampleResult sample(Entry entry) {
> > - Arguments args = getArguments();
> > - args.addArgument(TestElement.NAME, getName()); // Allow Sampler
> access
> > - // to test
> element name
> > - context = new JavaSamplerContext(args);
> > - if (javaClient == null) {
> > - log.debug(whoAmI() + "\tCreating Java Client");
> > - createJavaClient();
> > - javaClientAndContextSet.add(new Object[]{javaClient,
> context});
> > - javaClient.setupTest(context);
> > + public SampleResult sample(Entry entry) {
> > + try {
> > + Arguments args = getArguments();
> > + args.addArgument(TestElement.NAME, getName()); // Allow
> Sampler access
> > + // to test
> element name
> > + context = new JavaSamplerContext(args);
> > + if (javaClient == null) {
> > + log.debug(whoAmI() + "\tCreating Java Client");
> > + createJavaClient();
> > + registerForCleanup(javaClient, context);
> > + javaClient.setupTest(context);
> > + }
> > +
> > + SampleResult result = javaClient.runTest(context);
> > +
> > + // Only set the default label if it has not been set
> > + if (result != null && result.getSampleLabel().length() ==
> 0) {
> > + result.setSampleLabel(getName());
> > + }
> > +
> > + return result;
> > + } catch(Exception ex) {
> > + SampleResult sampleResultIfError = new SampleResult();
> > + sampleResultIfError.setSampleLabel(getName());
> > + sampleResultIfError.setSuccessful(false);
> > + sampleResultIfError.setResponseCode("500"); // $NON-NLS-1$
> > +
> sampleResultIfError.setResponseMessage(ExceptionUtils.getRootCauseMessage(ex));
> > +
> sampleResultIfError.setResponseData(ExceptionUtils.getStackTrace(ex),
> "UTF-8");
> > + return sampleResultIfError;
>
> There is already a special ErrorSamplerClient for such cases.
>
> Ok will use it.
> > }
> > + }
> >
> > - SampleResult result = javaClient.runTest(context);
> > -
> > - // Only set the default label if it has not been set
> > - if (result != null && result.getSampleLabel().length() == 0) {
> > - result.setSampleLabel(getName());
> > + /**
> > + * Only register jsClient if it contains a custom teardownTest
> method
> > + * @param jsClient JavaSamplerClient
> > + * @param jsContext JavaSamplerContext
> > + * @throws NoSuchMethodException
> > + * @throws SecurityException
> > + */
> > + private final void registerForCleanup(JavaSamplerClient jsClient,
> > + JavaSamplerContext jsContext) throws SecurityException,
> NoSuchMethodException {
> > + if(isToBeRegistered(jsClient.getClass())) {
> > + javaClientAndContextSet.add(new Object[]{jsClient,
> jsContext});
>
> This is only called if sample() is called.
>
This changes the behaviour in some edge cases.
>
> This is called only if javaClient is created , what would be this case ?
In case instance is cloned javaClient will also be null.
> > }
> > + }
> >
> > - return result;
> > + /**
> > + * Tests clazz to see if a custom teardown method has been written
> and caches the test result.
> > + * If classes uses {@link
> AbstractJavaSamplerClient#teardownTest(JavaSamplerContext)} then it won't
> > + * be registered for cleanup as it does nothing.
> > + * @param clazz Class to be verified
> > + * @return true if clazz should be registered for cleanup
> > + * @throws SecurityException
> > + * @throws NoSuchMethodException
>
> Should not throw NoSuchMethodException.
>
> Not sure it should throw SecurityException either.
>
It should but as I will fix previous SampleResult as you propose it will
be OK.
>
> > + */
> > + private boolean isToBeRegistered(Class<? extends JavaSamplerClient>
> clazz) throws SecurityException, NoSuchMethodException {
> > + Boolean isToBeRegistered =
> isToBeRegisteredCache.get(clazz.getName());
> > + if(isToBeRegistered == null) {
> > + Method method = clazz.getMethod("teardownTest", new
> Class[]{JavaSamplerContext.class});
>
> This does not deal with the case where a JavaSampler implementation
> does not sub-class AbstractJavaSamplerClient and does not define
> tearDownTest.
> We need a test for that.
>
I am not sure to understand, as method must implement JavaSamplerClient,
it must define tearDownTest.
Am I missing something ?
>
> If the method is not found, clearly there is no need to register the class.
>
> > + isToBeRegistered =
> Boolean.valueOf(!method.getDeclaringClass().equals(AbstractJavaSamplerClient.class));
> > + isToBeRegisteredCache.put(clazz.getName(),
> isToBeRegistered);
> > + }
> > + return isToBeRegistered.booleanValue();
> > }
> >
> > /**
> > @@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
> > }
> > javaClientAndContextSet.clear();
> > }
> > + isToBeRegisteredCache.clear();
> > }
> >
> > /* Implements TestStateListener.testEnded(String) */
> >
> > Modified: jmeter/trunk/xdocs/changes.xml
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/xdocs/changes.xml (original)
> > +++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
> > @@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
> > <ul>
> > <li><bugzilla>55310</bugzilla> - TestAction should implement
> Interruptible</li>
> > <li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP
> Request Defaults Control </li>
> > +<li><bugzilla>53782</bugzilla> - Enhance JavaSampler handling of
> JavaSamplerClient cleanup to use less memory</li>
> > </ul>
> >
> > <h3>Controllers</h3>
> >
> > Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
> > +++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sat Aug 25
> 13:17:19 2012
> > @@ -566,6 +566,9 @@ The fields allow variables to be used, s
> > </p>
> > </description>
> >
> > +<note>Since JMeter 2.8, if method teardownTest is not overriden by
> subclasses of AbstractJavaSamplerClient, then the method will not be called
> to optimise JMeter memory behaviour.
> > +This will not have any impact on existing Test plans.
> > +</note>
>
> I don't think the note should be added to component reference (it will
> just be confusing to readers); it belongs in changes.
>
> Ok I can remove it.
> > <note>The Add/Delete buttons don't serve any purpose at present.</note>
> >
> > <properties>
> >
> >
>
--
Cordialement.
Philippe Mouawad.
Re: svn commit: r1377291 - in /jmeter/trunk: src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
xdocs/changes.xml xdocs/usermanual/component_reference.xml
Posted by sebb <se...@gmail.com>.
On 25 August 2012 14:17, <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Sat Aug 25 13:17:19 2012
> New Revision: 1377291
>
> URL: http://svn.apache.org/viewvc?rev=1377291&view=rev
> Log:
> Bug 53782 - Enhance JavaSampler handling of JavaSamplerClient cleanup to use less memory
> Only register instance of JavaSamplerClient that have overriden or implemented teardownTest
> Bugzilla Id: 53782
-1
Sorry, but I don't think the fixes entirely solve the problem.
See below for details.
I think the first thing to do is to capture the existing behaviour in
some unit test cases. For this the current changes need to be
reverted.
Once the test cases are set up correctly, changes can be made to try
and implement the performance improvements.
> Modified:
> jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
> jmeter/trunk/xdocs/changes.xml
> jmeter/trunk/xdocs/usermanual/component_reference.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=1377291&r1=1377290&r2=1377291&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 Sat Aug 25 13:17:19 2012
> @@ -18,10 +18,14 @@
>
> package org.apache.jmeter.protocol.java.sampler;
>
> +import java.lang.reflect.Method;
> import java.util.Arrays;
> import java.util.HashSet;
> +import java.util.Map;
> import java.util.Set;
> +import java.util.concurrent.ConcurrentHashMap;
>
> +import org.apache.commons.lang3.exception.ExceptionUtils;
> import org.apache.jmeter.config.Arguments;
> import org.apache.jmeter.config.ConfigTestElement;
> import org.apache.jmeter.samplers.AbstractSampler;
> @@ -74,6 +78,13 @@ public class JavaSampler extends Abstrac
> private transient JavaSamplerContext context = null;
>
> /**
> + * Cache of classname, boolean that holds information about a class and wether or not it should
> + * be registered for cleanup.
> + * This is done to avoid using reflection on each registration
> + */
> + private transient Map<String, Boolean> isToBeRegisteredCache = new ConcurrentHashMap<String, Boolean>();
This is not needed.
There is only ever one class involved, which is the javaClient, so the
value can be deternined when the class is instantiated.
> +
> + /**
> * Set used to register all JavaSamplerClient and JavaSamplerContext.
> * This is used so that the JavaSamplerClient can be notified when the test ends.
> */
> @@ -137,27 +148,72 @@ public class JavaSampler extends Abstrac
> * @param entry
> * the Entry for this sample
> * @return test SampleResult
> + * @throws NoSuchMethodException
> + * @throws SecurityException
The new code should not cause additional exceptions.
> */
> - public SampleResult sample(Entry entry) {
> - Arguments args = getArguments();
> - args.addArgument(TestElement.NAME, getName()); // Allow Sampler access
> - // to test element name
> - context = new JavaSamplerContext(args);
> - if (javaClient == null) {
> - log.debug(whoAmI() + "\tCreating Java Client");
> - createJavaClient();
> - javaClientAndContextSet.add(new Object[]{javaClient, context});
> - javaClient.setupTest(context);
> + public SampleResult sample(Entry entry) {
> + try {
> + Arguments args = getArguments();
> + args.addArgument(TestElement.NAME, getName()); // Allow Sampler access
> + // to test element name
> + context = new JavaSamplerContext(args);
> + if (javaClient == null) {
> + log.debug(whoAmI() + "\tCreating Java Client");
> + createJavaClient();
> + registerForCleanup(javaClient, context);
> + javaClient.setupTest(context);
> + }
> +
> + SampleResult result = javaClient.runTest(context);
> +
> + // Only set the default label if it has not been set
> + if (result != null && result.getSampleLabel().length() == 0) {
> + result.setSampleLabel(getName());
> + }
> +
> + return result;
> + } catch(Exception ex) {
> + SampleResult sampleResultIfError = new SampleResult();
> + sampleResultIfError.setSampleLabel(getName());
> + sampleResultIfError.setSuccessful(false);
> + sampleResultIfError.setResponseCode("500"); // $NON-NLS-1$
> + sampleResultIfError.setResponseMessage(ExceptionUtils.getRootCauseMessage(ex));
> + sampleResultIfError.setResponseData(ExceptionUtils.getStackTrace(ex), "UTF-8");
> + return sampleResultIfError;
There is already a special ErrorSamplerClient for such cases.
> }
> + }
>
> - SampleResult result = javaClient.runTest(context);
> -
> - // Only set the default label if it has not been set
> - if (result != null && result.getSampleLabel().length() == 0) {
> - result.setSampleLabel(getName());
> + /**
> + * Only register jsClient if it contains a custom teardownTest method
> + * @param jsClient JavaSamplerClient
> + * @param jsContext JavaSamplerContext
> + * @throws NoSuchMethodException
> + * @throws SecurityException
> + */
> + private final void registerForCleanup(JavaSamplerClient jsClient,
> + JavaSamplerContext jsContext) throws SecurityException, NoSuchMethodException {
> + if(isToBeRegistered(jsClient.getClass())) {
> + javaClientAndContextSet.add(new Object[]{jsClient, jsContext});
This is only called if sample() is called.
This changes the behaviour in some edge cases.
> }
> + }
>
> - return result;
> + /**
> + * Tests clazz to see if a custom teardown method has been written and caches the test result.
> + * If classes uses {@link AbstractJavaSamplerClient#teardownTest(JavaSamplerContext)} then it won't
> + * be registered for cleanup as it does nothing.
> + * @param clazz Class to be verified
> + * @return true if clazz should be registered for cleanup
> + * @throws SecurityException
> + * @throws NoSuchMethodException
Should not throw NoSuchMethodException.
Not sure it should throw SecurityException either.
> + */
> + private boolean isToBeRegistered(Class<? extends JavaSamplerClient> clazz) throws SecurityException, NoSuchMethodException {
> + Boolean isToBeRegistered = isToBeRegisteredCache.get(clazz.getName());
> + if(isToBeRegistered == null) {
> + Method method = clazz.getMethod("teardownTest", new Class[]{JavaSamplerContext.class});
This does not deal with the case where a JavaSampler implementation
does not sub-class AbstractJavaSamplerClient and does not define
tearDownTest.
We need a test for that.
If the method is not found, clearly there is no need to register the class.
> + isToBeRegistered = Boolean.valueOf(!method.getDeclaringClass().equals(AbstractJavaSamplerClient.class));
> + isToBeRegisteredCache.put(clazz.getName(), isToBeRegistered);
> + }
> + return isToBeRegistered.booleanValue();
> }
>
> /**
> @@ -244,6 +300,7 @@ public class JavaSampler extends Abstrac
> }
> javaClientAndContextSet.clear();
> }
> + isToBeRegisteredCache.clear();
> }
>
> /* Implements TestStateListener.testEnded(String) */
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Sat Aug 25 13:17:19 2012
> @@ -135,6 +135,7 @@ Shortcut for Function Helper Dialog is n
> <ul>
> <li><bugzilla>55310</bugzilla> - TestAction should implement Interruptible</li>
> <li><bugzilla>53318</bugzilla> - Add Embedded URL Filter to HTTP Request Defaults Control </li>
> +<li><bugzilla>53782</bugzilla> - Enhance JavaSampler handling of JavaSamplerClient cleanup to use less memory</li>
> </ul>
>
> <h3>Controllers</h3>
>
> Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml
> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/component_reference.xml?rev=1377291&r1=1377290&r2=1377291&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/usermanual/component_reference.xml (original)
> +++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sat Aug 25 13:17:19 2012
> @@ -566,6 +566,9 @@ The fields allow variables to be used, s
> </p>
> </description>
>
> +<note>Since JMeter 2.8, if method teardownTest is not overriden by subclasses of AbstractJavaSamplerClient, then the method will not be called to optimise JMeter memory behaviour.
> +This will not have any impact on existing Test plans.
> +</note>
I don't think the note should be added to component reference (it will
just be confusing to readers); it belongs in changes.
> <note>The Add/Delete buttons don't serve any purpose at present.</note>
>
> <properties>
>
>