From f9e765e0ceddf30a5951e1f59ae6d9dece59819f Mon Sep 17 00:00:00 2001 From: Francesco Nigro Date: Thu, 14 Apr 2022 17:12:42 +0200 Subject: [PATCH] AbstractReferenceCountedByteBuf can save using a volatile set on constructor (#12288) Motivation: AbstractReferenceCountedByteBuf can be made faster by ensure proper safe initialization vs volatile set on construction Modification: Using Unsafe and plain store instead of volatile set Result: Faster AbstractReferenceCountedByteBuf allocation --- .../AbstractReferenceCountedByteBuf.java | 3 +- .../util/internal/PlatformDependent.java | 4 ++ .../util/internal/PlatformDependent0.java | 46 ++++++++++++++++++- .../util/internal/ReferenceCountUpdater.java | 9 ++++ ...tractReferenceCountedByteBufBenchmark.java | 12 +++++ 5 files changed, 72 insertions(+), 2 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java index 05751cbef71..bb15579d04c 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java @@ -43,10 +43,11 @@ protected long unsafeOffset() { // Value might not equal "real" reference count, all access should be via the updater @SuppressWarnings({"unused", "FieldMayBeFinal"}) - private volatile int refCnt = updater.initialValue(); + private volatile int refCnt; protected AbstractReferenceCountedByteBuf(int maxCapacity) { super(maxCapacity); + updater.setInitialValue(this); } @Override diff --git a/common/src/main/java/io/netty/util/internal/PlatformDependent.java b/common/src/main/java/io/netty/util/internal/PlatformDependent.java index d6b140988b4..85320463665 100644 --- a/common/src/main/java/io/netty/util/internal/PlatformDependent.java +++ b/common/src/main/java/io/netty/util/internal/PlatformDependent.java @@ -504,6 +504,10 @@ public static int getInt(Object object, long fieldOffset) { return PlatformDependent0.getInt(object, fieldOffset); } + static void safeConstructPutInt(Object object, long fieldOffset, int value) { + PlatformDependent0.safeConstructPutInt(object, fieldOffset, value); + } + public static int getIntVolatile(long address) { return PlatformDependent0.getIntVolatile(address); } diff --git a/common/src/main/java/io/netty/util/internal/PlatformDependent0.java b/common/src/main/java/io/netty/util/internal/PlatformDependent0.java index 1d2c78f8aae..ac6f6502fe7 100644 --- a/common/src/main/java/io/netty/util/internal/PlatformDependent0.java +++ b/common/src/main/java/io/netty/util/internal/PlatformDependent0.java @@ -49,6 +49,7 @@ final class PlatformDependent0 { private static final Method ALIGN_SLICE; private static final int JAVA_VERSION = javaVersion0(); private static final boolean IS_ANDROID = isAndroid0(); + private static final boolean STORE_FENCE_AVAILABLE; private static final Throwable UNSAFE_UNAVAILABILITY_CAUSE; private static final Object INTERNAL_UNSAFE; @@ -81,7 +82,7 @@ final class PlatformDependent0 { Throwable unsafeUnavailabilityCause = null; Unsafe unsafe; Object internalUnsafe = null; - + boolean storeFenceAvailable = false; if ((unsafeUnavailabilityCause = EXPLICIT_NO_UNSAFE_CAUSE) != null) { direct = null; addressField = null; @@ -170,6 +171,38 @@ public Object run() { } } + // ensure Unsafe::storeFence to be available: jdk < 8 shouldn't have it + if (unsafe != null) { + final Unsafe finalUnsafe = unsafe; + final Object maybeException = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Object run() { + try { + finalUnsafe.getClass().getDeclaredMethod("storeFence"); + return null; + } catch (NoSuchMethodException e) { + return e; + } catch (SecurityException e) { + return e; + } + } + }); + + if (maybeException == null) { + logger.debug("sun.misc.Unsafe.storeFence: available"); + storeFenceAvailable = true; + } else { + storeFenceAvailable = false; + // Unsafe.storeFence unavailable. + if (logger.isTraceEnabled()) { + logger.debug("sun.misc.Unsafe.storeFence: unavailable", (Throwable) maybeException); + } else { + logger.debug("sun.misc.Unsafe.storeFence: unavailable: {}", + ((Throwable) maybeException).getMessage()); + } + } + } + if (unsafe != null) { final Unsafe finalUnsafe = unsafe; @@ -239,6 +272,7 @@ public Object run() { UNALIGNED = false; DIRECT_BUFFER_CONSTRUCTOR = null; ALLOCATE_ARRAY_METHOD = null; + STORE_FENCE_AVAILABLE = false; } else { Constructor directBufferConstructor; long address = -1; @@ -424,6 +458,7 @@ public Object run() { logger.debug("jdk.internal.misc.Unsafe.allocateUninitializedArray(int): unavailable prior to Java9"); } ALLOCATE_ARRAY_METHOD = allocateArrayMethod; + STORE_FENCE_AVAILABLE = storeFenceAvailable; } if (javaVersion() > 9) { @@ -575,6 +610,15 @@ static int getInt(Object object, long fieldOffset) { return UNSAFE.getInt(object, fieldOffset); } + static void safeConstructPutInt(Object object, long fieldOffset, int value) { + if (STORE_FENCE_AVAILABLE) { + UNSAFE.putInt(object, fieldOffset, value); + UNSAFE.storeFence(); + } else { + UNSAFE.putIntVolatile(object, fieldOffset, value); + } + } + private static long getLong(Object object, long fieldOffset) { return UNSAFE.getLong(object, fieldOffset); } diff --git a/common/src/main/java/io/netty/util/internal/ReferenceCountUpdater.java b/common/src/main/java/io/netty/util/internal/ReferenceCountUpdater.java index 93d12bb29ba..8e4abd8ff6e 100644 --- a/common/src/main/java/io/netty/util/internal/ReferenceCountUpdater.java +++ b/common/src/main/java/io/netty/util/internal/ReferenceCountUpdater.java @@ -59,6 +59,15 @@ public final int initialValue() { return 2; } + public void setInitialValue(T instance) { + final long offset = unsafeOffset(); + if (offset == -1) { + updater().set(instance, initialValue()); + } else { + PlatformDependent.safeConstructPutInt(instance, offset, initialValue()); + } + } + private static int realRefCnt(int rawCnt) { return rawCnt != 2 && rawCnt != 4 && (rawCnt & 1) != 0 ? 0 : rawCnt >>> 1; } diff --git a/microbench/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBufBenchmark.java b/microbench/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBufBenchmark.java index 3e3a3292ee2..a823aae974c 100644 --- a/microbench/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBufBenchmark.java +++ b/microbench/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBufBenchmark.java @@ -18,6 +18,7 @@ import io.netty.microbench.util.AbstractMicrobenchmark; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.CompilerControl; import org.openjdk.jmh.annotations.GroupThreads; import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.annotations.OutputTimeUnit; @@ -57,6 +58,17 @@ public boolean retainReleaseUncontended() { return buf.release(); } + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + @CompilerControl(CompilerControl.Mode.DONT_INLINE) + public boolean createUseAndRelease(Blackhole useBuffer) { + ByteBuf unpooled = Unpooled.buffer(1); + useBuffer.consume(unpooled); + Blackhole.consumeCPU(delay); + return unpooled.release(); + } + @Benchmark @BenchmarkMode(Mode.AverageTime) @OutputTimeUnit(TimeUnit.NANOSECONDS)