From 1aff833282289b6d02c97f95a6bc4c68fa21b546 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 21 Jan 2014 00:35:38 +0100 Subject: [PATCH] Polishing --- .../util/ConcurrentReferenceHashMap.java | 16 ++-- .../util/ConcurrentReferenceHashMapTests.java | 76 +++++++++---------- 2 files changed, 43 insertions(+), 49 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 05eb94dc9a..4c8a1cf4e7 100644 --- a/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java +++ b/spring-core/src/main/java/org/springframework/util/ConcurrentReferenceHashMap.java @@ -60,10 +60,10 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen private static final int DEFAULT_INITIAL_CAPACITY = 16; - private static final int DEFAULT_CONCURRENCY_LEVEL = 16; - private static final float DEFAULT_LOAD_FACTOR = 0.75f; + private static final int DEFAULT_CONCURRENCY_LEVEL = 16; + private static final ReferenceType DEFAULT_REFERENCE_TYPE = ReferenceType.SOFT; private static final int MAXIMUM_CONCURRENCY_LEVEL = 1 << 16; @@ -116,7 +116,7 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen * Create a new {@code ConcurrentReferenceHashMap} instance. * @param initialCapacity the initial capacity of the map * @param loadFactor the load factor. When the average number of references per table - * exceeds this value resize will be attempted + * exceeds this value resize will be attempted */ public ConcurrentReferenceHashMap(int initialCapacity, float loadFactor) { this(initialCapacity, loadFactor, DEFAULT_CONCURRENCY_LEVEL, DEFAULT_REFERENCE_TYPE); @@ -157,9 +157,9 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen public ConcurrentReferenceHashMap(int initialCapacity, float loadFactor, int concurrencyLevel, ReferenceType referenceType) { - Assert.isTrue(concurrencyLevel > 0, "ConcurrencyLevel must be positive"); - Assert.isTrue(initialCapacity >= 0, "InitialCapacity must not be negative"); - Assert.isTrue(loadFactor > 0f, "LoadFactor must be positive"); + Assert.isTrue(initialCapacity >= 0, "Initial capacity must not be negative"); + Assert.isTrue(loadFactor > 0f, "Load factor must be positive"); + Assert.isTrue(concurrencyLevel > 0, "Concurrency level must be positive"); Assert.notNull(referenceType, "Reference type must not be null"); this.loadFactor = loadFactor; this.shift = calculateShift(concurrencyLevel, MAXIMUM_CONCURRENCY_LEVEL); @@ -639,7 +639,7 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen /** * Release this entry and ensure that it will be returned from - * {@link ReferenceManager#pollForPurge()}. + * {@code ReferenceManager#pollForPurge()}. */ void release(); } @@ -744,7 +744,7 @@ public class ConcurrentReferenceHashMap extends AbstractMap implemen /** - * Various options supported by a {@link Task}. + * Various options supported by a {@code Task}. */ private static enum TaskOption { 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 f9a6d1070e..d585d0aef1 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-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -16,9 +16,6 @@ package org.springframework.util; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; - import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collections; @@ -36,14 +33,19 @@ import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; + import org.springframework.util.ConcurrentReferenceHashMap.Entry; import org.springframework.util.ConcurrentReferenceHashMap.Reference; import org.springframework.util.ConcurrentReferenceHashMap.Restructure; import org.springframework.util.comparator.ComparableComparator; import org.springframework.util.comparator.NullSafeComparator; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + /** * Tests for {@link ConcurrentReferenceHashMap}. + * * @author Phillip Webb */ public class ConcurrentReferenceHashMapTests { @@ -56,6 +58,7 @@ public class ConcurrentReferenceHashMapTests { private TestWeakConcurrentCache map = new TestWeakConcurrentCache(); + @Test public void shouldCreateWithDefaults() throws Exception { ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap(); @@ -66,8 +69,7 @@ public class ConcurrentReferenceHashMapTests { @Test public void shouldCreateWithInitialCapacity() throws Exception { - ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap( - 32); + ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap(32); assertThat(map.getSegmentsSize(), is(16)); assertThat(map.getSegment(0).getSize(), is(2)); assertThat(map.getLoadFactor(), is(0.75f)); @@ -75,8 +77,7 @@ public class ConcurrentReferenceHashMapTests { @Test public void shouldCreateWithInitialCapacityAndLoadFactor() throws Exception { - ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap( - 32, 0.5f); + 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)); @@ -84,8 +85,7 @@ public class ConcurrentReferenceHashMapTests { @Test public void shouldCreateWithInitialCapacityAndConcurrenyLevel() throws Exception { - ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap( - 16, 2); + ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap(16, 2); assertThat(map.getSegmentsSize(), is(2)); assertThat(map.getSegment(0).getSize(), is(8)); assertThat(map.getLoadFactor(), is(0.75f)); @@ -93,8 +93,7 @@ public class ConcurrentReferenceHashMapTests { @Test public void shouldCreateFullyCustom() throws Exception { - ConcurrentReferenceHashMap map = new ConcurrentReferenceHashMap( - 5, 0.5f, 3); + 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,19 +101,11 @@ public class ConcurrentReferenceHashMapTests { assertThat(map.getLoadFactor(), is(0.5f)); } - @Test - public void shouldNeedPositiveConcurrenyLevel() throws Exception { - new ConcurrentReferenceHashMap(1, 1); - this.thrown.expect(IllegalArgumentException.class); - this.thrown.expectMessage("ConcurrencyLevel must be positive"); - new TestWeakConcurrentCache(1, 0); - } - @Test public void shouldNeedNonNegativeInitialCapacity() throws Exception { new ConcurrentReferenceHashMap(0, 1); this.thrown.expect(IllegalArgumentException.class); - this.thrown.expectMessage("InitialCapacity must not be negative"); + this.thrown.expectMessage("Initial capacity must not be negative"); new TestWeakConcurrentCache(-1, 1); } @@ -122,10 +113,18 @@ public class ConcurrentReferenceHashMapTests { public void shouldNeedPositiveLoadFactor() throws Exception { new ConcurrentReferenceHashMap(0, 0.1f, 1); this.thrown.expect(IllegalArgumentException.class); - this.thrown.expectMessage("LoadFactor must be positive"); + this.thrown.expectMessage("Load factor must be positive"); new TestWeakConcurrentCache(0, 0.0f, 1); } + @Test + public void shouldNeedPositiveConcurrencyLevel() throws Exception { + new ConcurrentReferenceHashMap(1, 1); + this.thrown.expect(IllegalArgumentException.class); + this.thrown.expectMessage("Concurrency level must be positive"); + new TestWeakConcurrentCache(1, 0); + } + @Test public void shouldPutAndGet() throws Exception { // NOTE we are using mock references so we don't need to worry about GC @@ -521,13 +520,11 @@ public class ConcurrentReferenceHashMapTests { /** * Time a multi-threaded access to a cache. - * - * @param cache the cache to test * @return the timing stopwatch - * @throws InterruptedException */ private StopWatch timeMultiThreaded(String id, final Map map, ValueFactory factory) throws InterruptedException { + StopWatch stopWatch = new StopWatch(id); for (int i = 0; i < 500; i++) { map.put(i, factory.newValue(i)); @@ -536,7 +533,6 @@ public class ConcurrentReferenceHashMapTests { stopWatch.start("Running threads"); for (int threadIndex = 0; threadIndex < threads.length; threadIndex++) { threads[threadIndex] = new Thread("Cache access thread " + threadIndex) { - @Override public void run() { for (int j = 0; j < 1000; j++) { @@ -544,29 +540,30 @@ public class ConcurrentReferenceHashMapTests { map.get(i); } } - }; + } }; } - for (int i = 0; i < threads.length; i++) { - threads[i].start(); + for (Thread thread : threads) { + thread.start(); } - for (int i = 0; i < threads.length; i++) { - if (threads[i].isAlive()) { - threads[i].join(2000); + for (Thread thread : threads) { + if (thread.isAlive()) { + thread.join(2000); } } stopWatch.stop(); return stopWatch; } + private static interface ValueFactory { V newValue(int k); } - private static class TestWeakConcurrentCache extends - ConcurrentReferenceHashMap { + + private static class TestWeakConcurrentCache extends ConcurrentReferenceHashMap { private int supplimentalHash; @@ -582,8 +579,7 @@ public class ConcurrentReferenceHashMapTests { this.disableTestHooks = disableTestHooks; } - public TestWeakConcurrentCache(int initialCapacity, float loadFactor, - int concurrencyLevel) { + public TestWeakConcurrentCache(int initialCapacity, float loadFactor, int concurrencyLevel) { super(initialCapacity, loadFactor, concurrencyLevel); } @@ -607,9 +603,7 @@ public class ConcurrentReferenceHashMapTests { @Override protected ReferenceManager createReferenceManager() { - return new ReferenceManager() { - @Override public Reference createReference(Entry entry, int hash, Reference next) { @@ -618,7 +612,6 @@ public class ConcurrentReferenceHashMapTests { } return new MockReference(entry, hash, next, TestWeakConcurrentCache.this.queue); } - @Override public Reference pollForPurge() { if (TestWeakConcurrentCache.this.disableTestHooks) { @@ -634,6 +627,7 @@ public class ConcurrentReferenceHashMapTests { } } + private static class MockReference implements Reference { private final int hash; @@ -644,8 +638,7 @@ public class ConcurrentReferenceHashMapTests { private final LinkedList> queue; - public MockReference(Entry entry, int hash, Reference next, - LinkedList> queue) { + public MockReference(Entry entry, int hash, Reference next, LinkedList> queue) { this.hash = hash; this.entry = entry; this.next = next; @@ -677,4 +670,5 @@ public class ConcurrentReferenceHashMapTests { this.queue.add(this); } } + }