Bug 57114

Summary:	Performance : Functions that only have values as instance variable should not synchronize execute
Product:	JMeter - Now in Github	Reporter:	Philippe Mouawad <p.mouawad>
Component:	Main	Assignee:	JMeter issues mailing list <issues>
Status:	RESOLVED FIXED
Severity:	enhancement	CC:	p.mouawad, sitnikov.vladimir
Priority:	P2
Version:	Nightly (Please specify date)
Target Milestone:	---
Hardware:	All
OS:	All
Attachments:	Avoid synchronization where it is not required

Description Philippe Mouawad 2014-10-18 12:28:20 UTC

As per Ubik Load Pack report:
-----------------------------------------------------------------------------
Reviewing code of Functions in JMeter we don't understand why Functions that only have values as instance variable (so Beanshell or StringFromFile or IterationCounter (which could be improved to avoid it) are not concerned) synchronize on execute.

It appears "values" instance variable is initialized within StandardJMeterEngine thread and then only accessed in read mode by JMeterThread

But it is true that if synchronized is removed then CompoundVariable is accessed by multi threads but it seems it uses thread related data so it should be OK.


-----------------------------------------------------------------------------

And as per dev mailing list :
http://mail-archives.apache.org/mod_mbox/jmeter-dev/201410.mbox/%3CCAB%3DJe-Gy4tp%3DMPih292oG1XpH7D1X0%2BUR%2BUa_rnLyXaUH-fePA%40mail.gmail.com%3E

Comment 1 Vladimir Sitnikov 2014-10-18 14:16:46 UTC

Created attachment 32124 [details]
Avoid synchronization where it is not required

I went through sub-classes of AbstractFunction and removed synchronization where it is not required.
Basically, if a function does not modify state and if it accesses thread-safe objects only, then synchronization is not required.

Please find the attached patch on top of e9228ccf / trunk@1632410.

Notes:
0) The patch contains just synchronization removal + some corrections to CV. All the tests pass except testPropfile(org.apache.jmeter.save.TestSaveService)junit.framework.AssertionFailedError: Property File Version mismatch, ensure you update SaveService#FILEVERSION field with revision id of saveservice.properties.

testPropfile fails without patch as well, so I did not investigate that.

1) I have no idea what to do with BeanShell function. bsh.Interpreter is thread safe, however bshInterpreter.set("vars",...) should not be accessed concurrently as that will just overwrite the value.
I think it makes sense to wrap bsh interpreter in a ThreadLocal, so synchronization can be removed from BeanShell.execute

Comment 2 Philippe Mouawad 2014-10-19 19:57:12 UTC

Date: Sun Oct 19 19:53:07 2014
New Revision: 1632983

URL: http://svn.apache.org/r1632983
Log:
Bug 57114 - Performance : Functions that only have values as instance variable should not synchronize execute
Bugzilla Id: 57114

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/engine/util/CompoundVariable.java
    jmeter/trunk/src/core/org/apache/jmeter/functions/AbstractFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/AbstractHostIPName.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/CSVRead.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/CharFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/EscapeHtml.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/EscapeOroRegexpChars.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/EvalFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/EvalVarFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/FileToString.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/IntSum.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/IterationCounter.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/JavaScript.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/Jexl2Function.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/JexlFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/LogFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/LogFunction2.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/LongSum.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/Property.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/Property2.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/Random.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/RandomString.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/RegexFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/SamplerName.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/SetProperty.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/SplitFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/TestPlanName.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/TimeFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/UnEscape.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/UnEscapeHtml.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/UrlDecode.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/UrlEncode.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/Variable.java
    jmeter/trunk/xdocs/changes.xml

Comment 3 Philippe Mouawad 2014-10-27 13:44:22 UTC

Date: Mon Oct 27 13:41:47 2014
New Revision: 1634535

URL: http://svn.apache.org/r1634535
Log:
Bug 57114 - Performance : Functions that only have values as instance variable should not synchronize execute
Remove synchronized on setParameters
Bugzilla Id: 57114

Modified:
    jmeter/trunk/src/functions/org/apache/jmeter/functions/EscapeOroRegexpChars.java


Date: Mon Oct 27 13:43:01 2014
New Revision: 1634537

URL: http://svn.apache.org/r1634537
Log:
Bug 57114 - Performance : Functions that only have values as instance variable should not synchronize execute
Remove old TODO
Bugzilla Id: 57114

Modified:
    jmeter/trunk/src/functions/org/apache/jmeter/functions/SamplerName.java


Date: Mon Oct 27 13:42:36 2014
New Revision: 1634536

URL: http://svn.apache.org/r1634536
Log:
Bug 57114 - Performance : Functions that only have values as instance variable should not synchronize execute
Keep synchronized where it is needed
Bugzilla Id: 57114

Modified:
    jmeter/trunk/src/functions/org/apache/jmeter/functions/LogFunction.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/LogFunction2.java

Comment 4 The ASF infrastructure team 2022-09-24 20:37:58 UTC

This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/3460