You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2018/06/04 05:58:10 UTC

[Bug 62426] New: Here are some Reduced ReportGenerator performance Code

https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

            Bug ID: 62426
           Summary: Here are some Reduced ReportGenerator performance Code
           Product: JMeter
           Version: 4.0
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Main
          Assignee: issues@jmeter.apache.org
          Reporter: 444104595@qq.com
  Target Milestone: ---

Importance from high to low:
1.
public class TransactionsPerSecondGraphConsumer extends
        AbstractOverTimeGraphConsumer
@Override
                    public Iterable<String> select(Sample sample) {
                        String label = String.format(STATUS_SERIES_FORMAT,
                                sample.getName(),
                                sample.getSuccess() ? SUCCESS_SERIES_SUFFIX
                                        : FAILURE_SERIES_SUFFIX);
                        return Arrays.asList(label);
                    }
String.format  can replace to "" + "";


2.
public class SyntheticResponseTimeDistributionGraphConsumer extends
        AbstractGraphConsumer {
                if(elapsedTime<=getSatisfiedThreshold()) {
                    return Arrays.asList(SATISFIED_LABEL.format(new Object[]
{Long.valueOf(getSatisfiedThreshold())}));
                } else if(elapsedTime <= getToleratedThreshold()) {
                    return Arrays.asList(TOLERATED_LABEL.format(new Object[]
{Long.valueOf(getSatisfiedThreshold()),
Long.valueOf(getToleratedThreshold())}));
                } else {
                    return Arrays.asList(UNTOLERATED_LABEL.format(new Object[]
{Long.valueOf(getToleratedThreshold())}));
                }
The same: String.format  to slow.

3.
public final class CSVSaveService {
    public static String[] csvReadFile(BufferedReader infile, char delim)
            throws IOException {
why not use like this:
        String str = infile.readLine();
        if (str == null) {
            return new String[]{};
        }
        return str.split("" + delim);
the code:
if ((ch == '\n' || ch == '\r') && state != ParserState.QUOTED) 
Consumes a lot of CPU time

4.
public class NormalizerSampleConsumer extends AbstractSampleConsumer {
public void consume(Sample s, int channel) {

I just return
super.produce(s, 0);

5.
public class SampleMetadata {
private TreeMap<String, Integer> index = new TreeMap<>();

why not use 
private HashMap<String, Integer> index = new HashMap<>();

6.
    public static <T> T convert(Class<T> clazz, String value)
            throws ConvertException {
why not just return like bellow:  

f (clazz.isAssignableFrom(String.class)) {
            return (T) value;
        }
        if (clazz.isAssignableFrom(Long.class) ||
clazz.isAssignableFrom(long.class)) {
            return (T) Long.valueOf(value);
        }
        if (clazz.isAssignableFrom(Boolean.class) ||
clazz.isAssignableFrom(boolean.class)) {
            return (T) Boolean.valueOf(value);
        }
        if (clazz.isAssignableFrom(Integer.class) ||
clazz.isAssignableFrom(int.class)) {
            return (T) Integer.valueOf(value);
        }
        if (clazz.isAssignableFrom(Double.class) ||
clazz.isAssignableFrom(double.class)) {
            return (T) Double.valueOf(value);
        }
        if (clazz.isAssignableFrom(Float.class) ||
clazz.isAssignableFrom(float.class)) {
            return (T) Float.valueOf(value);
        }
        if (clazz.isAssignableFrom(Character.class) ||
clazz.isAssignableFrom(char.class)) {
            return (T) Character.valueOf(value.charAt(0));
        }
        else {
            StringConverter<T> converter = Converters.getConverter(clazz);
            if (converter == null) {
                throw new ConvertException(value, clazz.getName());
            }
            result = converter.convert(value);
        }


My performance can be improved by at least 50%

The original code may be considered for special situations, but the default
generated report file, I have no difference between the test before and after
the modification

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
           Hardware|PC                          |All

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |p.mouawad@ubik-ingenierie.c
                   |                            |om

--- Comment #10 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
(In reply to Felix Schumacher from comment #9)
> With the attached patches my laptop processes the test files in 1:18 instead
> of 1:58.
> 
Nice !

> It should be noted, that report generation is single threaded. It might be
> interesting to see, if decoupling reading of csv and aggregating them into
> the different report classes into different threads might help.

Yes, maybe we should see which step is the most consuming.
Isn't it  initial sorting ?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #7 from Felix Schumacher <fe...@internetallee.de> ---
Created attachment 35973
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=35973&action=edit
Simple replacement for String#format

Simple (maybe too simple) implementation of a pre-compiled version of
String#format that can be used to replace the really simple variants that are
used in the report generation process.

It is about 10 times faster than String#format, but slower than concatenation.

Benchmark                          Mode  Cnt         Score         Error  Units
MyBenchmark.concatenate           thrpt    5  69766343,254 ± 2020598,233  ops/s
MyBenchmark.preCompiledFormatter  thrpt    5  21432505,044 ±  563230,597  ops/s
MyBenchmark.stringFormat          thrpt    5   1665297,020 ±   34538,703  ops/s

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #4 from Felix Schumacher <fe...@internetallee.de> ---
(In reply to Allen from comment #0)
> Importance from high to low:
> 1.
> public class TransactionsPerSecondGraphConsumer extends
>         AbstractOverTimeGraphConsumer
> @Override
>                     public Iterable<String> select(Sample sample) {
>                         String label = String.format(STATUS_SERIES_FORMAT,
>                                 sample.getName(),
>                                 sample.getSuccess() ? SUCCESS_SERIES_SUFFIX
>                                         : FAILURE_SERIES_SUFFIX);
>                         return Arrays.asList(label);
>                     }
> String.format  can replace to "" + "";

This would be possible and probably faster.

> 
> 
> 2.
> public class SyntheticResponseTimeDistributionGraphConsumer extends
>         AbstractGraphConsumer {
>                 if(elapsedTime<=getSatisfiedThreshold()) {
>                     return Arrays.asList(SATISFIED_LABEL.format(new Object[]
> {Long.valueOf(getSatisfiedThreshold())}));
>                 } else if(elapsedTime <= getToleratedThreshold()) {
>                     return Arrays.asList(TOLERATED_LABEL.format(new Object[]
> {Long.valueOf(getSatisfiedThreshold()),
> Long.valueOf(getToleratedThreshold())}));
>                 } else {
>                     return Arrays.asList(UNTOLERATED_LABEL.format(new
> Object[] {Long.valueOf(getToleratedThreshold())}));
>                 }
> The same: String.format  to slow.

String#format is slow, but simple concatenation doesn't help here, as the
labels are used for localization.

We probably would need a pre-compiled String#format mechanism, that would be
initialized ones and reused afterwards. Sadly, that is not possible with the
standard implementation of String#format.

> 
> 3.
> public final class CSVSaveService {
>     public static String[] csvReadFile(BufferedReader infile, char delim)
>             throws IOException {
> why not use like this:
>         String str = infile.readLine();
>         if (str == null) {
>             return new String[]{};
>         }
>         return str.split("" + delim);
> the code:
> if ((ch == '\n' || ch == '\r') && state != ParserState.QUOTED) 
> Consumes a lot of CPU time

This one is needed, as the CSV file may contain new lines inside of quoted
text. If we remove it, we can not read in all of our self generated csv files.

> 
> 4.
> public class NormalizerSampleConsumer extends AbstractSampleConsumer {
> public void consume(Sample s, int channel) {
> 
> I just return
> super.produce(s, 0);
> 
> 5.
> public class SampleMetadata {
> private TreeMap<String, Integer> index = new TreeMap<>();
> 
> why not use 
> private HashMap<String, Integer> index = new HashMap<>();
> 
> 6.
>     public static <T> T convert(Class<T> clazz, String value)
>             throws ConvertException {
> why not just return like bellow:  
> 
> f (clazz.isAssignableFrom(String.class)) {
>             return (T) value;
>         }
>         if (clazz.isAssignableFrom(Long.class) ||
> clazz.isAssignableFrom(long.class)) {
>             return (T) Long.valueOf(value);
>         }
>         if (clazz.isAssignableFrom(Boolean.class) ||
> clazz.isAssignableFrom(boolean.class)) {
>             return (T) Boolean.valueOf(value);
>         }
>         if (clazz.isAssignableFrom(Integer.class) ||
> clazz.isAssignableFrom(int.class)) {
>             return (T) Integer.valueOf(value);
>         }
>         if (clazz.isAssignableFrom(Double.class) ||
> clazz.isAssignableFrom(double.class)) {
>             return (T) Double.valueOf(value);
>         }
>         if (clazz.isAssignableFrom(Float.class) ||
> clazz.isAssignableFrom(float.class)) {
>             return (T) Float.valueOf(value);
>         }
>         if (clazz.isAssignableFrom(Character.class) ||
> clazz.isAssignableFrom(char.class)) {
>             return (T) Character.valueOf(value.charAt(0));
>         }
>         else {
>             StringConverter<T> converter = Converters.getConverter(clazz);
>             if (converter == null) {
>                 throw new ConvertException(value, clazz.getName());
>             }
>             result = converter.convert(value);
>         }
> 
> 
> My performance can be improved by at least 50%
> 
> The original code may be considered for special situations, but the default
> generated report file, I have no difference between the test before and
> after the modification

Do you have transaction samplers in your csv sample files?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #6 from Felix Schumacher <fe...@internetallee.de> ---
Created attachment 35972
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=35972&action=edit
Script to generate a jtl file that could be used for performance testing

Simple python script that generates a jtl file with 5 million lines which
equals to about 460 MB.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #11 from Felix Schumacher <fe...@internetallee.de> ---
Date: Sun Jul  8 11:45:32 2018
New Revision: 1835351

URL: http://svn.apache.org/viewvc?rev=1835351&view=rev
Log:
Optimize performance of report generation.

Compute things that don't change, but get used often eagerly.
Don't use String#format for simple concatenation, as it is really slow.
Based on feedback by Allen (444104595 at qq.com)

Bugzilla Id: 62426

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/report/core/Sample.java
   
jmeter/trunk/src/core/org/apache/jmeter/report/processor/graph/impl/SyntheticResponseTimeDistributionGraphConsumer.java
   
jmeter/trunk/src/core/org/apache/jmeter/report/processor/graph/impl/TransactionsPerSecondGraphConsumer.java
    jmeter/trunk/xdocs/changes.xml

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #5 from Vladimir Sitnikov <si...@gmail.com> ---
The suggested fixes do not look like to result in "50% speedup with no
drawbacks".


I agree SampleMetadata.index should be changed from TreeMap to HashMap, however
it should not be that important though. The field type should be just Map
interface, not the class type.


Regarding the other changes, I would like to see profiling results. I'm
inclined to decline "performance" fixes unless there's justification. 


>So I just suggest that there is a csv file that can be judged to be generated by default. If it is a simple csv file, then use a simple test report generation program.


I agree, there should be a test CSV file that is used to benchmark report
generator. However, I do not have such a file.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #2 from Allen <44...@qq.com> ---
(In reply to Vladimir Sitnikov from comment #1)
> Allen, thanks for the suggestions, however could you please provide a
> reproducible test case so the performance fixes can be validated?
> 
> Do you have profiling results that quantify the impact of the issues you
> mention?
Thanks for the reply, using simple textual descriptions to prove that
performance improvement is really difficult. 

I use the default condition generated csv result file as a test case file,
which is greater than 900MB, and my local CPU is i7-6600U, memory is 16GB,
window test command is :  Measure-Command {.\jmeter.bat-g D: \case.csv -o
D:\output01}.

you can observe the code is different, generate test report time will change. 

In my optimization proposal, except that the modifications of the
CSVSaveService and NormalizerSampleConsumer are more radical, other suggestions
are based on the language features of the Java code itself. 
The results are the same, and the performance is definitely improved. 

For the modifications of the CSVSaveService class and the
NormalizerSampleConsumer class, I only tested the csv file generated from the
default conditions. I believe that the code you write is so complicated and
there must be a reason, so I can't be 100% certain that my changes cover all
cases. 
For example, include special characters and so on. 

So I just suggest that there is a csv file that can be judged to be generated
by default. If it is a simple csv file, then use a simple test report
generation program. 

If you need a very comprehensive test basis to prove that the modifications of
the CSVSaveService class and the NormalizerSampleConsumer class are harmless, I
really don't have this. Because the csv test file I generated cannot cover all
the scenarios and conditions.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

Vladimir Sitnikov <si...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

--- Comment #1 from Vladimir Sitnikov <si...@gmail.com> ---
Allen, thanks for the suggestions, however could you please provide a
reproducible test case so the performance fixes can be validated?

Do you have profiling results that quantify the impact of the issues you
mention?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #8 from Felix Schumacher <fe...@internetallee.de> ---
Created attachment 35974
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=35974&action=edit
Eager initialization of fields in sample

A few more seconds can be gained by not recalculating data in the samples.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #9 from Felix Schumacher <fe...@internetallee.de> ---
With the attached patches my laptop processes the test files in 1:18 instead of
1:58.

It should be noted, that report generation is single threaded. It might be
interesting to see, if decoupling reading of csv and aggregating them into the
different report classes into different threads might help.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62426] Here are some Reduced ReportGenerator performance Code

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62426

--- Comment #3 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
*** Bug 62425 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are the assignee for the bug.