From 44b2657c8effc6eb5e8724a033c7a2f71b8dd6d0 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 3 Jul 2018 17:05:38 +0200 Subject: [PATCH] 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 --- .../util/ConcurrentReferenceHashMap.java | 118 ++++++++++-------- 1 file changed, 65 insertions(+), 53 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java b/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java index cee33acdf98..e1e3a7f81d0 100644 --- a/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java +++ b/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java @@ -52,6 +52,7 @@ import java.util.concurrent.locks.ReentrantLock; * {@linkplain SoftReference soft entry references}. * * @author Phillip Webb + * @author Juergen Hoeller * @since 3.2 * @param the key type * @param the value type @@ -94,7 +95,7 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen /** * Late binding entry set. */ - private Set> entrySet; + private volatile Set> entrySet; /** @@ -163,8 +164,8 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen * @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"); @@ -211,7 +212,7 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen * @return the resulting hash code */ protected int getHash(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); @@ -240,8 +241,8 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen } private Entry getEntryIfAvailable(Object key) { - Reference reference = getReference(key, Restructure.WHEN_NECESSARY); - return (reference != null ? reference.get() : null); + Reference ref = getReference(key, Restructure.WHEN_NECESSARY); + return (ref != null ? ref.get() : null); } /** @@ -269,13 +270,13 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen private V put(final K key, final V value, final boolean overwriteExisting) { return doTask(key, new Task(TaskOption.RESTRUCTURE_BEFORE, TaskOption.RESIZE) { @Override - protected V execute(Reference reference, Entry entry, Entries entries) { + protected V execute(Reference ref, Entry entry, Entries entries) { if (entry != null) { - V previousValue = entry.getValue(); + V oldValue = entry.getValue(); if (overwriteExisting) { entry.setValue(value); } - return previousValue; + return oldValue; } entries.add(value); return null; @@ -287,9 +288,9 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen public V remove(Object key) { return doTask(key, new Task(TaskOption.RESTRUCTURE_AFTER, TaskOption.SKIP_IF_EMPTY) { @Override - protected V execute(Reference reference, Entry entry) { + protected V execute(Reference ref, Entry entry) { if (entry != null) { - reference.release(); + ref.release(); return entry.value; } return null; @@ -301,9 +302,9 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen public boolean remove(Object key, final Object value) { return doTask(key, new Task(TaskOption.RESTRUCTURE_AFTER, TaskOption.SKIP_IF_EMPTY) { @Override - protected Boolean execute(Reference reference, Entry entry) { + protected Boolean execute(Reference ref, Entry entry) { if (entry != null && ObjectUtils.nullSafeEquals(entry.getValue(), value)) { - reference.release(); + ref.release(); return true; } return false; @@ -315,7 +316,7 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen public boolean replace(K key, final V oldValue, final V newValue) { return doTask(key, new Task(TaskOption.RESTRUCTURE_BEFORE, TaskOption.SKIP_IF_EMPTY) { @Override - protected Boolean execute(Reference reference, Entry entry) { + protected Boolean execute(Reference ref, Entry entry) { if (entry != null && ObjectUtils.nullSafeEquals(entry.getValue(), oldValue)) { entry.setValue(newValue); return true; @@ -329,11 +330,11 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen public V replace(K key, final V value) { return doTask(key, new Task(TaskOption.RESTRUCTURE_BEFORE, TaskOption.SKIP_IF_EMPTY) { @Override - protected V execute(Reference reference, Entry entry) { + protected V execute(Reference ref, Entry entry) { if (entry != null) { - V previousValue = entry.getValue(); + V oldValue = entry.getValue(); entry.setValue(value); - return previousValue; + return oldValue; } return null; } @@ -370,11 +371,23 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen } @Override - public Set> entrySet() { - if (this.entrySet == null) { - this.entrySet = new EntrySet(); + public boolean isEmpty() { + for (Segment segment : this.segments) { + if (segment.getCount() > 0) { + return false; + } + } + return true; + } + + @Override + public Set> entrySet() { + Set> entrySet = this.entrySet; + if (entrySet == null) { + entrySet = new EntrySet(); + this.entrySet = entrySet; } - return this.entrySet; + return entrySet; } private T doTask(Object key, Task task) { @@ -485,8 +498,8 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen try { final int index = getIndex(hash, this.references); final Reference head = this.references[index]; - Reference reference = findInChain(head, key, hash); - Entry entry = (reference != null ? reference.get() : null); + Reference ref = findInChain(head, key, hash); + Entry entry = (ref != null ? ref.get() : null); Entries entries = new Entries() { @Override public void add(V value) { @@ -497,7 +510,7 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen Segment.this.count++; } }; - return task.execute(reference, entry, entries); + return task.execute(ref, entry, entries); } finally { unlock(); @@ -531,19 +544,18 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen * @param allowResize if resizing is permitted */ protected final void restructureIfNecessary(boolean allowResize) { - boolean needsResize = ((this.count > 0) && (this.count >= this.resizeThreshold)); - Reference reference = this.referenceManager.pollForPurge(); - if ((reference != null) || (needsResize && allowResize)) { + boolean needsResize = (this.count > 0 && this.count >= this.resizeThreshold); + Reference ref = this.referenceManager.pollForPurge(); + if (ref != null || (needsResize && allowResize)) { lock(); try { int countAfterRestructure = this.count; - Set> 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(); @@ -559,22 +571,22 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen } // Either create a new table or reuse the existing one - Reference[] restructured = (resizing ? createReferenceArray(restructureSize) : this.references); + Reference[] 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) && (reference.get() != null)) { - int index = getIndex(reference.getHash(), restructured); + while (ref != null) { + if (!toPurge.contains(ref) && (ref.get() != null)) { + int index = getIndex(ref.getHash(), restructured); restructured[index] = this.referenceManager.createReference( - reference.get(), reference.getHash(), - restructured[index]); + ref.get(), ref.getHash(), restructured[index]); } - reference = reference.getNext(); + ref = ref.getNext(); } } @@ -590,8 +602,8 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen } } - private Reference findInChain(Reference reference, Object key, int hash) { - Reference currRef = reference; + private Reference findInChain(Reference ref, Object key, int hash) { + Reference currRef = ref; while (currRef != null) { if (currRef.getHash() == hash) { Entry entry = currRef.get(); @@ -744,24 +756,24 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen /** * Execute the task. - * @param reference the found reference 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) */ - protected T execute(Reference reference, Entry entry, Entries entries) { - return execute(reference, entry); + protected T execute(Reference ref, Entry entry, 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 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) */ - protected T execute(Reference reference, Entry entry) { + protected T execute(Reference ref, Entry entry) { return null; } } @@ -801,12 +813,12 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen @Override public boolean contains(Object o) { - if (o != null && o instanceof Map.Entry) { - Map.Entry entry = (java.util.Map.Entry) o; - Reference reference = ConcurrentReferenceHashMap.this.getReference(entry.getKey(), Restructure.NEVER); - Entry other = (reference != null ? reference.get() : null); - if (other != null) { - return ObjectUtils.nullSafeEquals(entry.getValue(), other.getValue()); + if (o instanceof Map.Entry) { + Map.Entry entry = (Map.Entry) o; + Reference ref = ConcurrentReferenceHashMap.this.getReference(entry.getKey(), Restructure.NEVER); + Entry otherEntry = (ref != null ? ref.get() : null); + if (otherEntry != null) { + return ObjectUtils.nullSafeEquals(otherEntry.getValue(), otherEntry.getValue()); } } return false;