From 52169cba95850e77136f1821ea3ae4d069505283 Mon Sep 17 00:00:00 2001 From: Alex Blewitt Date: Tue, 25 Jun 2019 20:46:26 +0100 Subject: [PATCH] Replace accumulation with blackhole.consume (#9275) Motivation: SpotJMHBugs reports that accumulating a value as a way of eliding dead code elimination may be inadvisable, as discussed in `JMHSample_34_SafeLooping::measureWrong_2`. Change the test so that it consumes the response with `Blackhole::consume` instead. Modifications: - Replace addition of results with explicit `blackhole.consume()` call Result: Tests work as before, but with different benchmark numbers. --- .../codec/http/HttpMethodMapBenchmark.java | 39 +++++++------------ .../FastThreadLocalFastPathBenchmark.java | 13 +++---- .../FastThreadLocalSlowPathBenchmark.java | 13 +++---- .../ReadOnlyHttp2HeadersBenchmark.java | 33 ++++++++-------- 4 files changed, 40 insertions(+), 58 deletions(-) diff --git a/microbench/src/main/java/io/netty/handler/codec/http/HttpMethodMapBenchmark.java b/microbench/src/main/java/io/netty/handler/codec/http/HttpMethodMapBenchmark.java index 69dae0f4c9..08bd84eb58 100644 --- a/microbench/src/main/java/io/netty/handler/codec/http/HttpMethodMapBenchmark.java +++ b/microbench/src/main/java/io/netty/handler/codec/http/HttpMethodMapBenchmark.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 The Netty Project + * Copyright 2019 The Netty Project * * The Netty Project licenses this file to you under the Apache License, * version 2.0 (the "License"); you may not use this file except in compliance @@ -22,6 +22,7 @@ import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; import java.util.HashMap; import java.util.Map; @@ -151,68 +152,56 @@ public class HttpMethodMapBenchmark extends AbstractMicrobenchmark { } @Benchmark - public int oldMapKnownMethods() throws Exception { - int x = 0; + public void oldMapKnownMethods(Blackhole bh) throws Exception { for (int i = 0; i < KNOWN_METHODS.length; ++i) { - x += OLD_MAP.get(KNOWN_METHODS[i]).toString().length(); + bh.consume(OLD_MAP.get(KNOWN_METHODS[i])); } - return x; } @Benchmark - public int newMapKnownMethods() throws Exception { - int x = 0; + public void newMapKnownMethods(Blackhole bh) throws Exception { for (int i = 0; i < KNOWN_METHODS.length; ++i) { - x += NEW_MAP.get(KNOWN_METHODS[i]).toString().length(); + bh.consume(NEW_MAP.get(KNOWN_METHODS[i])); } - return x; } @Benchmark - public int oldMapMixMethods() throws Exception { - int x = 0; + public void oldMapMixMethods(Blackhole bh) throws Exception { for (int i = 0; i < MIXED_METHODS.length; ++i) { HttpMethod method = OLD_MAP.get(MIXED_METHODS[i]); if (method != null) { - x += method.toString().length(); + bh.consume(method); } } - return x; } @Benchmark - public int newMapMixMethods() throws Exception { - int x = 0; + public void newMapMixMethods(Blackhole bh) throws Exception { for (int i = 0; i < MIXED_METHODS.length; ++i) { HttpMethod method = NEW_MAP.get(MIXED_METHODS[i]); if (method != null) { - x += method.toString().length(); + bh.consume(method); } } - return x; } @Benchmark - public int oldMapUnknownMethods() throws Exception { - int x = 0; + public void oldMapUnknownMethods(Blackhole bh) throws Exception { for (int i = 0; i < UNKNOWN_METHODS.length; ++i) { HttpMethod method = OLD_MAP.get(UNKNOWN_METHODS[i]); if (method != null) { - x += method.toString().length(); + bh.consume(method); } } - return x; } @Benchmark - public int newMapUnknownMethods() throws Exception { - int x = 0; + public void newMapUnknownMethods(Blackhole bh) throws Exception { for (int i = 0; i < UNKNOWN_METHODS.length; ++i) { HttpMethod method = NEW_MAP.get(UNKNOWN_METHODS[i]); if (method != null) { - x += method.toString().length(); + bh.consume(method); } } - return x; } } diff --git a/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalFastPathBenchmark.java b/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalFastPathBenchmark.java index c0073fb655..c371ad18d0 100644 --- a/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalFastPathBenchmark.java +++ b/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalFastPathBenchmark.java @@ -20,6 +20,7 @@ import io.netty.util.concurrent.FastThreadLocal; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Measurement; import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.infra.Blackhole; import java.util.Random; @@ -56,20 +57,16 @@ public class FastThreadLocalFastPathBenchmark extends AbstractMicrobenchmark { } @Benchmark - public int jdkThreadLocalGet() { - int result = 0; + public void jdkThreadLocalGet(Blackhole bh) { for (ThreadLocal i: jdkThreadLocals) { - result += i.get(); + bh.consume(i.get()); } - return result; } @Benchmark - public int fastThreadLocal() { - int result = 0; + public void fastThreadLocal(Blackhole bh) { for (FastThreadLocal i: fastThreadLocals) { - result += i.get(); + bh.consume(i.get()); } - return result; } } diff --git a/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalSlowPathBenchmark.java b/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalSlowPathBenchmark.java index 20d414aa1f..70a19a029a 100644 --- a/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalSlowPathBenchmark.java +++ b/microbench/src/main/java/io/netty/microbench/concurrent/FastThreadLocalSlowPathBenchmark.java @@ -20,6 +20,7 @@ import io.netty.util.concurrent.FastThreadLocal; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Measurement; import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.infra.Blackhole; import java.util.Random; @@ -60,20 +61,16 @@ public class FastThreadLocalSlowPathBenchmark extends AbstractMicrobenchmark { } @Benchmark - public int jdkThreadLocalGet() { - int result = 0; + public void jdkThreadLocalGet(Blackhole bh) { for (ThreadLocal i: jdkThreadLocals) { - result += i.get(); + bh.consume(i.get()); } - return result; } @Benchmark - public int fastThreadLocal() { - int result = 0; + public void fastThreadLocal(Blackhole bh) { for (FastThreadLocal i: fastThreadLocals) { - result += i.get(); + bh.consume(i.get()); } - return result; } } diff --git a/microbench/src/main/java/io/netty/microbench/headers/ReadOnlyHttp2HeadersBenchmark.java b/microbench/src/main/java/io/netty/microbench/headers/ReadOnlyHttp2HeadersBenchmark.java index 9efc2f3b20..920da6f12a 100644 --- a/microbench/src/main/java/io/netty/microbench/headers/ReadOnlyHttp2HeadersBenchmark.java +++ b/microbench/src/main/java/io/netty/microbench/headers/ReadOnlyHttp2HeadersBenchmark.java @@ -35,6 +35,7 @@ import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Threads; import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; import java.util.Map; import java.util.UUID; @@ -68,23 +69,23 @@ public class ReadOnlyHttp2HeadersBenchmark extends AbstractMicrobenchmark { @Benchmark @BenchmarkMode(Mode.AverageTime) - public int defaultTrailers() { + public void defaultTrailers(Blackhole bh) { Http2Headers headers = new DefaultHttp2Headers(false); for (int i = 0; i < headerCount; ++i) { headers.add(headerNames[i], headerValues[i]); } - return iterate(headers); + iterate(headers, bh); } @Benchmark @BenchmarkMode(Mode.AverageTime) - public int readOnlyTrailers() { - return iterate(ReadOnlyHttp2Headers.trailers(false, buildPairs())); + public void readOnlyTrailers(Blackhole bh) { + iterate(ReadOnlyHttp2Headers.trailers(false, buildPairs()), bh); } @Benchmark @BenchmarkMode(Mode.AverageTime) - public int defaultClientHeaders() { + public void defaultClientHeaders(Blackhole bh) { Http2Headers headers = new DefaultHttp2Headers(false); for (int i = 0; i < headerCount; ++i) { headers.add(headerNames[i], headerValues[i]); @@ -93,39 +94,37 @@ public class ReadOnlyHttp2HeadersBenchmark extends AbstractMicrobenchmark { headers.scheme(HttpScheme.HTTPS.name()); headers.path(path); headers.authority(authority); - return iterate(headers); + iterate(headers, bh); } @Benchmark @BenchmarkMode(Mode.AverageTime) - public int readOnlyClientHeaders() { - return iterate(ReadOnlyHttp2Headers.clientHeaders(false, HttpMethod.POST.asciiName(), path, - HttpScheme.HTTPS.name(), authority, buildPairs())); + public void readOnlyClientHeaders(Blackhole bh) { + iterate(ReadOnlyHttp2Headers.clientHeaders(false, HttpMethod.POST.asciiName(), path, + HttpScheme.HTTPS.name(), authority, buildPairs()), bh); } @Benchmark @BenchmarkMode(Mode.AverageTime) - public int defaultServerHeaders() { + public void defaultServerHeaders(Blackhole bh) { Http2Headers headers = new DefaultHttp2Headers(false); for (int i = 0; i < headerCount; ++i) { headers.add(headerNames[i], headerValues[i]); } headers.status(HttpResponseStatus.OK.codeAsText()); - return iterate(headers); + iterate(headers, bh); } @Benchmark @BenchmarkMode(Mode.AverageTime) - public int readOnlyServerHeaders() { - return iterate(ReadOnlyHttp2Headers.serverHeaders(false, HttpResponseStatus.OK.codeAsText(), buildPairs())); + public void readOnlyServerHeaders(Blackhole bh) { + iterate(ReadOnlyHttp2Headers.serverHeaders(false, HttpResponseStatus.OK.codeAsText(), buildPairs()), bh); } - private static int iterate(Http2Headers headers) { - int length = 0; + private static void iterate(Http2Headers headers, Blackhole bh) { for (Map.Entry entry : headers) { - length += entry.getKey().length() + entry.getValue().length(); + bh.consume(entry); } - return length; } private AsciiString[] buildPairs() {