You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by us...@apache.org on 2011/12/18 18:11:06 UTC
svn commit: r1220458 - in /lucene/dev/trunk: lucene/
lucene/src/java/org/apache/lucene/util/
lucene/src/test/org/apache/lucene/util/ solr/
Author: uschindler
Date: Sun Dec 18 17:11:06 2011
New Revision: 1220458
URL: http://svn.apache.org/viewvc?rev=1220458&view=rev
Log:
LUCENE-3653: Improve the sophisticated™ backwards layers (VirtualMethod) and instantiation cost in AttributeSource/AttributeFactory (new reflection cache using ConcurrentHashMap has lockless get)
Added:
lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityMap.java
- copied, changed from r1214550, lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityHashMap.java
lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityMap.java
- copied, changed from r1214550, lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityHashMap.java
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/NOTICE.txt
lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/AttributeSource.java
lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/VirtualMethod.java
lucene/dev/trunk/solr/NOTICE.txt
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1220458&r1=1220457&r2=1220458&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Sun Dec 18 17:11:06 2011
@@ -766,6 +766,12 @@ Bug fixes
double precision and to compute age to be how long ago the searcher
was replaced with a new searcher (Mike McCandless)
+Optimizations
+
+* LUCENE-3653: Improve concurrency in VirtualMethod and AttributeSource by
+ using a WeakIdentityMap based on a ConcurrentHashMap. (Uwe Schindler,
+ Gerrit Jansen van Vuuren)
+
Documentation
* LUCENE-3597: Fixed incorrect grouping documentation. (Martijn van Groningen, Robert Muir)
Modified: lucene/dev/trunk/lucene/NOTICE.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/NOTICE.txt?rev=1220458&r1=1220457&r2=1220458&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/NOTICE.txt (original)
+++ lucene/dev/trunk/lucene/NOTICE.txt Sun Dec 18 17:11:06 2011
@@ -30,6 +30,9 @@ with the same name. The implementation p
Lucene sorting code. In-place stable mergesort was borrowed from CGLIB,
which is Apache-licensed.
+The class org.apache.lucene.util.WeakIdentityMap was derived from
+the Apache CXF project and is Apache License 2.0.
+
The Google Code Prettify is Apache License 2.0.
See http://code.google.com/p/google-code-prettify/
Modified: lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/AttributeSource.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/AttributeSource.java?rev=1220458&r1=1220457&r2=1220458&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/AttributeSource.java (original)
+++ lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/AttributeSource.java Sun Dec 18 17:11:06 2011
@@ -22,7 +22,6 @@ import java.util.Collections;
import java.util.NoSuchElementException;
import java.util.Iterator;
import java.util.LinkedHashMap;
-import java.util.WeakHashMap;
import java.util.LinkedList;
import java.util.Map;
import java.util.Map.Entry;
@@ -55,8 +54,8 @@ public class AttributeSource {
public static final AttributeFactory DEFAULT_ATTRIBUTE_FACTORY = new DefaultAttributeFactory();
private static final class DefaultAttributeFactory extends AttributeFactory {
- private static final WeakHashMap<Class<? extends Attribute>, WeakReference<Class<? extends AttributeImpl>>> attClassImplMap =
- new WeakHashMap<Class<? extends Attribute>, WeakReference<Class<? extends AttributeImpl>>>();
+ private static final WeakIdentityMap<Class<? extends Attribute>, WeakReference<Class<? extends AttributeImpl>>> attClassImplMap =
+ WeakIdentityMap.newConcurrentHashMap();
private DefaultAttributeFactory() {}
@@ -72,23 +71,22 @@ public class AttributeSource {
}
private static Class<? extends AttributeImpl> getClassForInterface(Class<? extends Attribute> attClass) {
- synchronized(attClassImplMap) {
- final WeakReference<Class<? extends AttributeImpl>> ref = attClassImplMap.get(attClass);
- Class<? extends AttributeImpl> clazz = (ref == null) ? null : ref.get();
- if (clazz == null) {
- try {
- attClassImplMap.put(attClass,
- new WeakReference<Class<? extends AttributeImpl>>(
- clazz = Class.forName(attClass.getName() + "Impl", true, attClass.getClassLoader())
- .asSubclass(AttributeImpl.class)
- )
- );
- } catch (ClassNotFoundException e) {
- throw new IllegalArgumentException("Could not find implementing class for " + attClass.getName());
- }
+ final WeakReference<Class<? extends AttributeImpl>> ref = attClassImplMap.get(attClass);
+ Class<? extends AttributeImpl> clazz = (ref == null) ? null : ref.get();
+ if (clazz == null) {
+ // we have the slight chance that another thread may do the same, but who cares?
+ try {
+ attClassImplMap.put(attClass,
+ new WeakReference<Class<? extends AttributeImpl>>(
+ clazz = Class.forName(attClass.getName() + "Impl", true, attClass.getClassLoader())
+ .asSubclass(AttributeImpl.class)
+ )
+ );
+ } catch (ClassNotFoundException e) {
+ throw new IllegalArgumentException("Could not find implementing class for " + attClass.getName());
}
- return clazz;
}
+ return clazz;
}
}
}
@@ -199,30 +197,28 @@ public class AttributeSource {
}
/** a cache that stores all interfaces for known implementation classes for performance (slow reflection) */
- private static final WeakHashMap<Class<? extends AttributeImpl>,LinkedList<WeakReference<Class<? extends Attribute>>>> knownImplClasses =
- new WeakHashMap<Class<? extends AttributeImpl>,LinkedList<WeakReference<Class<? extends Attribute>>>>();
+ private static final WeakIdentityMap<Class<? extends AttributeImpl>,LinkedList<WeakReference<Class<? extends Attribute>>>> knownImplClasses =
+ WeakIdentityMap.newConcurrentHashMap();
static LinkedList<WeakReference<Class<? extends Attribute>>> getAttributeInterfaces(final Class<? extends AttributeImpl> clazz) {
- synchronized(knownImplClasses) {
- LinkedList<WeakReference<Class<? extends Attribute>>> foundInterfaces = knownImplClasses.get(clazz);
- if (foundInterfaces == null) {
- // we have a strong reference to the class instance holding all interfaces in the list (parameter "att"),
- // so all WeakReferences are never evicted by GC
- knownImplClasses.put(clazz, foundInterfaces = new LinkedList<WeakReference<Class<? extends Attribute>>>());
- // find all interfaces that this attribute instance implements
- // and that extend the Attribute interface
- Class<?> actClazz = clazz;
- do {
- for (Class<?> curInterface : actClazz.getInterfaces()) {
- if (curInterface != Attribute.class && Attribute.class.isAssignableFrom(curInterface)) {
- foundInterfaces.add(new WeakReference<Class<? extends Attribute>>(curInterface.asSubclass(Attribute.class)));
- }
+ LinkedList<WeakReference<Class<? extends Attribute>>> foundInterfaces = knownImplClasses.get(clazz);
+ if (foundInterfaces == null) {
+ // we have the slight chance that another thread may do the same, but who cares?
+ foundInterfaces = new LinkedList<WeakReference<Class<? extends Attribute>>>();
+ // find all interfaces that this attribute instance implements
+ // and that extend the Attribute interface
+ Class<?> actClazz = clazz;
+ do {
+ for (Class<?> curInterface : actClazz.getInterfaces()) {
+ if (curInterface != Attribute.class && Attribute.class.isAssignableFrom(curInterface)) {
+ foundInterfaces.add(new WeakReference<Class<? extends Attribute>>(curInterface.asSubclass(Attribute.class)));
}
- actClazz = actClazz.getSuperclass();
- } while (actClazz != null);
- }
- return foundInterfaces;
+ }
+ actClazz = actClazz.getSuperclass();
+ } while (actClazz != null);
+ knownImplClasses.put(clazz, foundInterfaces);
}
+ return foundInterfaces;
}
/** <b>Expert:</b> Adds a custom AttributeImpl instance with one or more Attribute interfaces.
Modified: lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/VirtualMethod.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/VirtualMethod.java?rev=1220458&r1=1220457&r2=1220458&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/VirtualMethod.java (original)
+++ lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/VirtualMethod.java Sun Dec 18 17:11:06 2011
@@ -20,7 +20,6 @@ package org.apache.lucene.util;
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.HashSet;
-import java.util.WeakHashMap;
import java.util.Set;
/**
@@ -64,8 +63,7 @@ public final class VirtualMethod<C> {
private final Class<C> baseClass;
private final String method;
private final Class<?>[] parameters;
- private final WeakHashMap<Class<? extends C>, Integer> cache =
- new WeakHashMap<Class<? extends C>, Integer>();
+ private final WeakIdentityMap<Class<? extends C>, Integer> cache = WeakIdentityMap.newConcurrentHashMap();
/**
* Creates a new instance for the given {@code baseClass} and method declaration.
@@ -93,9 +91,10 @@ public final class VirtualMethod<C> {
* in the inheritance path between {@code baseClass} and the given subclass {@code subclazz}.
* @return 0 iff not overridden, else the distance to the base class
*/
- public synchronized int getImplementationDistance(final Class<? extends C> subclazz) {
+ public int getImplementationDistance(final Class<? extends C> subclazz) {
Integer distance = cache.get(subclazz);
if (distance == null) {
+ // we have the slight chance that another thread may do the same, but who cares?
cache.put(subclazz, distance = Integer.valueOf(reflectImplementationDistance(subclazz)));
}
return distance.intValue();
Copied: lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityMap.java (from r1214550, lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityHashMap.java)
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityMap.java?p2=lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityMap.java&p1=lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityHashMap.java&r1=1214550&r2=1220458&rev=1220458&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityHashMap.java (original)
+++ lucene/dev/trunk/lucene/src/java/org/apache/lucene/util/WeakIdentityMap.java Sun Dec 18 17:11:06 2011
@@ -21,6 +21,8 @@ import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
/**
* Implements a combination of {@link java.util.WeakHashMap} and
@@ -34,8 +36,6 @@ import java.util.HashMap;
* when comparing objects. This class is designed for use only in the
* rare cases wherein reference-equality semantics are required.
*
- * <p><b>Note that this implementation is not synchronized.</b>
- *
* <p>This implementation was forked from <a href="http://cxf.apache.org/">Apache CXF</a>
* but modified to <b>not</b> implement the {@link java.util.Map} interface and
* without any set/iterator views on it, as those are error-prone
@@ -44,20 +44,22 @@ import java.util.HashMap;
*
* @lucene.internal
*/
-public class WeakIdentityHashMap<K,V> {
- final ReferenceQueue<Object> queue = new ReferenceQueue<Object>(); // pkg-private for inner class
- private final HashMap<IdentityWeakReference, V> backingStore;
+public final class WeakIdentityMap<K,V> {
+ private final ReferenceQueue<Object> queue = new ReferenceQueue<Object>();
+ private final Map<IdentityWeakReference, V> backingStore;
- public WeakIdentityHashMap() {
- backingStore = new HashMap<IdentityWeakReference, V>();
+ /** Creates a new {@code WeakIdentityMap} based on a non-synchronized {@link HashMap}. */
+ public static final <K,V> WeakIdentityMap<K,V> newHashMap() {
+ return new WeakIdentityMap<K,V>(new HashMap<IdentityWeakReference,V>());
}
- public WeakIdentityHashMap(int initialCapacity) {
- backingStore = new HashMap<IdentityWeakReference,V>(initialCapacity);
+ /** Creates a new {@code WeakIdentityMap} based on a {@link ConcurrentHashMap}. */
+ public static final <K,V> WeakIdentityMap<K,V> newConcurrentHashMap() {
+ return new WeakIdentityMap<K,V>(new ConcurrentHashMap<IdentityWeakReference,V>());
}
- public WeakIdentityHashMap(int initialCapacity, float loadFactor) {
- backingStore = new HashMap<IdentityWeakReference,V>(initialCapacity, loadFactor);
+ private WeakIdentityMap(Map<IdentityWeakReference, V> backingStore) {
+ this.backingStore = backingStore;
}
public void clear() {
@@ -67,22 +69,17 @@ public class WeakIdentityHashMap<K,V> {
public boolean containsKey(Object key) {
reap();
- return backingStore.containsKey(new IdentityWeakReference(key));
- }
-
- public boolean containsValue(Object value) {
- reap();
- return backingStore.containsValue(value);
+ return backingStore.containsKey(new IdentityWeakReference(key, queue));
}
public V get(Object key) {
reap();
- return backingStore.get(new IdentityWeakReference(key));
+ return backingStore.get(new IdentityWeakReference(key, queue));
}
public V put(K key, V value) {
reap();
- return backingStore.put(new IdentityWeakReference(key), value);
+ return backingStore.put(new IdentityWeakReference(key, queue), value);
}
public boolean isEmpty() {
@@ -90,12 +87,8 @@ public class WeakIdentityHashMap<K,V> {
}
public V remove(Object key) {
- try {
- reap();
- return backingStore.remove(new IdentityWeakReference(key));
- } finally {
- reap();
- }
+ reap();
+ return backingStore.remove(new IdentityWeakReference(key, queue));
}
public int size() {
@@ -112,10 +105,10 @@ public class WeakIdentityHashMap<K,V> {
}
}
- final class IdentityWeakReference extends WeakReference<Object> {
+ private static final class IdentityWeakReference extends WeakReference<Object> {
private final int hash;
- IdentityWeakReference(Object obj) {
+ IdentityWeakReference(Object obj, ReferenceQueue<Object> queue) {
super(obj == null ? NULL : obj, queue);
hash = System.identityHashCode(obj);
}
@@ -128,17 +121,17 @@ public class WeakIdentityHashMap<K,V> {
if (this == o) {
return true;
}
- if (o instanceof WeakReference) {
- final WeakReference ref = (WeakReference)o;
+ if (o instanceof IdentityWeakReference) {
+ final IdentityWeakReference ref = (IdentityWeakReference)o;
if (this.get() == ref.get()) {
return true;
}
}
return false;
}
- }
- // we keep a hard reference to our NULL key, so this map supports null keys that never get GCed:
- static final Object NULL = new Object();
+ // we keep a hard reference to our NULL key, so map supports null keys that never get GCed:
+ private static final Object NULL = new Object();
+ }
}
Copied: lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityMap.java (from r1214550, lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityHashMap.java)
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityMap.java?p2=lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityMap.java&p1=lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityHashMap.java&r1=1214550&r2=1220458&rev=1220458&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityHashMap.java (original)
+++ lucene/dev/trunk/lucene/src/test/org/apache/lucene/util/TestWeakIdentityMap.java Sun Dec 18 17:11:06 2011
@@ -18,15 +18,20 @@
package org.apache.lucene.util;
import java.util.Map;
-import java.util.WeakHashMap;
-
-public class TestWeakIdentityHashMap extends LuceneTestCase {
-
- public void test() {
- final WeakIdentityHashMap<String,String> map =
- new WeakIdentityHashMap<String,String>();
+import java.util.Random;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReferenceArray;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+
+public class TestWeakIdentityMap extends LuceneTestCase {
+
+ public void testSimpleHashMap() {
+ final WeakIdentityMap<String,String> map =
+ WeakIdentityMap.newHashMap();
// we keep strong references to the keys,
- // so WeakIdentityHashMap will not forget about them:
+ // so WeakIdentityMap will not forget about them:
String key1 = new String("foo");
String key2 = new String("foo");
String key3 = new String("foo");
@@ -42,6 +47,8 @@ public class TestWeakIdentityHashMap ext
map.put(key2, "bar2");
map.put(null, "null");
+ assertEquals(3, map.size());
+
assertEquals("bar1", map.get(key1));
assertEquals("bar2", map.get(key2));
assertEquals(null, map.get(key3));
@@ -52,7 +59,23 @@ public class TestWeakIdentityHashMap ext
assertFalse(map.containsKey(key3));
assertTrue(map.containsKey(null));
+ // repeat and check that we have no double entries
+ map.put(key1, "bar1");
+ map.put(key2, "bar2");
+ map.put(null, "null");
+
assertEquals(3, map.size());
+
+ assertEquals("bar1", map.get(key1));
+ assertEquals("bar2", map.get(key2));
+ assertEquals(null, map.get(key3));
+ assertEquals("null", map.get(null));
+
+ assertTrue(map.containsKey(key1));
+ assertTrue(map.containsKey(key2));
+ assertFalse(map.containsKey(key3));
+ assertTrue(map.containsKey(null));
+
map.remove(null);
assertEquals(2, map.size());
map.remove(key1);
@@ -65,13 +88,105 @@ public class TestWeakIdentityHashMap ext
// clear strong refs
key1 = key2 = key3 = null;
+ // check that GC does not cause problems in reap() method, wait 1 second and let GC work:
+ int size = map.size();
+ for (int i = 0; size > 0 && i < 10; i++) try {
+ System.runFinalization();
+ System.gc();
+ Thread.sleep(100L);
+ assertTrue(size >= map.size());
+ size = map.size();
+ } catch (InterruptedException ie) {}
+
+ map.clear();
+ assertEquals(0, map.size());
+ assertTrue(map.isEmpty());
+
+ key1 = new String("foo");
+ key2 = new String("foo");
+ map.put(key1, "bar1");
+ map.put(key2, "bar2");
+ assertEquals(2, map.size());
+
+ map.clear();
+ assertEquals(0, map.size());
+ assertTrue(map.isEmpty());
+ }
+
+ public void testConcurrentHashMap() throws Exception {
+ final int threadCount = atLeast(32), keyCount = atLeast(1024);
+ final ExecutorService exec = Executors.newFixedThreadPool(threadCount + 1);
+ final WeakIdentityMap<Object,Integer> map =
+ WeakIdentityMap.newConcurrentHashMap();
+ // we keep strong references to the keys,
+ // so WeakIdentityMap will not forget about them:
+ final AtomicReferenceArray<Object> keys = new AtomicReferenceArray<Object>(keyCount);
+ for (int j = 0; j < keyCount; j++) {
+ keys.set(j, new Object());
+ }
+
+ try {
+ final AtomicInteger running = new AtomicInteger(threadCount);
+ for (int t = 0; t < threadCount; t++) {
+ final Random rnd = new Random(random.nextLong());
+ final int count = atLeast(rnd, 20000);
+ exec.execute(new Runnable() {
+ public void run() {
+ for (int i = 0; i < count; i++) {
+ final int j = rnd.nextInt(keyCount);
+ switch (rnd.nextInt(4)) {
+ case 0:
+ map.put(keys.get(j), Integer.valueOf(j));
+ break;
+ case 1:
+ final Integer v = map.get(keys.get(j));
+ if (v != null) {
+ assertEquals(j, v.intValue());
+ }
+ break;
+ case 2:
+ map.remove(keys.get(j));
+ break;
+ case 3:
+ // renew key, the old one will be GCed at some time:
+ keys.set(j, new Object());
+ break;
+ default:
+ fail("Should not get here.");
+ }
+ }
+ running.decrementAndGet();
+ }
+ });
+ }
+ exec.execute(new Runnable() {
+ public void run() {
+ // check that GC does not cause problems in reap() method:
+ while (running.get() > 0) {
+ System.runFinalization();
+ System.gc();
+ map.isEmpty(); // simple access
+ }
+ }
+ });
+ } finally {
+ exec.shutdown();
+ while (!exec.awaitTermination(1000L, TimeUnit.MILLISECONDS));
+ }
+
+ // clear strong refs
+ for (int j = 0; j < keyCount; j++) {
+ keys.set(j, null);
+ }
+
// check that GC does not cause problems in reap() method:
- for (int i = 0; !map.isEmpty(); i++) try {
- if (i > 40)
- fail("The garbage collector did not reclaim all keys after 2 seconds, failing test!");
+ int size = map.size();
+ for (int i = 0; size > 0 && i < 10; i++) try {
System.runFinalization();
System.gc();
- Thread.currentThread().sleep(50L);
+ Thread.sleep(100L);
+ assertTrue(size >= map.size());
+ size = map.size();
} catch (InterruptedException ie) {}
}
Modified: lucene/dev/trunk/solr/NOTICE.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/NOTICE.txt?rev=1220458&r1=1220457&r2=1220458&view=diff
==============================================================================
--- lucene/dev/trunk/solr/NOTICE.txt (original)
+++ lucene/dev/trunk/solr/NOTICE.txt Sun Dec 18 17:11:06 2011
@@ -69,6 +69,9 @@ with the same name. The implementation p
Lucene sorting code. In-place stable mergesort was borrowed from CGLIB,
which is Apache-licensed.
+The class org.apache.lucene.util.WeakIdentityMap was derived from
+the Apache CXF project and is Apache License 2.0.
+
The Google Code Prettify is Apache License 2.0.
See http://code.google.com/p/google-code-prettify/