-
Notifications
You must be signed in to change notification settings - Fork 355
Add Accumulator: a striped long counter primitive as an alternative to LongAdder #12351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dougqh
wants to merge
21
commits into
master
Choose a base branch
from
dougqh/accumulator-primitive
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
c3def00
Add Accumulator: a striped long counter primitive as an alternative t…
dougqh 48e01bb
Oversize Accumulator's default stripe count to reduce contention coll…
dougqh 3629c97
Add JOL footprint test: Accumulator vs one LongAdder per counter
dougqh fc2f55c
Benchmark Accumulator against a per-counter-locked LongAdder alternative
dougqh 89d980c
Address review comments on Accumulator
dougqh 7107b5b
Split Accumulator into a typed wrapper and a nested EmbeddingSupport
dougqh 175154e
Update Accumulator tests/benchmark for the EmbeddingSupport split
dougqh c9f7fc2
Wrap Accumulator's update/accumulateAndReset in typed Stripe/Counts v…
dougqh a8e7448
Add benchmarks for Accumulator's typed API alongside raw EmbeddingSup…
dougqh bf67e0b
Record typed-vs-raw Accumulator benchmark results in AccumulatorBench…
dougqh 3484ed7
Rename accumulatorAccumulateAnd* benchmarks, cap footprint test threa…
dougqh 28de303
Add a non-destructive Accumulator.sum() for live diagnostic reads
dougqh 3c5b7ef
Add Accumulator.Counts.plus() to combine a stored total with a live sum
dougqh 93e2c2d
Add Accumulator.Counts.zero() to seed a running total without a scrat…
dougqh 34f1830
Add a contextual Accumulator.update(context, BiConsumer) overload
dougqh 8df6143
Let Counts expose its own keys and add Class<E>-based factories
dougqh 9bc7abc
Rename Counts.values() to Counts.keys()
dougqh 7e80a21
Add Accumulator.update(long, ObjLongConsumer) to avoid boxing a primi…
dougqh 69251a5
Only touch the real counter width in combine/reset, not the padded st…
dougqh baf0a33
Replace Accumulator's synchronized stripes with lock-free AtomicLongA…
dougqh e90422c
Correct AccumulatorBenchmark javadoc's high-contention drain numbers
dougqh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
230 changes: 230 additions & 0 deletions
230
internal-api/src/jmh/java/datadog/trace/util/AccumulatorBenchmark.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,230 @@ | ||
| package datadog.trace.util; | ||
|
|
||
| import static java.util.concurrent.TimeUnit.MICROSECONDS; | ||
|
|
||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
| import java.util.concurrent.atomic.LongAdder; | ||
| import org.openjdk.jmh.annotations.Benchmark; | ||
| import org.openjdk.jmh.annotations.BenchmarkMode; | ||
| import org.openjdk.jmh.annotations.Fork; | ||
| import org.openjdk.jmh.annotations.Group; | ||
| import org.openjdk.jmh.annotations.GroupThreads; | ||
| import org.openjdk.jmh.annotations.Measurement; | ||
| import org.openjdk.jmh.annotations.Mode; | ||
| import org.openjdk.jmh.annotations.OutputTimeUnit; | ||
| import org.openjdk.jmh.annotations.Scope; | ||
| import org.openjdk.jmh.annotations.State; | ||
| import org.openjdk.jmh.annotations.Threads; | ||
| import org.openjdk.jmh.annotations.Warmup; | ||
| import org.openjdk.jmh.infra.Blackhole; | ||
|
|
||
| /** | ||
| * {@link Accumulator} vs the alternatives it actually displaces: a single {@code LongAdder} (the | ||
| * collision-free baseline it can never beat, only approach), an independent {@code LongAdder} per | ||
| * counter guarded by a per-counter lock (the "just fix it with LongAdder" natural migration target | ||
| * -- {@code longAdderGroup*}), and the {@code ConcurrentHashMap.computeIfAbsent(key, k -> new | ||
| * AtomicLong())} anti-pattern ({@code chmAtomicLongIncrement*}) that {@link Accumulator} exists to | ||
| * avoid. The CHM variant allocates its counter under the bucket's bin lock the first time its one | ||
| * constant key is seen, but since the map is a {@code @State(Scope.Benchmark)} field shared across | ||
| * the whole run, that allocation happens exactly once; every sampled op after it hits the warmed, | ||
| * already-present fast path. So this measures steady-state {@code computeIfAbsent} lookup overhead | ||
| * on an already-populated map, not the one-time allocation-under-lock cost -- still a useful number | ||
| * (a fixed, small key set that's allocated once and hit for the life of the process, as {@code | ||
| * WafMetricCollector}-style CHM counters are, spends nearly all its time in this same warmed path), | ||
| * just not the pathology the name of this benchmark might suggest. | ||
| * | ||
| * <p><b>{@code longAdderGroup*}: is a "just fix it with LongAdder" helper actually cheaper?</b> | ||
| * {@code groupInc}/{@code groupAccumulateAnd} are the natural correct fix using {@code LongAdder} | ||
| * as the payload: one {@code LongAdder} per counter, with a per-counter lock guarding <em>both</em> | ||
| * the increment and the drain (locking only the drain does nothing -- {@code sumThenReset()}'s | ||
| * internal race is against the {@code LongAdder}'s own CAS-based {@code add()}, not against any | ||
| * lock a caller takes). With this benchmark's single counter, that per-counter lock collapses to | ||
| * one lock shared by every thread -- no thread-based distribution at all -- so it loses badly on | ||
| * the write path against {@link Accumulator}'s thread-sharded stripes, especially under contention. | ||
| * This is the realistic production baseline this class was built to replace (see {@code | ||
| * TracerHealthMetrics}'s pre-migration design, one {@code LongAdder} field per counter): <code> | ||
| * AccumulatorBenchmark.accumulatorIncrement_lowContention avgt 6 0.010 ± 0.001 us/op | ||
| * AccumulatorBenchmark.accumulatorIncrement_highContention avgt 6 0.025 ± 0.039 us/op | ||
| * AccumulatorBenchmark.longAdderGroupIncrement_lowContention avgt 6 0.012 ± 0.001 us/op | ||
| * AccumulatorBenchmark.longAdderGroupIncrement_highContention avgt 6 1.178 ± 0.134 us/op | ||
| * AccumulatorBenchmark.accumulatorAccumulateAndReset_lowContention avgt 6 0.104 ± 0.001 us/op | ||
| * AccumulatorBenchmark.accumulatorAccumulateAndReset_highContention avgt 6 13.357 ± 1.203 us/op | ||
| * AccumulatorBenchmark.longAdderGroupAccumulateAnd_lowContention avgt 6 0.024 ± 0.001 us/op | ||
| * AccumulatorBenchmark.longAdderGroupAccumulateAnd_highContention avgt 6 2.439 ± 0.337 us/op | ||
| * </code> At high contention, {@link Accumulator} beats the realistic {@code longAdderGroup} | ||
| * baseline by nearly 50x on increment (the call that runs on every event), but is itself | ||
| * roughly 5.5x <em>worse</em> than {@code longAdderGroup} on drain under that same high-contention | ||
| * topology (the call that runs once per reporting cycle) -- {@code accumulateAndReset} walks every | ||
| * stripe with a full {@code getAndSet} per counter, so more stripes (sized for core count) means | ||
| * more per-drain work than {@code longAdderGroup}'s one-lock-per-counter {@code sumThenReset}. This | ||
| * is still a clean win once weighted by call-site frequency -- the increment win is ~50x on a call | ||
| * that fires on every event, the drain loss is ~5.5x on a call that fires once per reporting cycle | ||
| * (e.g. a 30s flush tick) -- but the drain-side regression is real, not "slightly worse," and worth | ||
| * knowing before assuming this trade is free in every topology. | ||
| */ | ||
| @State(Scope.Benchmark) | ||
| @Warmup(iterations = 1, time = 10) | ||
| @Measurement(iterations = 3, time = 10) | ||
| @BenchmarkMode(Mode.AverageTime) | ||
| @OutputTimeUnit(MICROSECONDS) | ||
| @Fork(2) | ||
| public class AccumulatorBenchmark { | ||
|
|
||
| enum Counter { | ||
| HITS | ||
| } | ||
|
|
||
| private final LongAdder adder = new LongAdder(); | ||
| private final Accumulator<Counter> accumulator = Accumulator.of(Counter.values()); | ||
| private final ConcurrentHashMap<String, AtomicLong> chm = new ConcurrentHashMap<>(); | ||
| private final LongAdder[] longAdderGroup = {new LongAdder()}; | ||
|
|
||
| /** | ||
| * The natural "just use LongAdder" fix for the reset hazard: one {@code LongAdder} per counter, | ||
| * with a per-counter lock guarding both the increment and the drain -- external locking around | ||
| * only the drain does nothing, since {@code sumThenReset()}'s internal race is against the {@code | ||
| * LongAdder}'s own CAS-based {@code add()}, not against any lock a caller takes. This is the fair | ||
| * comparison point: it closes the same reset hazard {@link Accumulator} does, but stripes by | ||
| * <em>counter</em> (one lock per enum constant) instead of by <em>thread</em> (one shared table | ||
| * across all counters) -- so N threads hammering the *same* counter contend on one lock | ||
| * regardless of core count, with no thread-bucket distribution at all. | ||
| */ | ||
| private static void groupInc(LongAdder[] group, int ordinal) { | ||
| LongAdder counter = group[ordinal]; | ||
| synchronized (counter) { | ||
| counter.add(1L); | ||
| } | ||
| } | ||
|
|
||
| private static long[] groupAccumulateAnd(LongAdder[] group) { | ||
| long[] acc = new long[group.length]; | ||
| for (int i = 0; i < group.length; i++) { | ||
| LongAdder counter = group[i]; | ||
| synchronized (counter) { | ||
| acc[i] = counter.sumThenReset(); | ||
| } | ||
| } | ||
| return acc; | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(1) | ||
| public void longAdderIncrement_lowContention() { | ||
| adder.increment(); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(Threads.MAX) | ||
| public void longAdderIncrement_highContention() { | ||
| adder.increment(); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(1) | ||
| public void accumulatorIncrement_lowContention() { | ||
| accumulator.inc(Counter.HITS); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(Threads.MAX) | ||
| public void accumulatorIncrement_highContention() { | ||
| accumulator.inc(Counter.HITS); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(1) | ||
| public void chmAtomicLongIncrement_lowContention() { | ||
| chm.computeIfAbsent("hits", k -> new AtomicLong()).incrementAndGet(); | ||
|
dougqh marked this conversation as resolved.
|
||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(Threads.MAX) | ||
| public void chmAtomicLongIncrement_highContention() { | ||
| chm.computeIfAbsent("hits", k -> new AtomicLong()).incrementAndGet(); | ||
|
dougqh marked this conversation as resolved.
|
||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(1) | ||
| public void longAdderSumThenReset_lowContention(Blackhole blackhole) { | ||
| adder.increment(); | ||
| blackhole.consume(adder.sumThenReset()); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(Threads.MAX) | ||
| public void longAdderSumThenReset_highContention(Blackhole blackhole) { | ||
| adder.increment(); | ||
| blackhole.consume(adder.sumThenReset()); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(1) | ||
| public void accumulatorAccumulateAndReset_lowContention(Blackhole blackhole) { | ||
| accumulator.inc(Counter.HITS); | ||
| blackhole.consume(accumulator.accumulateAndReset()); | ||
| } | ||
|
|
||
| /** | ||
| * A deliberately pessimistic topology: every thread both writes and drains on every op, so {@code | ||
| * Threads.MAX} threads are all draining concurrently. Real callers don't do this -- see {@code | ||
| * accumulatorMixed-write}/{@code accumulatorMixed-drain} below for the "many writers, one rare | ||
| * drainer" shape this class actually targets. Kept as the worst-case upper bound: no production | ||
| * topology should be more contended on {@link Accumulator#accumulateAndReset} than this. | ||
| */ | ||
| @Benchmark | ||
| @Threads(Threads.MAX) | ||
| public void accumulatorAccumulateAndReset_highContention(Blackhole blackhole) { | ||
| accumulator.inc(Counter.HITS); | ||
| blackhole.consume(accumulator.accumulateAndReset()); | ||
| } | ||
|
|
||
| /** | ||
| * The realistic counterpart to {@code accumulatorAccumulateAndReset_highContention}: many writer | ||
| * threads incrementing, and a single dedicated thread polling {@link | ||
| * Accumulator#accumulateAndReset} -- not every thread doing both on every op. {@code | ||
| * accumulatorMixed-write} measures increment cost while a drain is actively running; {@code | ||
| * accumulatorMixed-drain} measures the drain's own cost under that same live write pressure. The | ||
| * 4:1 writer:drainer ratio is illustrative of "many writers, rare drain," not tuned to a specific | ||
| * core count. | ||
| */ | ||
| @Benchmark | ||
| @Group("accumulatorMixed") | ||
| @GroupThreads(4) | ||
| public void accumulatorMixed_write() { | ||
| accumulator.inc(Counter.HITS); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Group("accumulatorMixed") | ||
| @GroupThreads(1) | ||
| public void accumulatorMixed_drain(Blackhole blackhole) { | ||
| blackhole.consume(accumulator.accumulateAndReset()); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(1) | ||
| public void longAdderGroupIncrement_lowContention() { | ||
| groupInc(longAdderGroup, Counter.HITS.ordinal()); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(Threads.MAX) | ||
| public void longAdderGroupIncrement_highContention() { | ||
| groupInc(longAdderGroup, Counter.HITS.ordinal()); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(1) | ||
| public void longAdderGroupAccumulateAnd_lowContention(Blackhole blackhole) { | ||
| groupInc(longAdderGroup, Counter.HITS.ordinal()); | ||
| blackhole.consume(groupAccumulateAnd(longAdderGroup)); | ||
| } | ||
|
|
||
| @Benchmark | ||
| @Threads(Threads.MAX) | ||
| public void longAdderGroupAccumulateAnd_highContention(Blackhole blackhole) { | ||
| groupInc(longAdderGroup, Counter.HITS.ordinal()); | ||
| blackhole.consume(groupAccumulateAnd(longAdderGroup)); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.