From 6942cc1feff913ec2a52e2569cd57f78c73a7864 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Mon, 14 Apr 2025 15:50:42 +0200 Subject: [PATCH] Breaking change for StatisticsCollector Use `LongAdder` instead of `AtomicLong`, that is more suitable for high contention (see the javadoc). --- .../stats/DelegatingStatisticsCollector.java | 40 +++---- .../stats/NoOpStatisticsCollector.java | 36 +++--- .../stats/SimpleStatisticsCollector.java | 59 +++++----- .../dataloader/stats/StatisticsCollector.java | 44 +++---- .../stats/ThreadLocalStatisticsCollector.java | 40 +++---- .../org/dataloader/DataLoaderStatsTest.java | 32 ++--- .../dataloader/performance/AtomicVsAdder.java | 111 ++++++++++++++++++ 7 files changed, 224 insertions(+), 138 deletions(-) create mode 100644 src/test/java/org/dataloader/performance/AtomicVsAdder.java diff --git a/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java index 563d37b..36ed69a 100644 --- a/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java @@ -26,63 +26,63 @@ public DelegatingStatisticsCollector(StatisticsCollector delegateCollector) { } @Override - public long incrementLoadCount(IncrementLoadCountStatisticsContext context) { + public void incrementLoadCount(IncrementLoadCountStatisticsContext context) { delegateCollector.incrementLoadCount(context); - return collector.incrementLoadCount(context); + collector.incrementLoadCount(context); } @Deprecated @Override - public long incrementLoadCount() { - return incrementLoadCount(null); + public void incrementLoadCount() { + incrementLoadCount(null); } @Override - public long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { + public void incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { delegateCollector.incrementLoadErrorCount(context); - return collector.incrementLoadErrorCount(context); + collector.incrementLoadErrorCount(context); } @Deprecated @Override - public long incrementLoadErrorCount() { - return incrementLoadErrorCount(null); + public void incrementLoadErrorCount() { + incrementLoadErrorCount(null); } @Override - public long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { + public void incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { delegateCollector.incrementBatchLoadCountBy(delta, context); - return collector.incrementBatchLoadCountBy(delta, context); + collector.incrementBatchLoadCountBy(delta, context); } @Deprecated @Override - public long incrementBatchLoadCountBy(long delta) { - return incrementBatchLoadCountBy(delta, null); + public void incrementBatchLoadCountBy(long delta) { + incrementBatchLoadCountBy(delta, null); } @Override - public long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { + public void incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { delegateCollector.incrementBatchLoadExceptionCount(context); - return collector.incrementBatchLoadExceptionCount(context); + collector.incrementBatchLoadExceptionCount(context); } @Deprecated @Override - public long incrementBatchLoadExceptionCount() { - return incrementBatchLoadExceptionCount(null); + public void incrementBatchLoadExceptionCount() { + incrementBatchLoadExceptionCount(null); } @Override - public long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { + public void incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { delegateCollector.incrementCacheHitCount(context); - return collector.incrementCacheHitCount(context); + collector.incrementCacheHitCount(context); } @Deprecated @Override - public long incrementCacheHitCount() { - return incrementCacheHitCount(null); + public void incrementCacheHitCount() { + incrementCacheHitCount(null); } /** diff --git a/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java index e7267b3..6397d8f 100644 --- a/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java @@ -14,58 +14,54 @@ public class NoOpStatisticsCollector implements StatisticsCollector { private static final Statistics ZERO_STATS = new Statistics(); @Override - public long incrementLoadCount(IncrementLoadCountStatisticsContext context) { - return 0; + public void incrementLoadCount(IncrementLoadCountStatisticsContext context) { } @Deprecated @Override - public long incrementLoadCount() { - return incrementLoadCount(null); + public void incrementLoadCount() { + incrementLoadCount(null); } @Override - public long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { - return 0; + public void incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { } @Deprecated @Override - public long incrementLoadErrorCount() { - return incrementLoadErrorCount(null); + public void incrementLoadErrorCount() { + incrementLoadErrorCount(null); } @Override - public long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { - return 0; + public void incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { } @Deprecated @Override - public long incrementBatchLoadCountBy(long delta) { - return incrementBatchLoadCountBy(delta, null); + public void incrementBatchLoadCountBy(long delta) { + incrementBatchLoadCountBy(delta, null); } @Override - public long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { - return 0; + public void incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { + } @Deprecated @Override - public long incrementBatchLoadExceptionCount() { - return incrementBatchLoadExceptionCount(null); + public void incrementBatchLoadExceptionCount() { + incrementBatchLoadExceptionCount(null); } @Override - public long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { - return 0; + public void incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { } @Deprecated @Override - public long incrementCacheHitCount() { - return incrementCacheHitCount(null); + public void incrementCacheHitCount() { + incrementCacheHitCount(null); } @Override diff --git a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java index 22b3662..2c2898a 100644 --- a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java @@ -6,7 +6,7 @@ import org.dataloader.stats.context.IncrementLoadCountStatisticsContext; import org.dataloader.stats.context.IncrementLoadErrorCountStatisticsContext; -import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.LongAdder; /** * This simple collector uses {@link java.util.concurrent.atomic.AtomicLong}s to collect @@ -15,72 +15,73 @@ * @see org.dataloader.stats.StatisticsCollector */ public class SimpleStatisticsCollector implements StatisticsCollector { - private final AtomicLong loadCount = new AtomicLong(); - private final AtomicLong batchInvokeCount = new AtomicLong(); - private final AtomicLong batchLoadCount = new AtomicLong(); - private final AtomicLong cacheHitCount = new AtomicLong(); - private final AtomicLong batchLoadExceptionCount = new AtomicLong(); - private final AtomicLong loadErrorCount = new AtomicLong(); + + private final LongAdder loadCount = new LongAdder(); + private final LongAdder batchInvokeCount = new LongAdder(); + private final LongAdder batchLoadCount = new LongAdder(); + private final LongAdder cacheHitCount = new LongAdder(); + private final LongAdder batchLoadExceptionCount = new LongAdder(); + private final LongAdder loadErrorCount = new LongAdder(); @Override - public long incrementLoadCount(IncrementLoadCountStatisticsContext context) { - return loadCount.incrementAndGet(); + public void incrementLoadCount(IncrementLoadCountStatisticsContext context) { + loadCount.increment(); } @Deprecated @Override - public long incrementLoadCount() { - return incrementLoadCount(null); + public void incrementLoadCount() { + incrementLoadCount(null); } @Override - public long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { - return loadErrorCount.incrementAndGet(); + public void incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { + loadErrorCount.increment(); } @Deprecated @Override - public long incrementLoadErrorCount() { - return incrementLoadErrorCount(null); + public void incrementLoadErrorCount() { + incrementLoadErrorCount(null); } @Override - public long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { - batchInvokeCount.incrementAndGet(); - return batchLoadCount.addAndGet(delta); + public void incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { + batchInvokeCount.increment(); + batchLoadCount.add(delta); } @Deprecated @Override - public long incrementBatchLoadCountBy(long delta) { - return incrementBatchLoadCountBy(delta, null); + public void incrementBatchLoadCountBy(long delta) { + incrementBatchLoadCountBy(delta, null); } @Override - public long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { - return batchLoadExceptionCount.incrementAndGet(); + public void incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { + batchLoadExceptionCount.increment(); } @Deprecated @Override - public long incrementBatchLoadExceptionCount() { - return incrementBatchLoadExceptionCount(null); + public void incrementBatchLoadExceptionCount() { + incrementBatchLoadExceptionCount(null); } @Override - public long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { - return cacheHitCount.incrementAndGet(); + public void incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { + cacheHitCount.increment(); } @Deprecated @Override - public long incrementCacheHitCount() { - return incrementCacheHitCount(null); + public void incrementCacheHitCount() { + incrementCacheHitCount(null); } @Override public Statistics getStatistics() { - return new Statistics(loadCount.get(), loadErrorCount.get(), batchInvokeCount.get(), batchLoadCount.get(), batchLoadExceptionCount.get(), cacheHitCount.get()); + return new Statistics(loadCount.sum(), loadErrorCount.sum(), batchInvokeCount.sum(), batchLoadCount.sum(), batchLoadExceptionCount.sum(), cacheHitCount.sum()); } @Override diff --git a/src/main/java/org/dataloader/stats/StatisticsCollector.java b/src/main/java/org/dataloader/stats/StatisticsCollector.java index 33e417f..7b14eab 100644 --- a/src/main/java/org/dataloader/stats/StatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/StatisticsCollector.java @@ -18,21 +18,18 @@ public interface StatisticsCollector { * * @param the class of the key in the data loader * @param context the context containing metadata of the data loader invocation - * - * @return the current value after increment */ - default long incrementLoadCount(IncrementLoadCountStatisticsContext context) { - return incrementLoadCount(); + default void incrementLoadCount(IncrementLoadCountStatisticsContext context) { + incrementLoadCount(); } /** * Called to increment the number of loads * * @deprecated use {@link #incrementLoadCount(IncrementLoadCountStatisticsContext)} - * @return the current value after increment */ @Deprecated - long incrementLoadCount(); + void incrementLoadCount(); /** * Called to increment the number of loads that resulted in an object deemed in error @@ -40,20 +37,18 @@ default long incrementLoadCount(IncrementLoadCountStatisticsContext conte * @param the class of the key in the data loader * @param context the context containing metadata of the data loader invocation * - * @return the current value after increment */ - default long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { - return incrementLoadErrorCount(); + default void incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { + incrementLoadErrorCount(); } /** * Called to increment the number of loads that resulted in an object deemed in error * * @deprecated use {@link #incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext)} - * @return the current value after increment */ @Deprecated - long incrementLoadErrorCount(); + void incrementLoadErrorCount(); /** * Called to increment the number of batch loads @@ -61,11 +56,9 @@ default long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContex * @param the class of the key in the data loader * @param delta how much to add to the count * @param context the context containing metadata of the data loader invocation - * - * @return the current value after increment */ - default long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { - return incrementBatchLoadCountBy(delta); + default void incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { + incrementBatchLoadCountBy(delta); } /** @@ -74,52 +67,45 @@ default long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountBy * @param delta how much to add to the count * * @deprecated use {@link #incrementBatchLoadCountBy(long, IncrementBatchLoadCountByStatisticsContext)} - * @return the current value after increment */ @Deprecated - long incrementBatchLoadCountBy(long delta); + void incrementBatchLoadCountBy(long delta); /** * Called to increment the number of batch loads exceptions * * @param the class of the key in the data loader * @param context the context containing metadata of the data loader invocation - * - * @return the current value after increment */ - default long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { - return incrementBatchLoadExceptionCount(); + default void incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { + incrementBatchLoadExceptionCount(); } /** * Called to increment the number of batch loads exceptions * * @deprecated use {@link #incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext)} - * @return the current value after increment */ @Deprecated - long incrementBatchLoadExceptionCount(); + void incrementBatchLoadExceptionCount(); /** * Called to increment the number of cache hits * * @param the class of the key in the data loader * @param context the context containing metadata of the data loader invocation - * - * @return the current value after increment */ - default long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { - return incrementCacheHitCount(); + default void incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { + incrementCacheHitCount(); } /** * Called to increment the number of cache hits * * @deprecated use {@link #incrementCacheHitCount(IncrementCacheHitCountStatisticsContext)} - * @return the current value after increment */ @Deprecated - long incrementCacheHitCount(); + void incrementCacheHitCount(); /** * @return the statistics that have been gathered to this point in time diff --git a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java index d091c5a..cab6d0d 100644 --- a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java @@ -35,63 +35,63 @@ public ThreadLocalStatisticsCollector resetThread() { } @Override - public long incrementLoadCount(IncrementLoadCountStatisticsContext context) { + public void incrementLoadCount(IncrementLoadCountStatisticsContext context) { overallCollector.incrementLoadCount(context); - return collector.get().incrementLoadCount(context); + collector.get().incrementLoadCount(context); } @Deprecated @Override - public long incrementLoadCount() { - return incrementLoadCount(null); + public void incrementLoadCount() { + incrementLoadCount(null); } @Override - public long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { + public void incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { overallCollector.incrementLoadErrorCount(context); - return collector.get().incrementLoadErrorCount(context); + collector.get().incrementLoadErrorCount(context); } @Deprecated @Override - public long incrementLoadErrorCount() { - return incrementLoadErrorCount(null); + public void incrementLoadErrorCount() { + incrementLoadErrorCount(null); } @Override - public long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { + public void incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { overallCollector.incrementBatchLoadCountBy(delta, context); - return collector.get().incrementBatchLoadCountBy(delta, context); + collector.get().incrementBatchLoadCountBy(delta, context); } @Deprecated @Override - public long incrementBatchLoadCountBy(long delta) { - return incrementBatchLoadCountBy(delta, null); + public void incrementBatchLoadCountBy(long delta) { + incrementBatchLoadCountBy(delta, null); } @Override - public long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { + public void incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { overallCollector.incrementBatchLoadExceptionCount(context); - return collector.get().incrementBatchLoadExceptionCount(context); + collector.get().incrementBatchLoadExceptionCount(context); } @Deprecated @Override - public long incrementBatchLoadExceptionCount() { - return incrementBatchLoadExceptionCount(null); + public void incrementBatchLoadExceptionCount() { + incrementBatchLoadExceptionCount(null); } @Override - public long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { + public void incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { overallCollector.incrementCacheHitCount(context); - return collector.get().incrementCacheHitCount(context); + collector.get().incrementCacheHitCount(context); } @Deprecated @Override - public long incrementCacheHitCount() { - return incrementCacheHitCount(null); + public void incrementCacheHitCount() { + incrementCacheHitCount(null); } /** diff --git a/src/test/java/org/dataloader/DataLoaderStatsTest.java b/src/test/java/org/dataloader/DataLoaderStatsTest.java index b8393e6..87814d1 100644 --- a/src/test/java/org/dataloader/DataLoaderStatsTest.java +++ b/src/test/java/org/dataloader/DataLoaderStatsTest.java @@ -221,63 +221,55 @@ private static class ContextPassingStatisticsCollector implements StatisticsColl public List> incrementCacheHitCountStatisticsContexts = new ArrayList<>(); @Override - public long incrementLoadCount(IncrementLoadCountStatisticsContext context) { + public void incrementLoadCount(IncrementLoadCountStatisticsContext context) { incrementLoadCountStatisticsContexts.add(context); - return 0; } @Deprecated @Override - public long incrementLoadCount() { - return 0; + public void incrementLoadCount() { + } @Override - public long incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { + public void incrementLoadErrorCount(IncrementLoadErrorCountStatisticsContext context) { incrementLoadErrorCountStatisticsContexts.add(context); - return 0; } @Deprecated @Override - public long incrementLoadErrorCount() { - return 0; + public void incrementLoadErrorCount() { + } @Override - public long incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { + public void incrementBatchLoadCountBy(long delta, IncrementBatchLoadCountByStatisticsContext context) { incrementBatchLoadCountByStatisticsContexts.add(context); - return 0; } @Deprecated @Override - public long incrementBatchLoadCountBy(long delta) { - return 0; + public void incrementBatchLoadCountBy(long delta) { } @Override - public long incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { + public void incrementBatchLoadExceptionCount(IncrementBatchLoadExceptionCountStatisticsContext context) { incrementBatchLoadExceptionCountStatisticsContexts.add(context); - return 0; } @Deprecated @Override - public long incrementBatchLoadExceptionCount() { - return 0; + public void incrementBatchLoadExceptionCount() { } @Override - public long incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { + public void incrementCacheHitCount(IncrementCacheHitCountStatisticsContext context) { incrementCacheHitCountStatisticsContexts.add(context); - return 0; } @Deprecated @Override - public long incrementCacheHitCount() { - return 0; + public void incrementCacheHitCount() { } @Override diff --git a/src/test/java/org/dataloader/performance/AtomicVsAdder.java b/src/test/java/org/dataloader/performance/AtomicVsAdder.java new file mode 100644 index 0000000..ee5e02a --- /dev/null +++ b/src/test/java/org/dataloader/performance/AtomicVsAdder.java @@ -0,0 +1,111 @@ +package org.dataloader.performance; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.LongAdder; + +public class AtomicVsAdder { + + private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool(); + + public static void main(final String[] args) throws Exception { + // knobs + final var iterationsList = List.of(1 << 20L, 1 << 24L); + final var numberOfThreadsList = List.of(1, 2, 4, 8, 16); + final var strategies = List.of(new LongAdderStrategy(), new AtomicLongStrategy()); + + // test + System.out.println("testing with #cpu=" + Runtime.getRuntime().availableProcessors()); + for (int iterations : iterationsList) { + for (int numberOfThreads : numberOfThreadsList) { + for (Strategy strategy : strategies) { + performTest(iterations, numberOfThreads, strategy); + } + } + } + + EXECUTOR.shutdownNow(); + + } + + private static void performTest(final long iterations, final int numberOfThreads, Strategy strategy) throws Exception { + final List> futures = new ArrayList<>(); + System.out.println("start test with " + iterations + " iterations using " + numberOfThreads + " threads and strategy " + strategy.getClass().getSimpleName()); + final long start = System.nanoTime(); + + for (int i = 0; i < numberOfThreads; i++) { + Future submit = EXECUTOR.submit(() -> concurrentWork(strategy, iterations)); + futures.add(submit); + } + for (final Future future : futures) { + future.get(); // wait for all + } + final long end = System.nanoTime(); + System.out.println("done in " + Duration.ofNanos(end - start).toMillis() + "ms => result " + strategy.get()); + System.out.println("----"); + strategy.reset(); + } + + @SuppressWarnings("SameParameterValue") + private static void concurrentWork(final Strategy strategy, final long iterations) { + long work = iterations; + while (work-- > 0) { + strategy.increment(); + } + } + + interface Strategy { + void increment(); + + long get(); + + void reset(); + } + + static class LongAdderStrategy implements Strategy { + + private LongAdder longAdder = new LongAdder(); + + @Override + public void increment() { + longAdder.increment(); + } + + @Override + public long get() { + return longAdder.sum(); + } + + @Override + public void reset() { + longAdder = new LongAdder(); + } + } + + static class AtomicLongStrategy implements Strategy { + + private final AtomicLong atomicLong = new AtomicLong(0); + + @Override + public void increment() { + atomicLong.incrementAndGet(); + } + + @Override + public long get() { + return atomicLong.get(); + } + + @Override + public void reset() { + atomicLong.set(0); + } + } + +} \ No newline at end of file