Skip to content

Commit

Permalink
Define a custom Scope Key for reducing memory allocations (#116)
Browse files Browse the repository at this point in the history 
* save progress

* revert to master

* sync with master

* add gradle files

* gradle cleanup

* add unit tests and cleanup code

* self review

* self review

* review comments address

* fix flaky test as pointed in review

* add benchmark

* self review

* nit pick
  • Loading branch information
@sairamch04
sairamch04 authored Feb 13, 2023
1 parent 23c39f9 commit fa530e2
Show file tree
Hide file tree
Showing 12 changed files with 263 additions and 128 deletions.
44 changes: 35 additions & 9 deletions core/benchmark-tests.txt
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,35 @@
Benchmark Mode Cnt Score Error Units
ScopeImplBenchmark.scopeReportingBenchmark thrpt 10 1385.711 ± 55.291 ops/ms
ScopeImplBenchmark.scopeReportingBenchmark:·async thrpt NaN ---
ScopeImplBenchmark.scopeReportingBenchmark:·gc.alloc.rate thrpt 10 ≈ 10⁻⁴ MB/sec
ScopeImplBenchmark.scopeReportingBenchmark:·gc.alloc.rate.norm thrpt 10 ≈ 10⁻⁴ B/op
ScopeImplBenchmark.scopeReportingBenchmark:·gc.count thrpt 10 ≈ 0 counts
ScopeImplBenchmark.scopeReportingBenchmark:·threads.alive thrpt 10 5.800 ± 0.637 threads
ScopeImplBenchmark.scopeReportingBenchmark:·threads.daemon thrpt 10 4.000 ± 0.001 threads
ScopeImplBenchmark.scopeReportingBenchmark:·threads.started thrpt 10 26.000 threads
Benchmark Mode Cnt Score Error Units
ScopeImplBenchmark.scopeReportingBenchmark thrpt 10 1345.606 ± 129.913 ops/ms
ScopeImplBenchmark.scopeReportingBenchmark:·async thrpt NaN ---
ScopeImplBenchmark.scopeReportingBenchmark:·gc.alloc.rate thrpt 10 ≈ 10⁻⁴ MB/sec
ScopeImplBenchmark.scopeReportingBenchmark:·gc.alloc.rate.norm thrpt 10 ≈ 10⁻⁴ B/op
ScopeImplBenchmark.scopeReportingBenchmark:·gc.count thrpt 10 ≈ 0 counts
ScopeImplBenchmark.scopeReportingBenchmark:·threads.alive thrpt 10 5.800 ± 0.637 threads
ScopeImplBenchmark.scopeReportingBenchmark:·threads.daemon thrpt 10 4.000 ± 0.001 threads
ScopeImplBenchmark.scopeReportingBenchmark:·threads.started thrpt 10 26.000 threads
ScopeImplBenchmark.scopeTaggedBenchmark thrpt 10 2440.881 ± 73.028 ops/ms
ScopeImplBenchmark.scopeTaggedBenchmark:·async thrpt NaN ---
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.alloc.rate thrpt 10 1947.506 ± 58.647 MB/sec
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.alloc.rate.norm thrpt 10 880.000 ± 0.001 B/op
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.churn.G1_Eden_Space thrpt 10 1940.575 ± 57.874 MB/sec
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.churn.G1_Eden_Space.norm thrpt 10 876.878 ± 5.246 B/op
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.churn.G1_Old_Gen thrpt 10 0.168 ± 0.001 MB/sec
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.churn.G1_Old_Gen.norm thrpt 10 0.076 ± 0.002 B/op
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.count thrpt 10 1736.000 counts
ScopeImplBenchmark.scopeTaggedBenchmark:·gc.time thrpt 10 1772.000 ms
ScopeImplBenchmark.scopeTaggedBenchmark:·threads.alive thrpt 10 5.800 ± 0.637 threads
ScopeImplBenchmark.scopeTaggedBenchmark:·threads.daemon thrpt 10 4.000 ± 0.001 threads
ScopeImplBenchmark.scopeTaggedBenchmark:·threads.started thrpt 10 26.000 threads
ScopeImplConcurrent.hotkeyLockContention thrpt 10 1.430 ± 0.218 ops/ms
ScopeImplConcurrent.hotkeyLockContention:·async thrpt NaN ---
ScopeImplConcurrent.hotkeyLockContention:·gc.alloc.rate thrpt 10 1244.444 ± 187.772 MB/sec
ScopeImplConcurrent.hotkeyLockContention:·gc.alloc.rate.norm thrpt 10 960144.037 ± 0.011 B/op
ScopeImplConcurrent.hotkeyLockContention:·gc.churn.G1_Eden_Space thrpt 10 1237.763 ± 186.894 MB/sec
ScopeImplConcurrent.hotkeyLockContention:·gc.churn.G1_Eden_Space.norm thrpt 10 954972.637 ± 4468.064 B/op
ScopeImplConcurrent.hotkeyLockContention:·gc.churn.G1_Old_Gen thrpt 10 0.166 ± 0.001 MB/sec
ScopeImplConcurrent.hotkeyLockContention:·gc.churn.G1_Old_Gen.norm thrpt 10 129.528 ± 18.631 B/op
ScopeImplConcurrent.hotkeyLockContention:·gc.count thrpt 10 1564.000 counts
ScopeImplConcurrent.hotkeyLockContention:·gc.time thrpt 10 1336.000 ms
ScopeImplConcurrent.hotkeyLockContention:·threads.alive thrpt 10 5.800 ± 0.637 threads
ScopeImplConcurrent.hotkeyLockContention:·threads.daemon thrpt 10 4.000 ± 0.001 threads
ScopeImplConcurrent.hotkeyLockContention:·threads.started thrpt 10 26.000 threads
8 changes: 6 additions & 2 deletions core/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
description = 'Interfaces and utilities to report metrics to M3'
apply from: 'jmhFixtures.gradle'

dependencies {
// https://mvnrepository.com/artifact/nl.jqno.equalsverifier/equalsverifier
testImplementation("nl.jqno.equalsverifier:equalsverifier:3.13")
}

sourceSets {
jmh {
java.srcDirs = ['src/jmh/java']
Expand Down Expand Up @@ -57,9 +62,8 @@ task runJmhTests(type: JavaExec, dependsOn: jmhClasses) {
// NOTE: For this to work you need to make sure that async-profiler's library is either
// - Available in LD_LIBRARY_PATH (Linux), DYLD_LIBRARY_PATH (Mac)
// - Available in '-Djava.library.path'
// - Explicitly specified with 'async:libPath=</path/libasyncProfiler.so>'
// - Explicitly specified in pprof arg, the value 'async:libPath=</path/libasyncProfiler.so>'
args '-prof', project.properties.get('bprof', 'async:event=cpu;direction=forward;output=flamegraph')

args '-bm', project.properties.get('bm', 'thrpt')
args '-t', project.properties.get('bthreads', '1')
args 'com.uber.m3.tally.' + project.properties.get('benchclass', '')
Expand Down
3 changes: 3 additions & 0 deletions core/gradle.properties
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#bprof=async:event=cpu;direction=forward;output=flamegraph;dir=profile-results;libPath=<path-to-libasyncProfiler.so>
#benchclass=ScopeImplBenchmark.*
#output=benchmark-new.txt
8 changes: 8 additions & 0 deletions core/src/jmh/java/com/uber/m3/tally/ScopeImplBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;
import org.openjdk.jmh.infra.Blackhole;

import java.util.Random;
import java.util.concurrent.TimeUnit;
Expand All @@ -41,6 +42,8 @@ public class ScopeImplBenchmark {

private static final DurationBuckets EXPONENTIAL_BUCKETS = DurationBuckets.linear(Duration.ofMillis(1), Duration.ofMillis(10), 128);

private static final ImmutableMap<String, String> TAGS_STRING_MAP = ImmutableMap.of("tag1", "value1", "tag2", "value2", "tag3", "value3");

private static final String[] COUNTER_NAMES = {
"first-counter",
"second-counter",
Expand Down Expand Up @@ -70,6 +73,11 @@ public void scopeReportingBenchmark(BenchmarkState state) {
state.scope.reportLoopIteration();
}

@Benchmark
public void scopeTaggedBenchmark(Blackhole blackhole, BenchmarkState state) {
blackhole.consume(state.scope.tagged(TAGS_STRING_MAP));
}

@State(org.openjdk.jmh.annotations.Scope.Benchmark)
public static class BenchmarkState {

Expand Down
16 changes: 9 additions & 7 deletions core/src/jmh/java/com/uber/m3/tally/ScopeImplConcurrent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,25 @@
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.infra.Blackhole;

import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

@BenchmarkMode(Mode.Throughput)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Fork(value = 2, jvmArgsAppend = { "-server", "-XX:+UseG1GC" })
public class ScopeImplConcurrent {
private static final String[] KEYS = new String[]{
" ", "0", "@", "P",
};
private static final List<ScopeKey> SCOPE_KEYS =
Stream.of(" ", "0", "@", "P").map(prefix -> new ScopeKey(prefix, null)).collect(Collectors.toList());

@Benchmark
public void hotkeyLockContention(Blackhole bh, BenchmarkState state) {
ImmutableMap<String, String> common = new ImmutableMap.Builder<String, String>().build();
for (int i = 0; i < 10000; i++) {

for (String key : KEYS) {
Scope scope = state.scope.computeSubscopeIfAbsent("prefix", key, common);
for (ScopeKey scopeKey : SCOPE_KEYS) {
Scope scope = state.scope.computeSubscopeIfAbsent("prefix", scopeKey, common);
assert scope != null;
bh.consume(scope);
}
Expand All @@ -60,8 +62,8 @@ public void setup() {
.reporter(new TestStatsReporter())
.reportEvery(Duration.MAX_VALUE);

for (String key : KEYS) {
scope.computeSubscopeIfAbsent("prefix", key, new ImmutableMap.Builder<String, String>().build());
for (ScopeKey scopeKey : SCOPE_KEYS) {
scope.computeSubscopeIfAbsent("prefix", scopeKey, new ImmutableMap.Builder<String, String>().build());
}
}

Expand Down
100 changes: 36 additions & 64 deletions core/src/main/java/com/uber/m3/tally/ScopeImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@
import com.uber.m3.util.ImmutableMap;

import javax.annotation.Nullable;
import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ScheduledExecutorService;
Expand Down Expand Up @@ -90,7 +88,7 @@ public Timer timer(String name) {
@Override
public Histogram histogram(String name, @Nullable Buckets buckets) {
return histograms.computeIfAbsent(name, ignored ->
// NOTE: This will called at most once
// NOTE: This will be called at most once
new HistogramImpl(
this,
fullyQualifiedName(name),
Expand Down Expand Up @@ -152,35 +150,8 @@ void report(StatsReporter reporter) {

// Serializes a map to generate a key for a prefix/map combination
// Non-generic EMPTY ImmutableMap will never contain any elements
@SuppressWarnings("unchecked")
static String keyForPrefixedStringMap(String prefix, ImmutableMap<String, String> stringMap) {
if (prefix == null) {
prefix = "";
}

if (stringMap == null) {
stringMap = ImmutableMap.EMPTY;
}

Set<String> keySet = stringMap.keySet();
String[] sortedKeys = keySet.toArray(new String[keySet.size()]);
Arrays.sort(sortedKeys);

StringBuilder keyBuffer = new StringBuilder(prefix.length() + sortedKeys.length * 20);
keyBuffer.append(prefix);
keyBuffer.append("+");

for (int i = 0; i < sortedKeys.length; i++) {
keyBuffer.append(sortedKeys[i]);
keyBuffer.append("=");
keyBuffer.append(stringMap.get(sortedKeys[i]));

if (i != sortedKeys.length - 1) {
keyBuffer.append(",");
}
}

return keyBuffer.toString();
static ScopeKey keyForPrefixedStringMap(String prefix, ImmutableMap<String, String> stringMap) {
return new ScopeKey(prefix, stringMap);
}

String fullyQualifiedName(String name) {
Expand All @@ -202,61 +173,61 @@ public Snapshot snapshot() {
for (Map.Entry<String, CounterImpl> counter : subscope.counters.entrySet()) {
String name = subscope.fullyQualifiedName(counter.getKey());

String id = keyForPrefixedStringMap(name, tags);
ScopeKey scopeKey = keyForPrefixedStringMap(name, tags);

snap.counters().put(
id,
new CounterSnapshotImpl(
name,
tags,
counter.getValue().snapshot()
)
scopeKey,
new CounterSnapshotImpl(
name,
tags,
counter.getValue().snapshot()
)
);
}

for (Map.Entry<String, GaugeImpl> gauge : subscope.gauges.entrySet()) {
String name = subscope.fullyQualifiedName(gauge.getKey());

String id = keyForPrefixedStringMap(name, tags);
ScopeKey scopeKey = keyForPrefixedStringMap(name, tags);

snap.gauges().put(
id,
new GaugeSnapshotImpl(
name,
tags,
gauge.getValue().snapshot()
)
scopeKey,
new GaugeSnapshotImpl(
name,
tags,
gauge.getValue().snapshot()
)
);
}

for (Map.Entry<String, TimerImpl> timer : subscope.timers.entrySet()) {
String name = subscope.fullyQualifiedName(timer.getKey());

String id = keyForPrefixedStringMap(name, tags);
ScopeKey scopeKey = keyForPrefixedStringMap(name, tags);

snap.timers().put(
id,
new TimerSnapshotImpl(
name,
tags,
timer.getValue().snapshot()
)
scopeKey,
new TimerSnapshotImpl(
name,
tags,
timer.getValue().snapshot()
)
);
}

for (Map.Entry<String, HistogramImpl> histogram : subscope.histograms.entrySet()) {
String name = subscope.fullyQualifiedName(histogram.getKey());

String id = keyForPrefixedStringMap(name, tags);
ScopeKey scopeKey = keyForPrefixedStringMap(name, tags);

snap.histograms().put(
id,
new HistogramSnapshotImpl(
name,
tags,
histogram.getValue().snapshotValues(),
histogram.getValue().snapshotDurations()
)
scopeKey,
new HistogramSnapshotImpl(
name,
tags,
histogram.getValue().snapshotValues(),
histogram.getValue().snapshotDurations()
)
);
}
}
Expand All @@ -278,13 +249,13 @@ private Scope subScopeHelper(String prefix, Map<String, String> tags) {

ImmutableMap<String, String> mergedTags = mapBuilder.build();

String key = keyForPrefixedStringMap(prefix, mergedTags);
ScopeKey key = keyForPrefixedStringMap(prefix, mergedTags);

return computeSubscopeIfAbsent(prefix, key, mergedTags);
}

// This method must only be called on unit tests or benchmarks
protected Scope computeSubscopeIfAbsent(String prefix, String key, ImmutableMap<String, String> mergedTags) {
protected Scope computeSubscopeIfAbsent(String prefix, ScopeKey key, ImmutableMap<String, String> mergedTags) {
Scope scope = registry.subscopes.get(key);
if (scope != null) {
return scope;
Expand Down Expand Up @@ -342,6 +313,7 @@ private void reportUncaughtException(Exception uncaughtException) {
}

static class Registry {
Map<String, ScopeImpl> subscopes = new ConcurrentHashMap<>();
Map<ScopeKey, ScopeImpl> subscopes = new ConcurrentHashMap<>();
}

}
60 changes: 60 additions & 0 deletions core/src/main/java/com/uber/m3/tally/ScopeKey.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright (c) 2023 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package com.uber.m3.tally;

import com.uber.m3.util.ImmutableMap;

import java.util.Objects;

/**
* ScopeKey encapsulates the data to uniquely identify the {@link Scope}.
* This object overrides {@link #equals(Object)} and {@link #hashCode()} methods, so it can be used in Hash based {@link java.util.Map} implementations, to retrieve the corresponding {@link Scope}.
*/
public final class ScopeKey {
private final String prefix;
private final ImmutableMap<String, String> tags;

public ScopeKey(String prefix, ImmutableMap<String, String> tags) {
this.prefix = prefix;
this.tags = tags;
}

@Override
public int hashCode() {
return Objects.hash(prefix, tags);
}

@Override
public boolean equals(Object otherObj) {
if (this == otherObj) {
return true;
}
if (otherObj == null) {
return false;
}
if (getClass() != otherObj.getClass()) {
return false;
}
ScopeKey other = (ScopeKey) otherObj;
return Objects.equals(this.prefix, other.prefix) && Objects.equals(this.tags, other.tags);
}

}
Loading

0 comments on commit fa530e2

Please sign in to comment.