From 2d9e0f53a5059765f5f5976472440c0a2fec848d Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 13 May 2014 14:23:23 +0200 Subject: [PATCH] =?UTF-8?q?Better=20implementation=20of=20AttributeMap=20a?= =?UTF-8?q?nd=20also=20add=20hasAttr(...).=20See=C2=A0[#2439]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: The old DefaultAttributeMap impl did more synchronization then needed and also did not expose a efficient way to check if an attribute exists with a specific key. Modifications: * Rewrite DefaultAttributeMap to not use IdentityHashMap and synchronization on the map directly. The new impl uses a combination of AtomicReferenceArray and synchronization per chain (linked-list). Also access the first Attribute per bucket can be done without any synchronization at all and just uses atomic operations. This should fit for most use-cases pretty weel. * Add hasAttr(...) implementation Result: It's now possible to check for the existence of a attribute without create one. Synchronization is per linked-list and the first entry can even be added via atomic operation. --- .../main/java/io/netty/util/AttributeMap.java | 5 + .../io/netty/util/DefaultAttributeMap.java | 145 +++++++++++++++--- .../channel/DefaultChannelHandlerContext.java | 5 + 3 files changed, 130 insertions(+), 25 deletions(-) diff --git a/common/src/main/java/io/netty/util/AttributeMap.java b/common/src/main/java/io/netty/util/AttributeMap.java index 0524b1f287..826e6959a9 100644 --- a/common/src/main/java/io/netty/util/AttributeMap.java +++ b/common/src/main/java/io/netty/util/AttributeMap.java @@ -26,4 +26,9 @@ public interface AttributeMap { * an {@link Attribute} which does not have a value set yet. */ Attribute attr(AttributeKey key); + + /** + * Returns {@code} true if and only if the given {@link Attribute} exists in this {@link AttributeMap}. + */ + boolean hasAttr(AttributeKey key); } diff --git a/common/src/main/java/io/netty/util/DefaultAttributeMap.java b/common/src/main/java/io/netty/util/DefaultAttributeMap.java index 806610a035..a2ec5ea57e 100644 --- a/common/src/main/java/io/netty/util/DefaultAttributeMap.java +++ b/common/src/main/java/io/netty/util/DefaultAttributeMap.java @@ -17,65 +17,147 @@ package io.netty.util; import io.netty.util.internal.PlatformDependent; -import java.util.IdentityHashMap; -import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; /** - * Default {@link AttributeMap} implementation which use simple synchronization to keep the memory overhead + * Default {@link AttributeMap} implementation which use simple synchronization per bucket to keep the memory overhead * as low as possible. */ public class DefaultAttributeMap implements AttributeMap { @SuppressWarnings("rawtypes") - private static final AtomicReferenceFieldUpdater updater; + private static final AtomicReferenceFieldUpdater updater; static { @SuppressWarnings("rawtypes") - AtomicReferenceFieldUpdater referenceFieldUpdater = - PlatformDependent.newAtomicReferenceFieldUpdater(DefaultAttributeMap.class, "map"); + AtomicReferenceFieldUpdater referenceFieldUpdater = + PlatformDependent.newAtomicReferenceFieldUpdater(DefaultAttributeMap.class, "attributes"); if (referenceFieldUpdater == null) { - referenceFieldUpdater = AtomicReferenceFieldUpdater.newUpdater(DefaultAttributeMap.class, Map.class, "map"); + referenceFieldUpdater = AtomicReferenceFieldUpdater + .newUpdater(DefaultAttributeMap.class, AtomicReferenceArray.class, "attributes"); } updater = referenceFieldUpdater; } + private static final int BUCKET_SIZE = 4; + private static final int MASK = BUCKET_SIZE - 1; + // Initialize lazily to reduce memory consumption; updated by AtomicReferenceFieldUpdater above. @SuppressWarnings("UnusedDeclaration") - private volatile Map, Attribute> map; + private volatile AtomicReferenceArray> attributes; + @SuppressWarnings({ "unchecked", "rawtypes" }) @Override public Attribute attr(AttributeKey key) { - Map, Attribute> map = this.map; - if (map == null) { + if (key == null) { + throw new NullPointerException("key"); + } + AtomicReferenceArray> attributes = this.attributes; + if (attributes == null) { // Not using ConcurrentHashMap due to high memory consumption. - map = new IdentityHashMap, Attribute>(2); - if (!updater.compareAndSet(this, null, map)) { - map = this.map; + attributes = new AtomicReferenceArray>(BUCKET_SIZE); + + if (!updater.compareAndSet(this, null, attributes)) { + attributes = this.attributes; } } - synchronized (map) { - @SuppressWarnings("unchecked") - Attribute attr = (Attribute) map.get(key); - if (attr == null) { - attr = new DefaultAttribute(map, key); - map.put(key, attr); + int i = index(key); + DefaultAttribute head = attributes.get(i); + if (head == null) { + // No head exists yet which means we may be able to add the attribute without synchronization and just + // use compare and set. At worst we need to fallback to synchronization + head = new DefaultAttribute(key); + if (attributes.compareAndSet(i, null, head)) { + // we were able to add it so return the head right away + return (Attribute) head; + } else { + head = attributes.get(i); + } + } + + synchronized (head) { + DefaultAttribute curr = head; + for (;;) { + if (!curr.removed && curr.key == key) { + return (Attribute) curr; + } + + DefaultAttribute next = curr.next; + if (next == null) { + DefaultAttribute attr = new DefaultAttribute(head, key); + curr.next = attr; + attr.prev = curr; + } } - return attr; } } + @Override + public boolean hasAttr(AttributeKey key) { + if (key == null) { + throw new NullPointerException("key"); + } + AtomicReferenceArray> attributes = this.attributes; + if (attributes == null) { + // no attribute exists + return false; + } + + int i = index(key); + DefaultAttribute head = attributes.get(i); + if (head == null) { + // No attribute exists which point to the bucket in which the head should be located + return false; + } + + // check on the head can be done without synchronization + if (head.key == key && !head.removed) { + return true; + } + + synchronized (head) { + // we need to synchronize on the head + DefaultAttribute curr = head.next; + while (curr != null) { + if (!curr.removed && curr.key == key) { + return true; + } + curr = curr.next; + } + return false; + } + } + + private static int index(AttributeKey key) { + return key.id() & MASK; + } + + @SuppressWarnings("serial") private static final class DefaultAttribute extends AtomicReference implements Attribute { private static final long serialVersionUID = -2661411462200283011L; - private final Map, Attribute> map; + // The head of the linked-list this attribute belongs to, which may be itself + private final DefaultAttribute head; private final AttributeKey key; - DefaultAttribute(Map, Attribute> map, AttributeKey key) { - this.map = map; + // Double-linked list to prev and next node to allow fast removal + private DefaultAttribute prev; + private DefaultAttribute next; + + // Will be set to true one the attribute is removed via getAndRemove() or remove() + private volatile boolean removed; + + DefaultAttribute(DefaultAttribute head, AttributeKey key) { + this.head = head; + this.key = key; + } + + DefaultAttribute(AttributeKey key) { + head = this; this.key = key; } @@ -97,6 +179,7 @@ public class DefaultAttributeMap implements AttributeMap { @Override public T getAndRemove() { + removed = true; T oldValue = getAndSet(null); remove0(); return oldValue; @@ -104,13 +187,25 @@ public class DefaultAttributeMap implements AttributeMap { @Override public void remove() { + removed = true; set(null); remove0(); } private void remove0() { - synchronized (map) { - map.remove(key); + synchronized (head) { + // We only update the linked-list structure if prev != null because if it is null this + // DefaultAttribute acts also as head. The head must never be removed completely and just be + // marked as removed as all synchronization is done on the head itself for each bucket. + // The head itself will be GC'ed once the DefaultAttributeMap is GC'ed. So at most 5 heads will + // be removed lazy as the array size is 5. + if (prev != null) { + prev.next = next; + + if (next != null) { + next.prev = prev; + } + } } } } diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java index 9d1cc85468..af489926bb 100644 --- a/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java +++ b/transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java @@ -137,6 +137,11 @@ final class DefaultChannelHandlerContext implements ChannelHandlerContext, Resou return channel.attr(key); } + @Override + public boolean hasAttr(AttributeKey key) { + return channel.hasAttr(key); + } + @Override public ChannelHandlerContext fireChannelRegistered() { DefaultChannelHandlerContext next = findContextInbound();