Pad the drain status field

Profiling shows that this field is hot, especially after the last change which avoids the maintenance penalty on calling threads. By padding this field the read performance is increased due to avoiding false sharing of the cache line.
ben-manes · May 9, 2015 · d11df1a · d11df1a
1 parent 1bbfc86
commit d11df1a
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 43 deletions.
diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedBuffer.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedBuffer.java
@@ -57,7 +57,7 @@ protected Buffer<E> create(E e) {
     return new RingBuffer<>(e);
   }
 
-  static final class RingBuffer<E> extends ReadAndWriteCounterRef implements Buffer<E> {
+  static final class RingBuffer<E> extends BBHeader.ReadAndWriteCounterRef implements Buffer<E> {
     final AtomicReference<E>[] buffer;
 
     @SuppressWarnings({"unchecked", "cast", "rawtypes"})
@@ -121,44 +121,48 @@ public int writes() {
   }
 }
 
-abstract class PadReadCounter {
-  long p00, p01, p02, p03, p04, p05, p06, p07;
-  long p30, p31, p32, p33, p34, p35, p36, p37;
-}
+/** The namespace for field padding through inheritance. */
+final class BBHeader {
+
+  static abstract class PadReadCounter {
+    long p00, p01, p02, p03, p04, p05, p06, p07;
+    long p30, p31, p32, p33, p34, p35, p36, p37;
+  }
 
-/** Enforces a memory layout to avoid false sharing by padding the read count. */
-abstract class ReadCounterRef extends PadReadCounter {
-  static final long READ_OFFSET =
-      UnsafeAccess.objectFieldOffset(ReadCounterRef.class, "readCounter");
+  /** Enforces a memory layout to avoid false sharing by padding the read count. */
+  static abstract class ReadCounterRef extends PadReadCounter {
+    static final long READ_OFFSET =
+        UnsafeAccess.objectFieldOffset(ReadCounterRef.class, "readCounter");
 
-  volatile long readCounter;
+    volatile long readCounter;
 
-  void lazySetReadCounter(long count) {
-    UnsafeAccess.UNSAFE.putOrderedLong(this, READ_OFFSET, count);
+    void lazySetReadCounter(long count) {
+      UnsafeAccess.UNSAFE.putOrderedLong(this, READ_OFFSET, count);
+    }
   }
-}
 
-abstract class PadWriteCounter extends ReadCounterRef {
-  long p00, p01, p02, p03, p04, p05, p06, p07;
-  long p30, p31, p32, p33, p34, p35, p36, p37;
-}
+  static abstract class PadWriteCounter extends ReadCounterRef {
+    long p00, p01, p02, p03, p04, p05, p06, p07;
+    long p30, p31, p32, p33, p34, p35, p36, p37;
+  }
 
-/** Enforces a memory layout to avoid false sharing by padding the write count. */
-abstract class ReadAndWriteCounterRef extends PadWriteCounter {
-  static final long WRITE_OFFSET =
-      UnsafeAccess.objectFieldOffset(ReadAndWriteCounterRef.class, "writeCounter");
+  /** Enforces a memory layout to avoid false sharing by padding the write count. */
+  static abstract class ReadAndWriteCounterRef extends PadWriteCounter {
+    static final long WRITE_OFFSET =
+        UnsafeAccess.objectFieldOffset(ReadAndWriteCounterRef.class, "writeCounter");
 
-  volatile long writeCounter;
+    volatile long writeCounter;
 
-  ReadAndWriteCounterRef(int writes) {
-    UnsafeAccess.UNSAFE.putOrderedLong(this, WRITE_OFFSET, writes);
-  }
+    ReadAndWriteCounterRef(int writes) {
+      UnsafeAccess.UNSAFE.putOrderedLong(this, WRITE_OFFSET, writes);
+    }
 
-  long relaxedTail() {
-    return UnsafeAccess.UNSAFE.getLong(this, WRITE_OFFSET);
-  }
+    long relaxedTail() {
+      return UnsafeAccess.UNSAFE.getLong(this, WRITE_OFFSET);
+    }
 
-  boolean casWriteCounter(long expect, long update) {
-    return UnsafeAccess.UNSAFE.compareAndSwapLong(this, WRITE_OFFSET, expect, update);
+    boolean casWriteCounter(long expect, long update) {
+      return UnsafeAccess.UNSAFE.compareAndSwapLong(this, WRITE_OFFSET, expect, update);
+    }
   }
 }
diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java
@@ -48,7 +48,6 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.BiFunction;
@@ -61,6 +60,8 @@
 import javax.annotation.concurrent.GuardedBy;
 import javax.annotation.concurrent.ThreadSafe;
 
+import com.github.benmanes.caffeine.base.UnsafeAccess;
+import com.github.benmanes.caffeine.cache.BoundedLocalCache.DrainStatus;
 import com.github.benmanes.caffeine.cache.References.InternalReference;
 import com.github.benmanes.caffeine.cache.stats.DisabledStatsCounter;
 import com.github.benmanes.caffeine.cache.stats.StatsCounter;
@@ -80,7 +81,8 @@
  * @param <V> the type of mapped values
  */
 @ThreadSafe
-abstract class BoundedLocalCache<K, V> extends AbstractMap<K, V> implements LocalCache<K, V> {
+abstract class BoundedLocalCache<K, V> extends BLCHeader.DrainStatusRef<K, V>
+    implements LocalCache<K, V> {
 
   /*
    * This class performs a best-effort bounding of a ConcurrentHashMap using a page-replacement
@@ -119,7 +121,6 @@ abstract class BoundedLocalCache<K, V> extends AbstractMap<K, V> implements Loca
   static final long MAXIMUM_CAPACITY = Long.MAX_VALUE - Integer.MAX_VALUE;
 
   final ConcurrentHashMap<Object, Node<K, V>> data;
-  final AtomicReference<DrainStatus> drainStatus;
   final Consumer<Node<K, V>> accessPolicy;
   final Buffer<Node<K, V>> readBuffer;
   final Runnable drainBuffersTask;
@@ -139,7 +140,6 @@ protected BoundedLocalCache(Caffeine<K, V> builder, boolean isAsync) {
     this.isAsync = isAsync;
     weigher = builder.getWeigher(isAsync);
     id = tracer().register(builder.name());
-    drainStatus = new AtomicReference<DrainStatus>(IDLE);
     data = new ConcurrentHashMap<>(builder.getInitialCapacity());
     evictionLock = builder.hasExecutor() ? new ReentrantLock() : new NonReentrantLock();
     nodeFactory = NodeFactory.getFactory(builder.isStrongKeys(), builder.isWeakKeys(),
@@ -495,7 +495,7 @@ void refreshIfNeeded(Node<K, V> node, long now) {
    * @param delayable if draining the read buffer can be delayed
    */
   void drainOnReadIfNeeded(boolean delayable) {
-    final DrainStatus status = drainStatus.get();
+    final DrainStatus status = drainStatus;
     if (status.shouldDrainBuffers(delayable)) {
       scheduleDrainBuffers();
     }
@@ -516,7 +516,7 @@ void afterWrite(@Nullable Node<K, V> node, Runnable task) {
     if (buffersWrites()) {
       writeQueue().add(task);
     }
-    drainStatus.lazySet(REQUIRED);
+    lazySetDrainStatus(REQUIRED);
     scheduleDrainBuffers();
   }
 
@@ -527,7 +527,7 @@ void afterWrite(@Nullable Node<K, V> node, Runnable task) {
   void scheduleDrainBuffers() {
     if (evictionLock.tryLock()) {
       try {
-        drainStatus.lazySet(PROCESSING);
+        lazySetDrainStatus(PROCESSING);
         executor().execute(drainBuffersTask);
       } catch (Throwable t) {
         cleanUp();
@@ -542,10 +542,10 @@ void scheduleDrainBuffers() {
   public void cleanUp() {
     evictionLock.lock();
     try {
-      drainStatus.lazySet(PROCESSING);
+      lazySetDrainStatus(PROCESSING);
       maintenance();
     } finally {
-      drainStatus.compareAndSet(PROCESSING, IDLE);
+      casDrainStatus(PROCESSING, IDLE);
       evictionLock.unlock();
     }
   }
@@ -1933,3 +1933,28 @@ Object writeReplace() {
     }
   }
 }
+
+/** The namespace for field padding through inheritance. */
+final class BLCHeader {
+
+  static abstract class PadDrainStatus<K, V> extends AbstractMap<K, V> {
+    long p00, p01, p02, p03, p04, p05, p06, p07;
+    long p30, p31, p32, p33, p34, p35, p36, p37;
+  }
+
+  /** Enforces a memory layout to avoid false sharing by padding the drain status. */
+  static abstract class DrainStatusRef<K, V> extends PadDrainStatus<K, V> {
+    static final long DRAIN_STATUS_OFFSET =
+        UnsafeAccess.objectFieldOffset(DrainStatusRef.class, "drainStatus");
+
+    volatile DrainStatus drainStatus = IDLE;
+
+    void lazySetDrainStatus(DrainStatus drainStatus) {
+      UnsafeAccess.UNSAFE.putOrderedObject(this, DRAIN_STATUS_OFFSET, drainStatus);
+    }
+
+    boolean casDrainStatus(DrainStatus expect, DrainStatus update) {
+      return UnsafeAccess.UNSAFE.compareAndSwapObject(this, DRAIN_STATUS_OFFSET, expect, update);
+    }
+  }
+}
diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java
@@ -316,7 +316,7 @@ public void drain_nonblocking(Cache<Integer, Integer> cache) {
     BoundedLocalCache<Integer, Integer> localCache = asBoundedLocalCache(cache);
     AtomicBoolean done = new AtomicBoolean();
     Runnable task = () -> {
-      localCache.drainStatus.lazySet(DrainStatus.REQUIRED);
+      localCache.lazySetDrainStatus(DrainStatus.REQUIRED);
       localCache.scheduleDrainBuffers();
       done.set(true);
     };
@@ -360,7 +360,7 @@ void checkDrainBlocks(BoundedLocalCache<Integer, Integer> localCache, Runnable t
     lock.lock();
     try {
       executor.execute(() -> {
-        localCache.drainStatus.lazySet(DrainStatus.REQUIRED);
+        localCache.lazySetDrainStatus(DrainStatus.REQUIRED);
         task.run();
         done.set(true);
       });

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/Stresser.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/Stresser.java
@@ -98,7 +98,7 @@ public void run() {
         System.out.printf("---------- %s ----------%n", elapsedTime);
         System.out.printf("Pending reads = %s%n", pendingReads);
         System.out.printf("Pending write = %s%n", pendingWrites);
-        System.out.printf("Drain status = %s%n", local.drainStatus.get());
+        System.out.printf("Drain status = %s%n", local.drainStatus);
         System.out.printf("Evictions = %,d%n", evictions.intValue());
         System.out.printf("Lock = %s%n", local.evictionLock);
       }

diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
@@ -34,7 +34,7 @@ ext {
     jcache: '1.0.0',
     joor: '0.9.5',
     jsr305: '3.0.0',
-    univocity_parsers: '1.5.2',
+    univocity_parsers: '1.5.4',
   ]
   test_versions = [
     awaitility: '1.6.3',

diff --git a/wiki/read.png b/wiki/read.png
diff --git a/wiki/readwrite.png b/wiki/readwrite.png