Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ConcurrentReferenceHashMap properly handles getOrDefault for null values

Browse Source 
Issue: SPR-16584

(cherry picked from commit 356ef45)
This commit is contained in:
Juergen Hoeller
2018-03-14 17:17:04 +01:00
parent 2cac3a8255
commit 8d8bb04783
2 changed files with 101 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -223,18 +223,27 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
@Override
public V get(Object key) {
Reference<K, V> reference = getReference(key, Restructure.WHEN_NECESSARY);
Entry<K, V> entry = (reference != null ? reference.get() : null);
Entry<K, V> entry = getEntryIfAvailable(key);
return (entry != null ? entry.getValue() : null);
}
@Override
public V getOrDefault(Object key, V defaultValue) {
Entry<K, V> entry = getEntryIfAvailable(key);
return (entry != null ? entry.getValue() : defaultValue);
}
@Override
public boolean containsKey(Object key) {
Reference<K, V> reference = getReference(key, Restructure.WHEN_NECESSARY);
Entry<K, V> entry = (reference != null ? reference.get() : null);
Entry<K, V> entry = getEntryIfAvailable(key);
return (entry != null && ObjectUtils.nullSafeEquals(entry.getKey(), key));
}
private Entry<K, V> getEntryIfAvailable(Object key) {
Reference<K, V> reference = getReference(key, Restructure.WHEN_NECESSARY);
return (reference != null ? reference.get() : null);
}
/**
* Return a {@link Reference} to the {@link Entry} for the specified {@code key},
* or {@code null} if not found.
@@ -582,17 +591,18 @@ public class ConcurrentReferenceHashMap<K, V> extends AbstractMap<K, V> implemen
}
private Reference<K, V> findInChain(Reference<K, V> reference, Object key, int hash) {
while (reference != null) {
if (reference.getHash() == hash) {
Entry<K, V> entry = reference.get();
Reference<K, V> currRef = reference;
while (currRef != null) {
if (currRef.getHash() == hash) {
Entry<K, V> entry = currRef.get();
if (entry != null) {
K entryKey = entry.getKey();
if (entryKey == key || entryKey.equals(key)) {
return reference;
if (ObjectUtils.nullSafeEquals(entryKey, key)) {
return currRef;
}
}
}
reference = reference.getNext();
currRef = currRef.getNext();
}
return null;
}

168
spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -50,50 +50,50 @@ import static org.junit.Assert.*;
*/
public class ConcurrentReferenceHashMapTests {
private static final Comparator<? super String> NULL_SAFE_STRING_SORT = new NullSafeComparator<String>(
new ComparableComparator<String>(), true);
private static final Comparator<? super String> NULL_SAFE_STRING_SORT =
new NullSafeComparator<String>(new ComparableComparator<String>(), true);
@Rule
public ExpectedException thrown = ExpectedException.none();
private TestWeakConcurrentCache<Integer, String> map = new TestWeakConcurrentCache<Integer, String>();
private TestWeakConcurrentCache<Integer, String> map = new TestWeakConcurrentCache<>();
@Test
public void shouldCreateWithDefaults() throws Exception {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<Integer, String>();
public void shouldCreateWithDefaults() {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<>();
assertThat(map.getSegmentsSize(), is(16));
assertThat(map.getSegment(0).getSize(), is(1));
assertThat(map.getLoadFactor(), is(0.75f));
}
@Test
public void shouldCreateWithInitialCapacity() throws Exception {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<Integer, String>(32);
public void shouldCreateWithInitialCapacity() {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<>(32);
assertThat(map.getSegmentsSize(), is(16));
assertThat(map.getSegment(0).getSize(), is(2));
assertThat(map.getLoadFactor(), is(0.75f));
}
@Test
public void shouldCreateWithInitialCapacityAndLoadFactor() throws Exception {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<Integer, String>(32, 0.5f);
public void shouldCreateWithInitialCapacityAndLoadFactor() {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<>(32, 0.5f);
assertThat(map.getSegmentsSize(), is(16));
assertThat(map.getSegment(0).getSize(), is(2));
assertThat(map.getLoadFactor(), is(0.5f));
}
@Test
public void shouldCreateWithInitialCapacityAndConcurrenyLevel() throws Exception {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<Integer, String>(16, 2);
public void shouldCreateWithInitialCapacityAndConcurrenyLevel() {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<>(16, 2);
assertThat(map.getSegmentsSize(), is(2));
assertThat(map.getSegment(0).getSize(), is(8));
assertThat(map.getLoadFactor(), is(0.75f));
}
@Test
public void shouldCreateFullyCustom() throws Exception {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<Integer, String>(5, 0.5f, 3);
public void shouldCreateFullyCustom() {
ConcurrentReferenceHashMap<Integer, String> map = new ConcurrentReferenceHashMap<>(5, 0.5f, 3);
// concurrencyLevel of 3 ends up as 4 (nearest power of 2)
assertThat(map.getSegmentsSize(), is(4));
// initialCapacity is 5/4 (rounded up, to nearest power of 2)
@@ -102,7 +102,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldNeedNonNegativeInitialCapacity() throws Exception {
public void shouldNeedNonNegativeInitialCapacity() {
new ConcurrentReferenceHashMap<Integer, String>(0, 1);
this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("Initial capacity must not be negative");
@@ -110,7 +110,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldNeedPositiveLoadFactor() throws Exception {
public void shouldNeedPositiveLoadFactor() {
new ConcurrentReferenceHashMap<Integer, String>(0, 0.1f, 1);
this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("Load factor must be positive");
@@ -118,7 +118,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldNeedPositiveConcurrencyLevel() throws Exception {
public void shouldNeedPositiveConcurrencyLevel() {
new ConcurrentReferenceHashMap<Integer, String>(1, 1);
this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("Concurrency level must be positive");
@@ -126,7 +126,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldPutAndGet() throws Exception {
public void shouldPutAndGet() {
// NOTE we are using mock references so we don't need to worry about GC
assertThat(this.map.size(), is(0));
this.map.put(123, "123");
@@ -139,32 +139,40 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldReplaceOnDoublePut() throws Exception {
public void shouldReplaceOnDoublePut() {
this.map.put(123, "321");
this.map.put(123, "123");
assertThat(this.map.get(123), is("123"));
}
@Test
public void shouldPutNullKey() throws Exception {
public void shouldPutNullKey() {
assertThat(this.map.get(null), is(nullValue()));
assertThat(this.map.getOrDefault(null, "456"), is("456"));
this.map.put(null, "123");
assertThat(this.map.get(null), is("123"));
assertThat(this.map.getOrDefault(null, "456"), is("123"));
}
@Test
public void shouldPutNullValue() throws Exception {
public void shouldPutNullValue() {
assertThat(this.map.get(123), is(nullValue()));
assertThat(this.map.getOrDefault(123, "456"), is("456"));
this.map.put(123, "321");
assertThat(this.map.get(123), is("321"));
assertThat(this.map.getOrDefault(123, "456"), is("321"));
this.map.put(123, null);
assertThat(this.map.get(123), is(nullValue()));
assertThat(this.map.getOrDefault(123, "456"), is(nullValue()));
}
@Test
public void shouldGetWithNoItems() throws Exception {
public void shouldGetWithNoItems() {
assertThat(this.map.get(123), is(nullValue()));
}
@Test
public void shouldApplySupplimentalHash() throws Exception {
public void shouldApplySupplimentalHash() {
Integer key = 123;
this.map.put(key, "123");
assertThat(this.map.getSupplimentalHash(), is(not(key.hashCode())));
@@ -172,9 +180,9 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldGetFollowingNexts() throws Exception {
public void shouldGetFollowingNexts() {
// Use loadFactor to disable resize
this.map = new TestWeakConcurrentCache<Integer, String>(1, 10.0f, 1);
this.map = new TestWeakConcurrentCache<>(1, 10.0f, 1);
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
@@ -186,8 +194,8 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldResize() throws Exception {
this.map = new TestWeakConcurrentCache<Integer, String>(1, 0.75f, 1);
public void shouldResize() {
this.map = new TestWeakConcurrentCache<>(1, 0.75f, 1);
this.map.put(1, "1");
assertThat(this.map.getSegment(0).getSize(), is(1));
assertThat(this.map.get(1), is("1"));
@@ -216,8 +224,8 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldPurgeOnGet() throws Exception {
this.map = new TestWeakConcurrentCache<Integer, String>(1, 0.75f, 1);
public void shouldPurgeOnGet() {
this.map = new TestWeakConcurrentCache<>(1, 0.75f, 1);
for (int i = 1; i <= 5; i++) {
this.map.put(i, String.valueOf(i));
}
@@ -231,8 +239,8 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldPergeOnPut() throws Exception {
this.map = new TestWeakConcurrentCache<Integer, String>(1, 0.75f, 1);
public void shouldPergeOnPut() {
this.map = new TestWeakConcurrentCache<>(1, 0.75f, 1);
for (int i = 1; i <= 5; i++) {
this.map.put(i, String.valueOf(i));
}
@@ -247,28 +255,28 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldPutIfAbsent() throws Exception {
public void shouldPutIfAbsent() {
assertThat(this.map.putIfAbsent(123, "123"), is(nullValue()));
assertThat(this.map.putIfAbsent(123, "123b"), is("123"));
assertThat(this.map.get(123), is("123"));
}
@Test
public void shouldPutIfAbsentWithNullValue() throws Exception {
public void shouldPutIfAbsentWithNullValue() {
assertThat(this.map.putIfAbsent(123, null), is(nullValue()));
assertThat(this.map.putIfAbsent(123, "123"), is(nullValue()));
assertThat(this.map.get(123), is(nullValue()));
}
@Test
public void shouldPutIfAbsentWithNullKey() throws Exception {
public void shouldPutIfAbsentWithNullKey() {
assertThat(this.map.putIfAbsent(null, "123"), is(nullValue()));
assertThat(this.map.putIfAbsent(null, "123b"), is("123"));
assertThat(this.map.get(null), is("123"));
}
@Test
public void shouldRemoveKeyAndValue() throws Exception {
public void shouldRemoveKeyAndValue() {
this.map.put(123, "123");
assertThat(this.map.remove(123, "456"), is(false));
assertThat(this.map.get(123), is("123"));
@@ -278,7 +286,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldRemoveKeyAndValueWithExistingNull() throws Exception {
public void shouldRemoveKeyAndValueWithExistingNull() {
this.map.put(123, null);
assertThat(this.map.remove(123, "456"), is(false));
assertThat(this.map.get(123), is(nullValue()));
@@ -288,7 +296,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldReplaceOldValueWithNewValue() throws Exception {
public void shouldReplaceOldValueWithNewValue() {
this.map.put(123, "123");
assertThat(this.map.replace(123, "456", "789"), is(false));
assertThat(this.map.get(123), is("123"));
@@ -297,7 +305,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldReplaceOldNullValueWithNewValue() throws Exception {
public void shouldReplaceOldNullValueWithNewValue() {
this.map.put(123, null);
assertThat(this.map.replace(123, "456", "789"), is(false));
assertThat(this.map.get(123), is(nullValue()));
@@ -306,21 +314,21 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldReplaceValue() throws Exception {
public void shouldReplaceValue() {
this.map.put(123, "123");
assertThat(this.map.replace(123, "456"), is("123"));
assertThat(this.map.get(123), is("456"));
}
@Test
public void shouldReplaceNullValue() throws Exception {
public void shouldReplaceNullValue() {
this.map.put(123, null);
assertThat(this.map.replace(123, "456"), is(nullValue()));
assertThat(this.map.get(123), is("456"));
}
@Test
public void shouldGetSize() throws Exception {
public void shouldGetSize() {
assertThat(this.map.size(), is(0));
this.map.put(123, "123");
this.map.put(123, null);
@@ -329,7 +337,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldSupportIsEmpty() throws Exception {
public void shouldSupportIsEmpty() {
assertThat(this.map.isEmpty(), is(true));
this.map.put(123, "123");
this.map.put(123, null);
@@ -338,7 +346,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldContainKey() throws Exception {
public void shouldContainKey() {
assertThat(this.map.containsKey(123), is(false));
assertThat(this.map.containsKey(456), is(false));
this.map.put(123, "123");
@@ -348,7 +356,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldContainValue() throws Exception {
public void shouldContainValue() {
assertThat(this.map.containsValue("123"), is(false));
assertThat(this.map.containsValue(null), is(false));
this.map.put(123, "123");
@@ -358,7 +366,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldRemoveWhenKeyIsInMap() throws Exception {
public void shouldRemoveWhenKeyIsInMap() {
this.map.put(123, null);
this.map.put(456, "456");
this.map.put(null, "789");
@@ -369,15 +377,15 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldRemoveWhenKeyIsNotInMap() throws Exception {
public void shouldRemoveWhenKeyIsNotInMap() {
assertThat(this.map.remove(123), is(nullValue()));
assertThat(this.map.remove(null), is(nullValue()));
assertThat(this.map.isEmpty(), is(true));
}
@Test
public void shouldPutAll() throws Exception {
Map<Integer, String> m = new HashMap<Integer, String>();
public void shouldPutAll() {
Map<Integer, String> m = new HashMap<>();
m.put(123, "123");
m.put(456, null);
m.put(null, "789");
@@ -389,7 +397,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldClear() throws Exception {
public void shouldClear() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
@@ -401,11 +409,11 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldGetKeySet() throws Exception {
public void shouldGetKeySet() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
Set<Integer> expected = new HashSet<Integer>();
Set<Integer> expected = new HashSet<>();
expected.add(123);
expected.add(456);
expected.add(null);
@@ -413,26 +421,26 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldGetValues() throws Exception {
public void shouldGetValues() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
List<String> actual = new ArrayList<String>(this.map.values());
List<String> expected = new ArrayList<String>();
List<String> actual = new ArrayList<>(this.map.values());
List<String> expected = new ArrayList<>();
expected.add("123");
expected.add(null);
expected.add("789");
Collections.sort(actual, NULL_SAFE_STRING_SORT);
Collections.sort(expected, NULL_SAFE_STRING_SORT);
actual.sort(NULL_SAFE_STRING_SORT);
expected.sort(NULL_SAFE_STRING_SORT);
assertThat(actual, is(expected));
}
@Test
public void shouldGetEntrySet() throws Exception {
public void shouldGetEntrySet() {
this.map.put(123, "123");
this.map.put(456, null);
this.map.put(null, "789");
HashMap<Integer, String> expected = new HashMap<Integer, String>();
HashMap<Integer, String> expected = new HashMap<>();
expected.put(123, "123");
expected.put(456, null);
expected.put(null, "789");
@@ -440,13 +448,13 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldGetEntrySetFollowingNext() throws Exception {
public void shouldGetEntrySetFollowingNext() {
// Use loadFactor to disable resize
this.map = new TestWeakConcurrentCache<Integer, String>(1, 10.0f, 1);
this.map = new TestWeakConcurrentCache<>(1, 10.0f, 1);
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
HashMap<Integer, String> expected = new HashMap<Integer, String>();
HashMap<Integer, String> expected = new HashMap<>();
expected.put(1, "1");
expected.put(2, "2");
expected.put(3, "3");
@@ -454,7 +462,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldRemoveViaEntrySet() throws Exception {
public void shouldRemoveViaEntrySet() {
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
@@ -469,7 +477,7 @@ public class ConcurrentReferenceHashMapTests {
}
@Test
public void shouldSetViaEntrySet() throws Exception {
public void shouldSetViaEntrySet() {
this.map.put(1, "1");
this.map.put(2, "2");
this.map.put(3, "3");
@@ -484,36 +492,21 @@ public class ConcurrentReferenceHashMapTests {
@Test
@Ignore("Intended for use during development only")
public void shouldBeFasterThanSynchronizedMap() throws Exception {
public void shouldBeFasterThanSynchronizedMap() throws InterruptedException {
Map<Integer, WeakReference<String>> synchronizedMap = Collections.synchronizedMap(new WeakHashMap<Integer, WeakReference<String>>());
StopWatch mapTime = timeMultiThreaded("SynchronizedMap", synchronizedMap,
new ValueFactory<WeakReference<String>>() {
@Override
public WeakReference<String> newValue(int v) {
return new WeakReference<String>(String.valueOf(v));
}
});
StopWatch mapTime = timeMultiThreaded("SynchronizedMap", synchronizedMap, v -> new WeakReference<>(String.valueOf(v)));
System.out.println(mapTime.prettyPrint());
this.map.setDisableTestHooks(true);
StopWatch cacheTime = timeMultiThreaded("WeakConcurrentCache", this.map,
new ValueFactory<String>() {
@Override
public String newValue(int v) {
return String.valueOf(v);
}
});
StopWatch cacheTime = timeMultiThreaded("WeakConcurrentCache", this.map, String::valueOf);
System.out.println(cacheTime.prettyPrint());
// We should be at least 4 time faster
assertThat(cacheTime.getTotalTimeSeconds(),
is(lessThan(mapTime.getTotalTimeSeconds() / 4.0)));
assertThat(cacheTime.getTotalTimeSeconds(), is(lessThan(mapTime.getTotalTimeSeconds() / 4.0)));
}
@Test
public void shouldSupportNullReference() throws Exception {
public void shouldSupportNullReference() {
// GC could happen during restructure so we must be able to create a reference for a null entry
map.createReferenceManager().createReference(null, 1234, null);
}
@@ -557,7 +550,7 @@ public class ConcurrentReferenceHashMapTests {
}
private static interface ValueFactory<V> {
private interface ValueFactory<V> {
V newValue(int k);
}
@@ -567,7 +560,7 @@ public class ConcurrentReferenceHashMapTests {
private int supplimentalHash;
private final LinkedList<MockReference<K, V>> queue = new LinkedList<MockReference<K, V>>();
private final LinkedList<MockReference<K, V>> queue = new LinkedList<>();
private boolean disableTestHooks;
@@ -605,12 +598,11 @@ public class ConcurrentReferenceHashMapTests {
protected ReferenceManager createReferenceManager() {
return new ReferenceManager() {
@Override
public Reference<K, V> createReference(Entry<K, V> entry, int hash,
Reference<K, V> next) {
public Reference<K, V> createReference(Entry<K, V> entry, int hash, Reference<K, V> next) {
if (TestWeakConcurrentCache.this.disableTestHooks) {
return super.createReference(entry, hash, next);
}
return new MockReference<K, V>(entry, hash, next, TestWeakConcurrentCache.this.queue);
return new MockReference<>(entry, hash, next, TestWeakConcurrentCache.this.queue);
}
@Override
public Reference<K, V> pollForPurge() {