ConcurrentReferenceHashMap caches EntrySet in volatile field

Includes an efficient implementation of isEmpty(), not relying on a full entry count but rather backing out once a non-empty hash segment has been found.

Issue: SPR-16994

Author: jhoeller

spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java

@@ -98,7 +98,7 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
 	 * Late binding entry set.
 	 */
 	@Nullable
-	private Set<Map.Entry<K, V>> entrySet;
+	private volatile Set<Map.Entry<K, V>> entrySet;
 
 
 	/**
@@ -167,8 +167,8 @@ public ConcurrentReferenceHashMap(int initialCapacity, float loadFactor, int con
 	 * @param referenceType the reference type used for entries (soft or weak)
 	 */
 	@SuppressWarnings("unchecked")
-	public ConcurrentReferenceHashMap(int initialCapacity, float loadFactor, int concurrencyLevel,
-			ReferenceType referenceType) {
+	public ConcurrentReferenceHashMap(
+			int initialCapacity, float loadFactor, int concurrencyLevel, ReferenceType referenceType) {
 
 		Assert.isTrue(initialCapacity >= 0, "Initial capacity must not be negative");
 		Assert.isTrue(loadFactor > 0f, "Load factor must be positive");
@@ -215,7 +215,7 @@ protected ReferenceManager createReferenceManager() {
 	 * @return the resulting hash code
 	 */
 	protected int getHash(@Nullable Object o) {
-		int hash = o == null ? 0 : o.hashCode();
+		int hash = (o != null ? o.hashCode() : 0);
 		hash += (hash << 15) ^ 0xffffcd7d;
 		hash ^= (hash >>> 10);
 		hash += (hash << 3);
@@ -247,8 +247,8 @@ public boolean containsKey(@Nullable Object key) {
 
 	@Nullable
 	private Entry<K, V> getEntryIfAvailable(@Nullable Object key) {
-		Reference<K, V> reference = getReference(key, Restructure.WHEN_NECESSARY);
-		return (reference != null ? reference.get() : null);
+		Reference<K, V> ref = getReference(key, Restructure.WHEN_NECESSARY);
+		return (ref != null ? ref.get() : null);
 	}
 
 	/**
@@ -281,7 +281,7 @@ private V put(@Nullable final K key, @Nullable final V value, final boolean over
 		return doTask(key, new Task<V>(TaskOption.RESTRUCTURE_BEFORE, TaskOption.RESIZE) {
 			@Override
 			@Nullable
-			protected V execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> entry, @Nullable Entries entries) {
+			protected V execute(@Nullable Reference<K, V> ref, @Nullable Entry<K, V> entry, @Nullable Entries entries) {
 				if (entry != null) {
 					V oldValue = entry.getValue();
 					if (overwriteExisting) {
@@ -302,10 +302,10 @@ public V remove(Object key) {
 		return doTask(key, new Task<V>(TaskOption.RESTRUCTURE_AFTER, TaskOption.SKIP_IF_EMPTY) {
 			@Override
 			@Nullable
-			protected V execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> entry) {
+			protected V execute(@Nullable Reference<K, V> ref, @Nullable Entry<K, V> entry) {
 				if (entry != null) {
-					if (reference != null) {
-						reference.release();
+					if (ref != null) {
+						ref.release();
 					}
 					return entry.value;
 				}
@@ -318,10 +318,10 @@ protected V execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> e
 	public boolean remove(Object key, final Object value) {
 		Boolean result = doTask(key, new Task<Boolean>(TaskOption.RESTRUCTURE_AFTER, TaskOption.SKIP_IF_EMPTY) {
 			@Override
-			protected Boolean execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> entry) {
+			protected Boolean execute(@Nullable Reference<K, V> ref, @Nullable Entry<K, V> entry) {
 				if (entry != null && ObjectUtils.nullSafeEquals(entry.getValue(), value)) {
-					if (reference != null) {
-						reference.release();
+					if (ref != null) {
+						ref.release();
 					}
 					return true;
 				}
@@ -335,7 +335,7 @@ protected Boolean execute(@Nullable Reference<K, V> reference, @Nullable Entry<K
 	public boolean replace(K key, final V oldValue, final V newValue) {
 		Boolean result = doTask(key, new Task<Boolean>(TaskOption.RESTRUCTURE_BEFORE, TaskOption.SKIP_IF_EMPTY) {
 			@Override
-			protected Boolean execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> entry) {
+			protected Boolean execute(@Nullable Reference<K, V> ref, @Nullable Entry<K, V> entry) {
 				if (entry != null && ObjectUtils.nullSafeEquals(entry.getValue(), oldValue)) {
 					entry.setValue(newValue);
 					return true;
@@ -352,7 +352,7 @@ public V replace(K key, final V value) {
 		return doTask(key, new Task<V>(TaskOption.RESTRUCTURE_BEFORE, TaskOption.SKIP_IF_EMPTY) {
 			@Override
 			@Nullable
-			protected V execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> entry) {
+			protected V execute(@Nullable Reference<K, V> ref, @Nullable Entry<K, V> entry) {
 				if (entry != null) {
 					V oldValue = entry.getValue();
 					entry.setValue(value);
@@ -393,11 +393,23 @@ public int size() {
 	}
 
 	@Override
-	public Set<java.util.Map.Entry<K, V>> entrySet() {
-		if (this.entrySet == null) {
-			this.entrySet = new EntrySet();
+	public boolean isEmpty() {
+		for (Segment segment : this.segments) {
+			if (segment.getCount() > 0) {
+				return false;
+			}
 		}
-		return this.entrySet;
+		return true;
+	}
+
+	@Override
+	public Set<Map.Entry<K, V>> entrySet() {
+		Set<Map.Entry<K, V>> entrySet = this.entrySet;
+		if (entrySet == null) {
+			entrySet = new EntrySet();
+			this.entrySet = entrySet;
+		}
+		return entrySet;
 	}
 
 	@Nullable
@@ -512,8 +524,8 @@ public <T> T doTask(final int hash, @Nullable final Object key, final Task<T> ta
 			try {
 				final int index = getIndex(hash, this.references);
 				final Reference<K, V> head = this.references[index];
-				Reference<K, V> reference = findInChain(head, key, hash);
-				Entry<K, V> entry = (reference != null ? reference.get() : null);
+				Reference<K, V> ref = findInChain(head, key, hash);
+				Entry<K, V> entry = (ref != null ? ref.get() : null);
 				Entries entries = new Entries() {
 					@Override
 					public void add(@Nullable V value) {
@@ -524,7 +536,7 @@ public void add(@Nullable V value) {
 						Segment.this.count++;
 					}
 				};
-				return task.execute(reference, entry, entries);
+				return task.execute(ref, entry, entries);
 			}
 			finally {
 				unlock();
@@ -559,19 +571,18 @@ public void clear() {
 		 * @param allowResize if resizing is permitted
 		 */
 		protected final void restructureIfNecessary(boolean allowResize) {
-			boolean needsResize = ((this.count > 0) && (this.count >= this.resizeThreshold));
-			Reference<K, V> reference = this.referenceManager.pollForPurge();
-			if ((reference != null) || (needsResize && allowResize)) {
+			boolean needsResize = (this.count > 0 && this.count >= this.resizeThreshold);
+			Reference<K, V> ref = this.referenceManager.pollForPurge();
+			if (ref != null || (needsResize && allowResize)) {
 				lock();
 				try {
 					int countAfterRestructure = this.count;
-
 					Set<Reference<K, V>> toPurge = Collections.emptySet();
-					if (reference != null) {
+					if (ref != null) {
 						toPurge = new HashSet<>();
-						while (reference != null) {
-							toPurge.add(reference);
-							reference = this.referenceManager.pollForPurge();
+						while (ref != null) {
+							toPurge.add(ref);
+							ref = this.referenceManager.pollForPurge();
 						}
 					}
 					countAfterRestructure -= toPurge.size();
@@ -587,24 +598,25 @@ protected final void restructureIfNecessary(boolean allowResize) {
 					}
 
 					// Either create a new table or reuse the existing one
-					Reference<K, V>[] restructured = (resizing ? createReferenceArray(restructureSize) : this.references);
+					Reference<K, V>[] restructured =
+							(resizing ? createReferenceArray(restructureSize) : this.references);
 
 					// Restructure
 					for (int i = 0; i < this.references.length; i++) {
-						reference = this.references[i];
+						ref = this.references[i];
 						if (!resizing) {
 							restructured[i] = null;
 						}
-						while (reference != null) {
-							if (!toPurge.contains(reference)) {
-								Entry<K, V> entry = reference.get();
+						while (ref != null) {
+							if (!toPurge.contains(ref)) {
+								Entry<K, V> entry = ref.get();
 								if (entry != null) {
-									int index = getIndex(reference.getHash(), restructured);
+									int index = getIndex(ref.getHash(), restructured);
 									restructured[index] = this.referenceManager.createReference(
-											entry, reference.getHash(), restructured[index]);
+											entry, ref.getHash(), restructured[index]);
 								}
 							}
-							reference = reference.getNext();
+							ref = ref.getNext();
 						}
 					}
 
@@ -622,8 +634,8 @@ protected final void restructureIfNecessary(boolean allowResize) {
 		}
 
 		@Nullable
-		private Reference<K, V> findInChain(Reference<K, V> reference, @Nullable Object key, int hash) {
-			Reference<K, V> currRef = reference;
+		private Reference<K, V> findInChain(Reference<K, V> ref, @Nullable Object key, int hash) {
+			Reference<K, V> currRef = ref;
 			while (currRef != null) {
 				if (currRef.getHash() == hash) {
 					Entry<K, V> entry = currRef.get();
@@ -774,26 +786,26 @@ public boolean hasOption(TaskOption option) {
 
 		/**
 		 * Execute the task.
-		 * @param reference the found reference or {@code null}
-		 * @param entry the found entry or {@code null}
+		 * @param ref the found reference (or {@code null})
+		 * @param entry the found entry (or {@code null})
 		 * @param entries access to the underlying entries
 		 * @return the result of the task
 		 * @see #execute(Reference, Entry)
 		 */
 		@Nullable
-		protected T execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> entry, @Nullable Entries entries) {
-			return execute(reference, entry);
+		protected T execute(@Nullable Reference<K, V> ref, @Nullable Entry<K, V> entry, @Nullable Entries entries) {
+			return execute(ref, entry);
 		}
 
 		/**
 		 * Convenience method that can be used for tasks that do not need access to {@link Entries}.
-		 * @param reference the found reference or {@code null}
-		 * @param entry the found entry or {@code null}
+		 * @param ref the found reference (or {@code null})
+		 * @param entry the found entry (or {@code null})
 		 * @return the result of the task
 		 * @see #execute(Reference, Entry, Entries)
 		 */
 		@Nullable
-		protected T execute(@Nullable Reference<K, V> reference, @Nullable Entry<K, V> entry) {
+		protected T execute(@Nullable Reference<K, V> ref, @Nullable Entry<K, V> entry) {
 			return null;
 		}
 	}
@@ -834,9 +846,9 @@ public Iterator<Map.Entry<K, V>> iterator() {
 		@Override
 		public boolean contains(@Nullable Object o) {
 			if (o instanceof Map.Entry<?, ?>) {
-				Map.Entry<?, ?> entry = (java.util.Map.Entry<?, ?>) o;
-				Reference<K, V> reference = ConcurrentReferenceHashMap.this.getReference(entry.getKey(), Restructure.NEVER);
-				Entry<K, V> otherEntry = (reference != null ? reference.get() : null);
+				Map.Entry<?, ?> entry = (Map.Entry<?, ?>) o;
+				Reference<K, V> ref = ConcurrentReferenceHashMap.this.getReference(entry.getKey(), Restructure.NEVER);
+				Entry<K, V> otherEntry = (ref != null ? ref.get() : null);
 				if (otherEntry != null) {
 					return ObjectUtils.nullSafeEquals(otherEntry.getValue(), otherEntry.getValue());
 				}