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 2014/08/15 12:50:14 UTC
svn commit: r1618148 - in /lucene/dev/trunk/lucene: ./
core/src/java/org/apache/lucene/util/
test-framework/src/java/org/apache/lucene/analysis/
Author: uschindler
Date: Fri Aug 15 10:50:13 2014
New Revision: 1618148
URL: http://svn.apache.org/r1618148
Log:
LUCENE-5887: Remove WeakIdentityMap caching in AttributeFactory, AttributeSource, and VirtualMethod in favour of Java 7's ClassValue. Always use MethodHandles to create AttributeImpl classes.
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeFactory.java
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeImpl.java
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeSource.java
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1618148&r1=1618147&r2=1618148&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Fri Aug 15 10:50:13 2014
@@ -220,6 +220,11 @@ Optimizations
* LUCENE-5882: Add Lucene410DocValuesFormat, with faster term lookups
for SORTED/SORTED_SET fields. (Robert Muir)
+* LUCENE-5887: Remove WeakIdentityMap caching in AttributeFactory,
+ AttributeSource, and VirtualMethod in favour of Java 7's ClassValue.
+ Always use MethodHandles to create AttributeImpl classes.
+ (Uwe Schindler)
+
Bug Fixes
* LUCENE-5796: Fixes the Scorer.getChildren() method for two combinations
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeFactory.java?rev=1618148&r1=1618147&r2=1618148&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeFactory.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeFactory.java Fri Aug 15 10:50:13 2014
@@ -20,8 +20,6 @@ package org.apache.lucene.util;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
-import java.lang.ref.Reference;
-import java.lang.ref.WeakReference;
/**
* An AttributeFactory creates instances of {@link AttributeImpl}s.
@@ -52,48 +50,25 @@ public abstract class AttributeFactory {
* This is the default factory that creates {@link AttributeImpl}s using the
* class name of the supplied {@link Attribute} interface class by appending <code>Impl</code> to it.
*/
- public static final AttributeFactory DEFAULT_ATTRIBUTE_FACTORY = new DefaultAttributeFactory(true);
+ public static final AttributeFactory DEFAULT_ATTRIBUTE_FACTORY = new DefaultAttributeFactory();
- static final class DefaultAttributeFactory extends AttributeFactory {
- private final WeakIdentityMap<Class<? extends Attribute>, Object> attClassImplMap =
- WeakIdentityMap.newConcurrentHashMap(false);
- private final ClassLoader myClassLoader = getClass().getClassLoader();
- private final boolean useMethodHandles;
-
- // this constructor is available for tests, to be able to test the pure-reflective case, too
- DefaultAttributeFactory(boolean useMethodHandles) {
- this.useMethodHandles = useMethodHandles;
- }
+ private static final class DefaultAttributeFactory extends AttributeFactory {
+ private final ClassValue<MethodHandle> constructors = new ClassValue<MethodHandle>() {
+ @Override
+ protected MethodHandle computeValue(Class<?> attClass) {
+ return findAttributeImplCtor(findImplClass(attClass.asSubclass(Attribute.class)));
+ }
+ };
+
+ DefaultAttributeFactory() {}
@Override
public AttributeImpl createAttributeInstance(Class<? extends Attribute> attClass) {
- // first lookup from cache:
- Object cached = attClassImplMap.get(attClass);
- if (cached instanceof MethodHandle) {
- return invokeMethodHandle((MethodHandle) cached);
- } else if (cached instanceof Reference) {
- @SuppressWarnings("unchecked") final Class<? extends AttributeImpl> clazz =
- ((Reference<Class<? extends AttributeImpl>>) cached).get();
- if (clazz != null) {
- return invokeReflective(clazz);
- }
- cached = null;
- // fall-through
- }
- // No cache hit!
- // Please note: we have the slight chance that another thread may do the same, but who cares?
- assert cached == null;
- final Class<? extends AttributeImpl> implClazz = findImplClass(attClass);
- // if the attribute impl is from our own ClassLoader, we optimize to use pre-allocated MethodHandle to instantiate the object
- if (useMethodHandles && implClazz.getClassLoader() == myClassLoader) {
- final MethodHandle constr = findAttributeImplCtor(implClazz);
- attClassImplMap.put(attClass, constr);
- return invokeMethodHandle(constr);
- } else {
- // otherwise, to not refer to the class forever (because the MethodHandle strongly
- // references the class), so it can never be unloaded, we use slower reflection:
- attClassImplMap.put(attClass, new WeakReference<>(implClazz));
- return invokeReflective(implClazz);
+ try {
+ return (AttributeImpl) constructors.get(attClass).invokeExact();
+ } catch (Throwable t) {
+ rethrow(t);
+ throw new AssertionError();
}
}
@@ -104,23 +79,6 @@ public abstract class AttributeFactory {
throw new IllegalArgumentException("Cannot find implementing class for: " + attClass.getName());
}
}
-
- private AttributeImpl invokeMethodHandle(MethodHandle constr) {
- try {
- return (AttributeImpl) constr.invokeExact();
- } catch (Throwable t) {
- rethrow(t);
- throw new AssertionError();
- }
- }
-
- private AttributeImpl invokeReflective(Class<? extends AttributeImpl> implClass) {
- try {
- return implClass.newInstance();
- } catch (InstantiationException | IllegalAccessException e) {
- throw new IllegalArgumentException("Cannot instantiate implementing class: " + implClass.getName(), e);
- }
- }
}
/** <b>Expert</b>: AttributeFactory returning an instance of the given {@code clazz} for the
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeImpl.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeImpl.java?rev=1618148&r1=1618147&r2=1618148&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeImpl.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeImpl.java Fri Aug 15 10:50:13 2014
@@ -19,7 +19,6 @@ package org.apache.lucene.util;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
-import java.lang.ref.Reference;
/**
* Base class for Attributes that can be added to a
@@ -90,14 +89,12 @@ public abstract class AttributeImpl impl
*/
public void reflectWith(AttributeReflector reflector) {
final Class<? extends AttributeImpl> clazz = this.getClass();
- final Reference<Class<? extends Attribute>>[] interfaces = AttributeSource.getAttributeInterfaces(clazz);
+ final Class<? extends Attribute>[] interfaces = AttributeSource.getAttributeInterfaces(clazz);
if (interfaces.length != 1) {
throw new UnsupportedOperationException(clazz.getName() +
" implements more than one Attribute interface, the default reflectWith() implementation cannot handle this.");
}
- final Class<? extends Attribute> interf = interfaces[0].get();
- assert (interf != null) :
- "We have a strong reference on the class holding the interfaces, so they should never get evicted";
+ final Class<? extends Attribute> interf = interfaces[0];
final Field[] fields = clazz.getDeclaredFields();
try {
for (int i = 0; i < fields.length; i++) {
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeSource.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeSource.java?rev=1618148&r1=1618147&r2=1618148&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeSource.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/AttributeSource.java Fri Aug 15 10:50:13 2014
@@ -17,16 +17,14 @@ package org.apache.lucene.util;
* limitations under the License.
*/
-import java.lang.ref.Reference;
-import java.lang.ref.WeakReference;
-import java.util.ArrayList;
import java.util.Collections;
-import java.util.List;
-import java.util.NoSuchElementException;
import java.util.Iterator;
import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.NoSuchElementException;
+import java.util.Set;
import org.apache.lucene.analysis.TokenStream; // for javadocs
@@ -150,30 +148,28 @@ public class AttributeSource {
}
/** a cache that stores all interfaces for known implementation classes for performance (slow reflection) */
- private static final WeakIdentityMap<Class<? extends AttributeImpl>,Reference<Class<? extends Attribute>>[]> knownImplClasses =
- WeakIdentityMap.newConcurrentHashMap(false);
-
- static Reference<Class<? extends Attribute>>[] getAttributeInterfaces(final Class<? extends AttributeImpl> clazz) {
- Reference<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?
- final List<Reference<Class<? extends Attribute>>> intfList = new ArrayList<>();
+ private static final ClassValue<Class<? extends Attribute>[]> implInterfaces = new ClassValue<Class<? extends Attribute>[]>() {
+ @Override
+ protected Class<? extends Attribute>[] computeValue(Class<?> clazz) {
+ final Set<Class<? extends Attribute>> intfSet = new LinkedHashSet<>();
// find all interfaces that this attribute instance implements
// and that extend the Attribute interface
- Class<?> actClazz = clazz;
do {
- for (Class<?> curInterface : actClazz.getInterfaces()) {
+ for (Class<?> curInterface : clazz.getInterfaces()) {
if (curInterface != Attribute.class && Attribute.class.isAssignableFrom(curInterface)) {
- intfList.add(new WeakReference<Class<? extends Attribute>>(curInterface.asSubclass(Attribute.class)));
+ intfSet.add(curInterface.asSubclass(Attribute.class));
}
}
- actClazz = actClazz.getSuperclass();
- } while (actClazz != null);
- @SuppressWarnings({"unchecked", "rawtypes"}) final Reference<Class<? extends Attribute>>[] a =
- intfList.toArray(new Reference[intfList.size()]);
- knownImplClasses.put(clazz, foundInterfaces = a);
+ clazz = clazz.getSuperclass();
+ } while (clazz != null);
+ @SuppressWarnings({"unchecked", "rawtypes"}) final Class<? extends Attribute>[] a =
+ intfSet.toArray(new Class[intfSet.size()]);
+ return a;
}
- return foundInterfaces;
+ };
+
+ static Class<? extends Attribute>[] getAttributeInterfaces(final Class<? extends AttributeImpl> clazz) {
+ return implInterfaces.get(clazz);
}
/** <b>Expert:</b> Adds a custom AttributeImpl instance with one or more Attribute interfaces.
@@ -189,10 +185,7 @@ public class AttributeSource {
if (attributeImpls.containsKey(clazz)) return;
// add all interfaces of this AttributeImpl to the maps
- for (Reference<Class<? extends Attribute>> curInterfaceRef : getAttributeInterfaces(clazz)) {
- final Class<? extends Attribute> curInterface = curInterfaceRef.get();
- assert (curInterface != null) :
- "We have a strong reference on the class holding the interfaces, so they should never get evicted";
+ for (final Class<? extends Attribute> curInterface : getAttributeInterfaces(clazz)) {
// Attribute is a superclass of this interface
if (!attributes.containsKey(curInterface)) {
// invalidate state to force recomputation in captureState()
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java?rev=1618148&r1=1618147&r2=1618148&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java Fri Aug 15 10:50:13 2014
@@ -63,7 +63,12 @@ public final class VirtualMethod<C> {
private final Class<C> baseClass;
private final String method;
private final Class<?>[] parameters;
- private final WeakIdentityMap<Class<? extends C>, Integer> cache = WeakIdentityMap.newConcurrentHashMap(false);
+ private final ClassValue<Integer> distanceOfClass = new ClassValue<Integer>() {
+ @Override
+ protected Integer computeValue(Class<?> subclazz) {
+ return Integer.valueOf(reflectImplementationDistance(subclazz));
+ }
+ };
/**
* Creates a new instance for the given {@code baseClass} and method declaration.
@@ -92,12 +97,7 @@ public final class VirtualMethod<C> {
* @return 0 iff not overridden, else the distance to the base class
*/
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();
+ return distanceOfClass.get(subclazz).intValue();
}
/**
@@ -111,7 +111,7 @@ public final class VirtualMethod<C> {
return getImplementationDistance(subclazz) > 0;
}
- private int reflectImplementationDistance(final Class<? extends C> subclazz) {
+ int reflectImplementationDistance(final Class<?> subclazz) {
if (!baseClass.isAssignableFrom(subclazz))
throw new IllegalArgumentException(subclazz.getName() + " is not a subclass of " + baseClass.getName());
boolean overridden = false;
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java?rev=1618148&r1=1618147&r2=1618148&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java Fri Aug 15 10:50:13 2014
@@ -936,35 +936,15 @@ public abstract class BaseTokenStreamTes
return mockTokenizer;
}
- /**
- * This provides the default AttributeFactory in reflective-only mode (package private)
- * so we can test it.
- */
- private final static AttributeFactory REFLECTIVE_ATTRIBUTE_FACTORY;
- static {
- try {
- final Constructor<? extends AttributeFactory> constr = Class
- .forName(AttributeFactory.class.getName() + "$DefaultAttributeFactory")
- .asSubclass(AttributeFactory.class)
- .getDeclaredConstructor(boolean.class);
- constr.setAccessible(true);
- REFLECTIVE_ATTRIBUTE_FACTORY = constr.newInstance(false);
- } catch (ReflectiveOperationException e) {
- throw new Error("Cannot initantiate a reflective-only DefaultAttributeFactory", e);
- }
- }
-
/** Returns a random AttributeFactory impl */
public static AttributeFactory newAttributeFactory(Random random) {
- switch (random.nextInt(4)) {
+ switch (random.nextInt(3)) {
case 0:
return TokenStream.DEFAULT_TOKEN_ATTRIBUTE_FACTORY;
case 1:
return Token.TOKEN_ATTRIBUTE_FACTORY;
case 2:
return AttributeFactory.DEFAULT_ATTRIBUTE_FACTORY;
- case 3:
- return REFLECTIVE_ATTRIBUTE_FACTORY;
default:
throw new AssertionError("Please fix the Random.nextInt() call above");
}