diff --git a/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java index e657ad86f8..d3058c0fb6 100644 --- a/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBuf.java @@ -59,18 +59,12 @@ public abstract class AbstractReferenceCountedByteBuf extends AbstractByteBuf { return retain0(checkPositive(increment, "increment")); } - private ByteBuf retain0(int increment) { - for (;;) { - int refCnt = this.refCnt; - final int nextCnt = refCnt + increment; - - // Ensure we not resurrect (which means the refCnt was 0) and also that we encountered an overflow. - if (nextCnt <= increment) { - throw new IllegalReferenceCountException(refCnt, increment); - } - if (refCntUpdater.compareAndSet(this, refCnt, nextCnt)) { - break; - } + private ByteBuf retain0(final int increment) { + int oldRef = refCntUpdater.getAndAdd(this, increment); + if (oldRef <= 0 || oldRef + increment < oldRef) { + // Ensure we don't resurrect (which means the refCnt was 0) and also that we encountered an overflow. + refCntUpdater.getAndAdd(this, -increment); + throw new IllegalReferenceCountException(oldRef, increment); } return this; } @@ -96,20 +90,16 @@ public abstract class AbstractReferenceCountedByteBuf extends AbstractByteBuf { } private boolean release0(int decrement) { - for (;;) { - int refCnt = this.refCnt; - if (refCnt < decrement) { - throw new IllegalReferenceCountException(refCnt, -decrement); - } - - if (refCntUpdater.compareAndSet(this, refCnt, refCnt - decrement)) { - if (refCnt == decrement) { - deallocate(); - return true; - } - return false; - } + int oldRef = refCntUpdater.getAndAdd(this, -decrement); + if (oldRef == decrement) { + deallocate(); + return true; + } else if (oldRef < decrement || oldRef - decrement > oldRef) { + // Ensure we don't over-release, and avoid underflow. + refCntUpdater.getAndAdd(this, decrement); + throw new IllegalReferenceCountException(oldRef, decrement); } + return false; } /** * Called once {@link #refCnt()} is equals 0. diff --git a/common/src/main/java/io/netty/util/AbstractReferenceCounted.java b/common/src/main/java/io/netty/util/AbstractReferenceCounted.java index 493bcc1872..5fae9dca11 100644 --- a/common/src/main/java/io/netty/util/AbstractReferenceCounted.java +++ b/common/src/main/java/io/netty/util/AbstractReferenceCounted.java @@ -52,17 +52,11 @@ public abstract class AbstractReferenceCounted implements ReferenceCounted { } private ReferenceCounted retain0(int increment) { - for (;;) { - int refCnt = this.refCnt; - final int nextCnt = refCnt + increment; - - // Ensure we not resurrect (which means the refCnt was 0) and also that we encountered an overflow. - if (nextCnt <= increment) { - throw new IllegalReferenceCountException(refCnt, increment); - } - if (refCntUpdater.compareAndSet(this, refCnt, nextCnt)) { - break; - } + int oldRef = refCntUpdater.getAndAdd(this, increment); + if (oldRef <= 0 || oldRef + increment < oldRef) { + // Ensure we don't resurrect (which means the refCnt was 0) and also that we encountered an overflow. + refCntUpdater.getAndAdd(this, -increment); + throw new IllegalReferenceCountException(oldRef, increment); } return this; } @@ -83,20 +77,16 @@ public abstract class AbstractReferenceCounted implements ReferenceCounted { } private boolean release0(int decrement) { - for (;;) { - int refCnt = this.refCnt; - if (refCnt < decrement) { - throw new IllegalReferenceCountException(refCnt, -decrement); - } - - if (refCntUpdater.compareAndSet(this, refCnt, refCnt - decrement)) { - if (refCnt == decrement) { - deallocate(); - return true; - } - return false; - } + int oldRef = refCntUpdater.getAndAdd(this, -decrement); + if (oldRef == decrement) { + deallocate(); + return true; + } else if (oldRef < decrement || oldRef - decrement > oldRef) { + // Ensure we don't over-release, and avoid underflow. + refCntUpdater.getAndAdd(this, decrement); + throw new IllegalReferenceCountException(oldRef, decrement); } + return false; } /** diff --git a/microbench/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBufBenchmark.java b/microbench/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBufBenchmark.java new file mode 100644 index 0000000000..985eb7056a --- /dev/null +++ b/microbench/src/main/java/io/netty/buffer/AbstractReferenceCountedByteBufBenchmark.java @@ -0,0 +1,69 @@ +/* + * Copyright 2017 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 + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.buffer; + +import io.netty.microbench.util.AbstractMicrobenchmark; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.GroupThreads; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +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.concurrent.TimeUnit; + +@State(Scope.Benchmark) +public class AbstractReferenceCountedByteBufBenchmark extends AbstractMicrobenchmark { + + @Param({ "1", "10", "100", "1000", "10000" }) + public int delay; + + AbstractReferenceCountedByteBuf buf; + + @Setup + public void setUp() { + buf = (AbstractReferenceCountedByteBuf) Unpooled.buffer(1); + } + + @TearDown + public void tearDown() { + buf.release(); + } + + @Benchmark + @BenchmarkMode(Mode.SampleTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public boolean retainReleaseUncontended() { + buf.retain(); + Blackhole.consumeCPU(delay); + return buf.release(); + } + + @Benchmark + @BenchmarkMode(Mode.SampleTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + @GroupThreads(6) + public boolean retainReleaseContended() { + buf.retain(); + Blackhole.consumeCPU(delay); + return buf.release(); + } +}