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 17127f2eeb..cee33acdf9 100644 --- a/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java +++ b/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java @@ -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 extends AbstractMap implemen @Override public V get(Object key) { - Reference reference = getReference(key, Restructure.WHEN_NECESSARY); - Entry entry = (reference != null ? reference.get() : null); + Entry entry = getEntryIfAvailable(key); return (entry != null ? entry.getValue() : null); } + @Override + public V getOrDefault(Object key, V defaultValue) { + Entry entry = getEntryIfAvailable(key); + return (entry != null ? entry.getValue() : defaultValue); + } + @Override public boolean containsKey(Object key) { - Reference reference = getReference(key, Restructure.WHEN_NECESSARY); - Entry entry = (reference != null ? reference.get() : null); + Entry entry = getEntryIfAvailable(key); return (entry != null && ObjectUtils.nullSafeEquals(entry.getKey(), key)); } + private Entry getEntryIfAvailable(Object key) { + Reference 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 extends AbstractMap implemen } private Reference findInChain(Reference reference, Object key, int hash) { - while (reference != null) { - if (reference.getHash() == hash) { - Entry entry = reference.get(); + Reference currRef = reference; + while (currRef != null) { + if (currRef.getHash() == hash) { + Entry 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; } diff --git a/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java b/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java index d585d0aef1..5930a283bd 100644 --- a/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java +++ b/spring-core/src/test/java/org/springframework/util/ConcurrentReferenceHashMapTests.java @@ -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 NULL_SAFE_STRING_SORT = new NullSafeComparator( - new ComparableComparator(), true); + private static final Comparator NULL_SAFE_STRING_SORT = + new NullSafeComparator(new ComparableComparator(), true); @Rule public ExpectedException thrown = ExpectedException.none(); - private TestWeakConcurrentCache map = new TestWeakConcurrentCache(); + private TestWeakConcurrentCache map = new TestWeakConcurrentCache<>(); @Test - public void shouldCreateWithDefaults() throws Exception { - ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap(); + public void shouldCreateWithDefaults() { + ConcurrentReferenceHashMap 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 map = new ConcurrentReferenceHashMap(32); + public void shouldCreateWithInitialCapacity() { + ConcurrentReferenceHashMap 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 map = new ConcurrentReferenceHashMap(32, 0.5f); + public void shouldCreateWithInitialCapacityAndLoadFactor() { + ConcurrentReferenceHashMap 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 map = new ConcurrentReferenceHashMap(16, 2); + public void shouldCreateWithInitialCapacityAndConcurrenyLevel() { + ConcurrentReferenceHashMap 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 map = new ConcurrentReferenceHashMap(5, 0.5f, 3); + public void shouldCreateFullyCustom() { + ConcurrentReferenceHashMap 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(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(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(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(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(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(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(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 m = new HashMap(); + public void shouldPutAll() { + Map 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 expected = new HashSet(); + Set 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 actual = new ArrayList(this.map.values()); - List expected = new ArrayList(); + List actual = new ArrayList<>(this.map.values()); + List 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 expected = new HashMap(); + HashMap 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(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 expected = new HashMap(); + HashMap 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> synchronizedMap = Collections.synchronizedMap(new WeakHashMap>()); - StopWatch mapTime = timeMultiThreaded("SynchronizedMap", synchronizedMap, - new ValueFactory>() { - - @Override - public WeakReference newValue(int v) { - return new WeakReference(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() { - - @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 { + private interface ValueFactory { V newValue(int k); } @@ -567,7 +560,7 @@ public class ConcurrentReferenceHashMapTests { private int supplimentalHash; - private final LinkedList> queue = new LinkedList>(); + private final LinkedList> queue = new LinkedList<>(); private boolean disableTestHooks; @@ -605,12 +598,11 @@ public class ConcurrentReferenceHashMapTests { protected ReferenceManager createReferenceManager() { return new ReferenceManager() { @Override - public Reference createReference(Entry entry, int hash, - Reference next) { + public Reference createReference(Entry entry, int hash, Reference next) { if (TestWeakConcurrentCache.this.disableTestHooks) { return super.createReference(entry, hash, next); } - return new MockReference(entry, hash, next, TestWeakConcurrentCache.this.queue); + return new MockReference<>(entry, hash, next, TestWeakConcurrentCache.this.queue); } @Override public Reference pollForPurge() {