Skip to content

Commit 787eef3

Browse files
DieBauerfstab
authored andcommitted
Improve CKMSQuantiles and address memory leak
CKMSQuantiles is copied from an implementation of 2012, where it states that a ‘HACK’ was done, admitting a space leak. This leak has been noticed several times (#422, #550, #654). By correctly applying the algorithm from the paper we fix the leak. I have added unit-tests to show that the behaviour is correct. I have also added a Benchmark in the benchmark module showing the difference with the old and current implementation. According to my benchmarks, is in the new implementation a `get` of a quantile that has ‘seen’ 1 million elements 440 times faster. Inserting 1 million elements is 3.5 times faster. While going through the CKMS paper and the Java implementation I have added remarks and snippets from the paper, to clarify why certain choices are made. Signed-off-by: Jens <jenskat@gmail.com> Fix assertion in HistogramTest Median of 1..11 = 6 Signed-off-by: Jens <jenskat@gmail.com> Address PR remarks Signed-off-by: Jens <jenskat@gmail.com>
1 parent c83877a commit 787eef3

File tree

6 files changed

+576
-108
lines changed

6 files changed

+576
-108
lines changed

benchmarks/pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@
2727
<dependency>
2828
<groupId>org.openjdk.jmh</groupId>
2929
<artifactId>jmh-core</artifactId>
30-
<version>1.3.2</version>
30+
<version>1.34</version>
3131
</dependency>
3232
<dependency>
3333
<groupId>org.openjdk.jmh</groupId>
3434
<artifactId>jmh-generator-annprocess</artifactId>
35-
<version>1.3.2</version>
35+
<version>1.34</version>
3636
</dependency>
3737
<dependency>
3838
<groupId>javax.annotation</groupId>
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package io.prometheus.client;
2+
3+
import io.prometheus.client.CKMSQuantiles.Quantile;
4+
import org.openjdk.jmh.annotations.*;
5+
import org.openjdk.jmh.infra.Blackhole;
6+
import org.openjdk.jmh.runner.Runner;
7+
import org.openjdk.jmh.runner.RunnerException;
8+
import org.openjdk.jmh.runner.options.Options;
9+
import org.openjdk.jmh.runner.options.OptionsBuilder;
10+
11+
import java.util.*;
12+
import java.util.concurrent.TimeUnit;
13+
14+
public class CKMSQuantileBenchmark {
15+
16+
@State(Scope.Benchmark)
17+
public static class EmptyBenchmarkState {
18+
@Param({"10000", "100000", "1000000"})
19+
public int value;
20+
21+
CKMSQuantiles ckmsQuantiles;
22+
23+
List<Quantile> quantiles;
24+
Random rand = new Random(0);
25+
26+
long[] shuffle;
27+
28+
@Setup(Level.Trial)
29+
public void setup() {
30+
quantiles = new ArrayList<Quantile>();
31+
quantiles.add(new Quantile(0.50, 0.050));
32+
quantiles.add(new Quantile(0.90, 0.010));
33+
quantiles.add(new Quantile(0.95, 0.005));
34+
quantiles.add(new Quantile(0.99, 0.001));
35+
36+
37+
shuffle = new long[value];
38+
for (int i = 0; i < shuffle.length; i++) {
39+
shuffle[i] = i;
40+
}
41+
Collections.shuffle(Arrays.asList(shuffle), rand);
42+
43+
}
44+
}
45+
46+
@Benchmark
47+
@BenchmarkMode({Mode.AverageTime})
48+
@OutputTimeUnit(TimeUnit.MILLISECONDS)
49+
public void ckmsQuantileInsertBenchmark(Blackhole blackhole, EmptyBenchmarkState state) {
50+
CKMSQuantiles q = new CKMSQuantiles(state.quantiles.toArray(new Quantile[]{}));
51+
for (long l : state.shuffle) {
52+
q.insert(l);
53+
}
54+
}
55+
56+
@State(Scope.Benchmark)
57+
public static class PrefilledBenchmarkState {
58+
public int value = 1000000;
59+
60+
CKMSQuantiles ckmsQuantiles;
61+
62+
List<Quantile> quantiles;
63+
Random rand = new Random(0);
64+
65+
long[] shuffle;
66+
67+
@Setup(Level.Trial)
68+
public void setup() {
69+
quantiles = new ArrayList<Quantile>();
70+
quantiles.add(new Quantile(0.50, 0.050));
71+
quantiles.add(new Quantile(0.90, 0.010));
72+
quantiles.add(new Quantile(0.95, 0.005));
73+
quantiles.add(new Quantile(0.99, 0.001));
74+
75+
76+
shuffle = new long[value];
77+
for (int i = 0; i < shuffle.length; i++) {
78+
shuffle[i] = i;
79+
}
80+
Collections.shuffle(Arrays.asList(shuffle), rand);
81+
ckmsQuantiles = new CKMSQuantiles(quantiles.toArray(new Quantile[]{}));
82+
for (long l : shuffle) {
83+
ckmsQuantiles.insert(l);
84+
}
85+
86+
}
87+
}
88+
89+
@Benchmark
90+
@BenchmarkMode({Mode.AverageTime})
91+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
92+
public void ckmsQuantileGetBenchmark(Blackhole blackhole, PrefilledBenchmarkState state) {
93+
blackhole.consume(state.ckmsQuantiles.get(0.95));
94+
}
95+
96+
public static void main(String[] args) throws RunnerException {
97+
98+
Options opt = new OptionsBuilder()
99+
.include(CKMSQuantileBenchmark.class.getSimpleName())
100+
.warmupIterations(5)
101+
.measurementIterations(4)
102+
.threads(1)
103+
.forks(1)
104+
.build();
105+
106+
new Runner(opt).run();
107+
}
108+
}

simpleclient/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,11 @@
5454
<version>4.13.2</version>
5555
<scope>test</scope>
5656
</dependency>
57+
<dependency>
58+
<groupId>org.apache.commons</groupId>
59+
<artifactId>commons-math3</artifactId>
60+
<version>3.2</version>
61+
<scope>test</scope>
62+
</dependency>
5763
</dependencies>
5864
</project>

0 commit comments

Comments
 (0)