Latency utils by shaqianqian · Pull Request #50 · ubikloadpack/jmeter

shaqianqian · 2019-06-25T15:46:40Z

Description

use LatencyUtil to calculate the mean and percentile of datas

How Has This Been Tested?

Yes

update the version of jmeter

ubikloadpack · 2019-07-05T11:48:14Z

        mySB.append("Max: " + this.getMax() + "  ");
        mySB.append("Error Rate: " + this.getErrorPercentage() + "  ");
        mySB.append("Sample Rate: " + this.getRate());
+        mySB.append("elapse: " + this.getElapsed());


Modify 'elapse:' to 'elapsed:'

ubikloadpack · 2019-07-05T11:51:16Z

+    }
+
+    @Override
+    public void clear() {


Shouldn't you be clearing/resetting latencyStats ? and also call stop on object to make threads stop ?

ubikloadpack · 2019-07-05T11:55:22Z

 */
 public class StatisticsSummaryData {
-
+    LatencyStats latencyStats = new LatencyStats(1,3600000000000L,2,1024,10000000000L,null);


Why is PauseDetector null ? We want this feature

Is the window 1024 big enough ? What is the logic behind ? Make it a configurable property

intervalEstimatorWindowLength is 10000000000L, What is the logic behind ?

ubikloadpack · 2019-07-05T11:56:15Z

+
+    void addAll(IStatCalculator<T> calc);
+
+    T getMedian();


Document all interface methods

ubikloadpack · 2019-07-05T11:59:29Z

+    @Override
+    public Map<Number, Number[]> getDistribution() {
+        Map<Number, Number[]> result = new HashMap<>();
+        histogram.percentiles(5).forEach(p -> {


vlsi · 2019-07-16T07:20:28Z

@@ -0,0 +1,27 @@
+BSD License
+
+Copyright (c) 2000-2015 www.hamcrest.org


Are you sure HdrHistogram is copyrighted by hamcrest.org?

vlsi · 2019-07-16T07:20:50Z

@@ -0,0 +1,27 @@
+BSD License
+
+Copyright (c) 2000-2015 www.hamcrest.org


Are you sure LatencyUtils is copyrighted by hamcrest?

vlsi · 2019-07-16T07:22:17Z

-
+    private LatencyStats latencyStats = LatencyStats.Builder.create().build();
+    Histogram intervalHistogram = latencyStats.getIntervalHistogram();
+    private Histogram histogram = new Histogram(intervalHistogram);


What's the precision of the resulting histogram?
What's the memory consumption?

This is work in progress. See question asked on dev list also

vlsi · 2019-07-16T07:27:24Z

+        min = Math.min(val, min);
+        updateValueCount(val,1);
+    }
+    private void updateValueCount(Long actualValue, long sampleCount) {


This is sad. Is it really required?

It is a method for calculating distribution, so I try to record the value when it is added . Can you give another idea? Thanks

What's the purpose of valuesMap?
Histogram / LatencyStats should track the distribution.

In fact in the old code, the distribution value comes from this method :

public Map<Number, Number[]> getDistribution() { Map<Number, Number[]> items = new HashMap<>(); for (Entry<T, MutableLong> entry : valuesMap.entrySet()) { Number[] dis = new Number[2]; dis[0] = entry.getKey(); dis[1] = entry.getValue(); items.put(entry.getKey(), dis); } return items; }

I don't think the LatencyStats can track this distribution

vlsi · 2019-07-16T07:29:00Z

+            histogram.recordValue(l*1000000L);
+        }
+        // test max/min
+        assertTrue(10 == statisticsSummaryData.getMax());


Please avoid writing tests like that.
It would be hard to analyze especially at Travis/Jenkins CI.

Please use something like assertEquals(Arrays.toString(values) + ".getMax()", 10, statisticsSummaryData.getMax())

General rule: assertTrue and assertFalse are almost always bad.

vlsi · 2019-07-16T07:30:11Z

+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHisStatCalculator {


Please avoid use his / her wording

his => Histogram
But better use TestHistogramStatCalculator

vlsi · 2019-07-16T07:30:29Z

+            calc1.addValue(l);
+        }
+        calc.addAll(calc1);
+        assertTrue(23 == calc.getMax());


Please avoid assertTrue

vlsi · 2019-07-16T07:31:18Z

 * asm-7.0.jar (BSD)
 * dnsjava-2.1.8.jar (BSD)
 * hamcrest-core-1.3.jar (BSD)
+* LatencyUtils-2.0.3.jar (BSD)


BSD is an invalid (too broad) license reference. Please use SPDX license ID when possible: https://spdx.org/licenses/

https://github.com/LatencyUtils/LatencyUtils/blob/master/LICENSE
https://github.com/HdrHistogram/HdrHistogram/blob/master/LICENSE.txt
These two licenses are both in BSD, what can I do to use SPDX License ? Thanks

vlsi · 2019-07-16T07:33:17Z

 # are contained in the JMeter binary release.

 maven2.repo                 = https://repo1.maven.org/maven2
+maven.repo                  = https://repo.maven.apache.org/maven2


AFAIK, LatencyUtils is available in Maven Central. Why do you add maven.repo?

vlsi

I think methods like addSentBytes, addBytes, getTotalBytes, getTotalSentBytes do not belong to a generic IStatCalculator interface, and the methods should be removed from the interface.

vlsi · 2019-07-16T07:36:03Z

+     * 
+     * @param newValue number of newly sent bytes
+     */
+    void addSentBytes(long newValue);


This looks like a violation of "single responsibility principle".
How getStandardDeviation is related to addSentBytes?

I think methods like addSentBytes, addBytes, getTotalBytes, getTotalSentBytes do not belong to a generic IStatCalculator interface.

Is "getStandardDeviation" related to sentBytes? In my view the standardDeviation is for the response time of Http Request, and "sent bytes" is for the amount of data used for transmission

ubikloadpack · 2019-07-16T07:48:17Z

-        percentile3 = new PercentileAggregator(percentileIndex3);
-        mean = new MeanAggregator();
+    public final long getPercentile(double percent) {
+        return histogram.getValueAtPercentile(percent)/1000000;


This should not be needed as per https://stackoverflow.com/questions/56986503/time-unit-problem-about-latencyutils-recordlatency-method

The only answer in thie question doesn't give us enough information.
So don't we need to muliple and divide by 1,000,000 each time?

When the number of Thread is a big number like 1000 -10000, the maximum is bigger than percentile 99%. And IntcountHistogram can also give a correct result.

…e assertTrue to assertEquals

shaqianqian added 18 commits April 1, 2019 20:03

Merge pull request #1 from ubikloadpack/trunk

296049d

update the version of jmeter

import packqge

545f8a1

import packqge

4224ccc

tot

8ce628e

add HdrHistogram

d4f2ee3

init latencyUtils

ea64d31

use latencyUtil to replace Hd

cb6912e

replace StatisticsSummaryData by latencyUtil

ee9e805

add comment apache for new class

ba56cad

add comment apache for new class

210cc43

add comment apache for new class

a30ad92

remove System.out

cc229f0

add espace in the fin of file

32d7487

add espace in the fin of file

9664943

update the version of Histogram

1be0644

update the version of Histogram

9264161

remove final

63a1f2e

modify json

168e937

ubikloadpack requested changes Jul 5, 2019

View reviewed changes

shaqianqian added 7 commits July 8, 2019 17:50

modify latency

8b5302b

change method to private

3663437

modify 'gitDistribuion()'

ba10bc6

add junit test for statisummarydata

b8e4ef6

change method to private

f9f7dfb

modify the parameters of LatencyUtils

821c0de

modify Latencyutils

0e2f2a3

vlsi suggested changes Jul 16, 2019

View reviewed changes

ubikloadpack reviewed Jul 16, 2019

View reviewed changes

modify the licenses,remove uselesses methods of IStatCalculate, chang…

6de2f12

…e assertTrue to assertEquals

update license of LatencyUtils and HdrHistrogram

d1fbb52

		@@ -0,0 +1,27 @@
		BSD License

		Copyright (c) 2000-2015 www.hamcrest.org

Conversation

shaqianqian commented Jun 25, 2019

Description

How Has This Been Tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shaqianqian Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vlsi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shaqianqian Jul 16, 2019 •

edited

Loading