Bug 54453 - Performance enhancements : Replace Random by ThreadLocalRandom in __Random function
Summary: Performance enhancements : Replace Random by ThreadLocalRandom in __Random fu...
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: 2.8
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-21 11:21 UTC by Philippe Mouawad
Modified: 2015-04-16 19:15 UTC (History)
2 users (show)



Attachments
Patch that introduces ThreadLocalRandom (8.39 KB, patch)
2014-10-19 21:19 UTC, Philippe Mouawad
Details | Diff
Scalability of ${__Random} function (27.80 KB, image/png)
2014-11-10 10:47 UTC, Mikhail Epikhin
Details
${__Random}-benchmark testplan (6.65 KB, text/plain)
2014-11-10 10:49 UTC, Mikhail Epikhin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Mouawad 2013-01-21 11:21:59 UTC
Hello,
Java 7 introduced ThreadLocalRandom which has much better performances than using Random as static instance.
See:

http://docs.oracle.com/javase/tutorial/essential/concurrency/threadlocalrandom.html
http://niklasschlimm.blogspot.fr/2012/01/java-7-how-to-write-really-fast-java.html


Implementation is here for Java6 but can we use it with Apache License ?:
https://github.com/codahale/metrics/blob/master/metrics-core/src/main/java/com/yammer/metrics/stats/ThreadLocalRandom.java

Or should we migrate to Java 7 as Java6 will be soon EOL ?
Comment 1 Mikhail Epikhin 2013-04-02 07:02:40 UTC
It's very good idea, but many of people uses jdk version 1.6 and less:(
Comment 2 Philippe Mouawad 2013-04-02 19:15:57 UTC
Yes, but if implemented we would backport class, so no problem for JDK6.
Comment 3 Mikhail Epikhin 2013-04-02 20:29:10 UTC
Hmm. Did you have real tests with bottleneck on Random?
Comment 4 Philippe Mouawad 2013-08-15 21:31:43 UTC
Tests do not show any significant improvement, so closing.
Comment 5 Philippe Mouawad 2014-10-19 21:19:45 UTC
Created attachment 32129 [details]
Patch that introduces ThreadLocalRandom
Comment 6 Mikhail Epikhin 2014-11-10 10:45:36 UTC
Hello, Philippe!

Your patch is good, but why not use "rand = ThreadLocalRandom.current().nextLong(min, max);" instead of "rand = min + (ThreadLocalRandom.current().nextLong(max - min + 1));" ?
 It's more clearly and readability.

I tested this patch on GNU/Linux with 4 JDK (hs_jdk1.7.0_71, hs_jdk1.8.0_25, hs_jdk1.9.0_ea_b37, zulu_1.8.0_25-8.4.0.1-x86lx64). All works fine.
I created some benchmarks with 1 dummy sampler (from jmeter-plugins) and 100 ${__Random} functions in him (for more powerful amplification), and tested scalability on machine with large number of cores.

Some time before you removed synchronized block from ${__Random} function https://github.com/apache/jmeter/commit/f03a8bdbe5ba31d9f42adf816887c714cd8c7dce#diff-d388a962eb391a4d5b119abef27b819dL72
 And this commit improves a little bit performance. But scalability is not so good. With large number of threads we have new contention. Previous contention on synchronized block, and new on Math.random block.

At all tests with large number of threads i have many threads spending on this stacks:

"Thread Group 1-12" #33 prio=5 os_prio=0 tid=0x00007fc2cc039800 nid=0x1677 runnable [0x00007fc3290a8000]
   java.lang.Thread.State: RUNNABLE
--->    at java.util.Random.next(Random.java:204)
--->    at java.util.Random.nextDouble(Random.java:532)
--->    at java.lang.Math.random(Math.java:773)
        at org.apache.jmeter.functions.Random.execute(Random.java:65)
        at org.apache.jmeter.engine.util.CompoundVariable.execute(CompoundVariable.java:142)
        at org.apache.jmeter.engine.util.CompoundVariable.execute(CompoundVariable.java:118)
        at org.apache.jmeter.testelement.property.FunctionProperty.getStringValue(FunctionProperty.java:101)
        at org.apache.jmeter.testelement.AbstractTestElement.getPropertyAsString(AbstractTestElement.java:274)
        at kg.apc.jmeter.samplers.DummySampler.getRequestData(DummySampler.java:111)
        at kg.apc.jmeter.samplers.DummySampler.sample(DummySampler.java:38)
        at org.apache.jmeter.threads.JMeterThread.process_sampler(JMeterThread.java:431)
        at org.apache.jmeter.threads.JMeterThread.run(JMeterThread.java:258)
        at java.lang.Thread.run(Thread.java:745)

I looked at HotSpot JDK1.8 sources, and found that we have contention on Compare-and-Swap at "seed" atomic value.

     198     protected int next(int bits) {
     199         long oldseed, nextseed;
     200         AtomicLong seed = this.seed;
     201         do {
     202             oldseed = seed.get();
     203             nextseed = (oldseed * multiplier + addend) & mask;
---> 204         } while (!seed.compareAndSet(oldseed, nextseed));
     205         return (int)(nextseed >>> (48 - bits));

With your ThreadLocal patch we just sharded this atomic seed values. So, throughput scales good:) It's linearly scales with number of threads!

What do your think about include this patch into Apache JMeter 2.12?
Comment 8 Mikhail Epikhin 2014-11-10 10:49:12 UTC
Created attachment 32202 [details]
${__Random}-benchmark testplan
Comment 9 Philippe Mouawad 2014-11-10 13:53:36 UTC
Thanks Mikhail for your tests.
I will commit it for next 2.13 or maybe 3.0 :-)
Comment 10 Philippe Mouawad 2014-11-10 14:40:33 UTC
Date: Mon Nov 10 14:39:40 2014
New Revision: 1637877

URL: http://svn.apache.org/r1637877
Log:
Bug 54453 - Performance enhancements : Replace Random by ThreadLocalRandom
Bugzilla Id: 54453

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/util/JMeterVersion.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/Random.java
    jmeter/trunk/xdocs/changes.xml
    jmeter/trunk/xdocs/changes_history.xml
Comment 11 Andrey Pokhilko 2015-04-16 10:51:23 UTC
This fix introduced issue, on Jmeter 2.12 function __random(1,1) works, on 2.13 now works. It is because the change is:
-        long rand = min + (long) (Math.random() * (max - min + 1));
+        long rand = ThreadLocalRandom.current().nextLong(min, max);

and new version lost "+1" boundary shift...
Comment 12 Philippe Mouawad 2015-04-16 19:15:47 UTC
Created 57825 for this.