This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-beanutils.git
The following commit(s) were added to refs/heads/master by this push:
new 21d335c7 Update the global cache from a synchronized map to a
concurrent map
21d335c7 is described below
commit 21d335c76c11c0fed21984716861efb49a4971ca
Author: Gary D. Gregory <garydgreg...@gmail.com>
AuthorDate: Mon Aug 25 17:39:37 2025 -0400
Update the global cache from a synchronized map to a concurrent map
---
.../org/apache/commons/beanutils2/MethodUtils.java | 257 ++++-----------------
1 file changed, 51 insertions(+), 206 deletions(-)
diff --git a/src/main/java/org/apache/commons/beanutils2/MethodUtils.java
b/src/main/java/org/apache/commons/beanutils2/MethodUtils.java
index 58a161a2..636d6404 100644
--- a/src/main/java/org/apache/commons/beanutils2/MethodUtils.java
+++ b/src/main/java/org/apache/commons/beanutils2/MethodUtils.java
@@ -17,17 +17,15 @@
package org.apache.commons.beanutils2;
-import java.lang.ref.Reference;
-import java.lang.ref.WeakReference;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
-import java.util.Collections;
import java.util.Map;
import java.util.Objects;
-import java.util.WeakHashMap;
+import java.util.function.Function;
-import org.apache.commons.lang3.ClassUtils;
+import org.apache.commons.collections4.map.ConcurrentReferenceHashMap;
+import
org.apache.commons.collections4.map.ConcurrentReferenceHashMap.ReferenceType;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -57,6 +55,7 @@ public final class MethodUtils {
* Represents the key to looking up a Method by reflection.
*/
private static final class MethodKey {
+
private final Class<?> cls;
private final String methodName;
private final Class<?>[] paramTypes;
@@ -104,6 +103,11 @@ public final class MethodUtils {
public int hashCode() {
return hashCode;
}
+
+ @Override
+ public String toString() {
+ return "MethodKey [cls=" + cls + ", methodName=" + methodName + ",
paramTypes=" + Arrays.toString(paramTypes) + ", exact=" + exact + "]";
+ }
}
private static final Log LOG = LogFactory.getLog(MethodUtils.class);
@@ -115,35 +119,28 @@ public final class MethodUtils {
* would mean having a map keyed by context classloader which may
introduce memory-leak problems.
* </p>
*/
- private static boolean CACHE_METHODS = true;
+ private static boolean CACHE_ENABLED = true;
/**
- * Stores a cache of MethodDescriptor -> Method in a WeakHashMap.
+ * Stores a cache of MethodKey -> Method.
* <p>
* The keys into this map only ever exist as temporary variables within
methods of this class, and are never exposed to users of this class. This means
that
- * the WeakHashMap is used only as a mechanism for limiting the size of
the cache, that is, a way to tell the garbage collector that the contents of the
- * cache can be completely garbage-collected whenever it needs the memory.
Whether this is a good approach to this problem is doubtful; something like the
- * commons-collections LRUMap may be more appropriate (though of course
selecting an appropriate size is an issue).
+ * this map is used only as a mechanism for limiting the size of the
cache, that is, a way to tell the garbage collector that the contents of the
cache can
+ * be completely garbage-collected whenever it needs the memory. Whether
this is a good approach to this problem is doubtful; something like the Commons
+ * Collections LRUMap may be more appropriate (though of course selecting
an appropriate size is an issue).
* </p>
* <p>
- * This static variable is safe even when this code is deployed via a
shared classloader because it is keyed via a MethodDescriptor object which has
a Class
- * as one of its members and that member is used in the
MethodDescriptor.equals method. So two components that load the same class via
different class
- * loaders will generate non-equal MethodDescriptor objects and hence end
up with different entries in the map.
+ * This static variable is safe even when this code is deployed via a
shared class loader because it is keyed via a MethodKey object which has a
Class as
+ * one of its members and that member is used in the MethodKey.equals
method. So two components that load the same class via different class loaders
will
+ * generate non-equal MethodKey objects and hence end up with different
entries in the map.
* </p>
*/
- private static final Map<MethodKey, Reference<Method>> CACHE =
Collections.synchronizedMap(new WeakHashMap<>());
-
- /**
- * Add a method to the cache.
- *
- * @param key The method descriptor.
- * @param method The method to cache.
- */
- private static void cacheMethod(final MethodKey key, final Method method) {
- if (CACHE_METHODS && method != null) {
- CACHE.put(key, new WeakReference<>(method));
- }
- }
+ // @formatter:off
+ private static final Map<MethodKey, Method> CACHE = new
ConcurrentReferenceHashMap.Builder<MethodKey, Method>()
+ .setKeyReferenceType(ReferenceType.WEAK)
+ .setValueReferenceType(ReferenceType.WEAK)
+ .get();
+ // @formatter:on
/**
* Clear the method cache.
@@ -157,9 +154,17 @@ public final class MethodUtils {
return size;
}
+ private static Method computeIfAbsent(final MethodKey key, final
Function<MethodKey, ? extends Method> mappingFunction) {
+ final Method method = CACHE_ENABLED ? CACHE.computeIfAbsent(key, k ->
mappingFunction.apply(k)) : mappingFunction.apply(key);
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Matched " + key + " with method: " + method + ",
CACHE_ENABLED: " + CACHE_ENABLED);
+ }
+ return method;
+ }
+
/**
- * Gets an accessible method (that is, one that can be invoked via
reflection) that implements the specified Method. If no such method can be
found,
- * return {@code null}.
+ * Gets an accessible method (that is, one that can be invoked via
reflection) that implements the specified Method. If no such method can be
found, return
+ * {@code null}.
*
* @param clazz The class of the object.
* @param method The method that we wish to call.
@@ -171,12 +176,10 @@ public final class MethodUtils {
if (method == null) {
return null;
}
-
// If the requested method is not public we cannot call it
if (!Modifier.isPublic(method.getModifiers())) {
return null;
}
-
boolean sameClass = true;
if (clazz == null) {
clazz = method.getDeclaringClass();
@@ -186,7 +189,6 @@ public final class MethodUtils {
}
sameClass = clazz.equals(method.getDeclaringClass());
}
-
// If the class is public, we are done
if (Modifier.isPublic(clazz.getModifiers())) {
if (!sameClass &&
!Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
@@ -194,18 +196,14 @@ public final class MethodUtils {
}
return method;
}
-
final String methodName = method.getName();
final Class<?>[] parameterTypes = method.getParameterTypes();
-
// Check the implemented interfaces and subinterfaces
method = getAccessibleMethodFromInterfaceNest(clazz, methodName,
parameterTypes);
-
// Check the superclass chain
if (method == null) {
method = getAccessibleMethodFromSuperclass(clazz, methodName,
parameterTypes);
}
-
return method;
}
@@ -219,24 +217,18 @@ public final class MethodUtils {
* @return The accessible method.
*/
public static Method getAccessibleMethod(final Class<?> clazz, final
String methodName, final Class<?>... parameterTypes) {
- try {
- final MethodKey key = new MethodKey(clazz, methodName,
parameterTypes, true);
- // Check the cache first
- Method method = getCachedMethod(key);
- if (method != null) {
- return method;
+ return computeIfAbsent(new MethodKey(clazz, methodName,
parameterTypes, true), k -> {
+ try {
+ return getAccessibleMethod(clazz, clazz.getMethod(methodName,
parameterTypes));
+ } catch (final NoSuchMethodException e) {
+ return null;
}
- method = getAccessibleMethod(clazz, clazz.getMethod(methodName,
parameterTypes));
- cacheMethod(key, method);
- return method;
- } catch (final NoSuchMethodException e) {
- return null;
- }
+ });
}
/**
- * Gets an accessible method (that is, one that can be invoked via
reflection) that implements the specified Method. If no such method can be
found,
- * return {@code null}.
+ * Gets an accessible method (that is, one that can be invoked via
reflection) that implements the specified Method. If no such method can be
found, return
+ * {@code null}.
*
* @param method The method that we wish to call.
* @return The accessible method.
@@ -292,8 +284,8 @@ public final class MethodUtils {
}
/**
- * Gets an accessible method (that is, one that can be invoked via
reflection) by scanning through the superclasses. If no such method can be
found,
- * return {@code null}.
+ * Gets an accessible method (that is, one that can be invoked via
reflection) by scanning through the superclasses. If no such method can be
found, return
+ * {@code null}.
*
* @param clazz Class to be checked.
* @param methodName Method name of the method we wish to call.
@@ -314,22 +306,6 @@ public final class MethodUtils {
return null;
}
- /**
- * Gets the method from the cache, if present.
- *
- * @param key The method descriptor.
- * @return The cached method.
- */
- private static Method getCachedMethod(final MethodKey key) {
- if (CACHE_METHODS) {
- final Reference<Method> methodRef = CACHE.get(key);
- if (methodRef != null) {
- return methodRef.get();
- }
- }
- return null;
- }
-
/**
* Gets an accessible method that matches the given name and has
compatible parameters. Compatible parameters mean that every method parameter
is assignable
* from the given parameters. In other words, it finds a method with the
given name that will take the parameters given.
@@ -341,6 +317,7 @@ public final class MethodUtils {
* <p>
* This method can match primitive parameter by passing in wrapper
classes. For example, a {@code Boolean</code> will match a primitive
<code>boolean}
* parameter.
+ * </p>
*
* @param clazz find method in this class.
* @param methodName find method with this name.
@@ -348,145 +325,13 @@ public final class MethodUtils {
* @return The accessible method.
*/
public static Method getMatchingAccessibleMethod(final Class<?> clazz,
final String methodName, final Class<?>[] parameterTypes) {
- // trace logging
- if (LOG.isTraceEnabled()) {
- LOG.trace("Matching name=" + methodName + " on " + clazz);
- }
- final MethodKey key = new MethodKey(clazz, methodName, parameterTypes,
false);
- // see if we can find the method directly
- // most of the time this works and it's much faster
- try {
- // Check the cache first
- Method method = getCachedMethod(key);
+ return computeIfAbsent(new MethodKey(clazz, methodName,
parameterTypes, false), k -> {
+ final Method method =
org.apache.commons.lang3.reflect.MethodUtils.getMatchingAccessibleMethod(clazz,
methodName, parameterTypes);
if (method != null) {
- return method;
- }
- method = clazz.getMethod(methodName, parameterTypes);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Found straight match: " + method);
- LOG.trace("isPublic:" +
Modifier.isPublic(method.getModifiers()));
+ setMethodAccessible(method); // Default access superclass
workaround
}
- setMethodAccessible(method); // Default access superclass
workaround
- cacheMethod(key, method);
return method;
- } catch (final NoSuchMethodException e) {
- /* SWALLOW */ }
- // search through all methods
- final int paramSize = parameterTypes.length;
- Method bestMatch = null;
- final Method[] methods = clazz.getMethods();
- float bestMatchCost = Float.MAX_VALUE;
- float myCost = Float.MAX_VALUE;
- for (final Method method2 : methods) {
- if (method2.getName().equals(methodName)) {
- // log some trace information
- if (LOG.isTraceEnabled()) {
- LOG.trace("Found matching name:");
- LOG.trace(method2);
- }
- // compare parameters
- final Class<?>[] methodsParams = method2.getParameterTypes();
- final int methodParamSize = methodsParams.length;
- if (methodParamSize == paramSize) {
- boolean match = true;
- for (int n = 0; n < methodParamSize; n++) {
- if (LOG.isTraceEnabled()) {
- LOG.trace("Param=" + parameterTypes[n].getName());
- LOG.trace("Method=" + methodsParams[n].getName());
- }
- if (!ClassUtils.isAssignable(parameterTypes[n],
methodsParams[n])) {
- if (LOG.isTraceEnabled()) {
- LOG.trace(methodsParams[n] + " is not
assignable from " + parameterTypes[n]);
- }
- match = false;
- break;
- }
- }
- if (match) {
- // get accessible version of method
- final Method method = getAccessibleMethod(clazz,
method2);
- if (method != null) {
- if (LOG.isTraceEnabled()) {
- LOG.trace(method + " accessible version of " +
method2);
- }
- setMethodAccessible(method); // Default access
superclass workaround
- myCost =
getTotalTransformationCost(parameterTypes, method.getParameterTypes());
- if (myCost < bestMatchCost) {
- bestMatch = method;
- bestMatchCost = myCost;
- }
- }
- LOG.trace("Couldn't find accessible method.");
- }
- }
- }
- }
- if (bestMatch != null) {
- cacheMethod(key, bestMatch);
- } else {
- // didn't find a match
- LOG.trace("No match found.");
- }
- return bestMatch;
- }
-
- /**
- * Gets the number of steps required needed to turn the source class into
the destination class. This represents the number of steps in the object
hierarchy
- * graph.
- *
- * @param srcClass The source class.
- * @param destClass The destination class.
- * @return The cost of transforming an object.
- */
- private static float getObjectTransformationCost(Class<?> srcClass, final
Class<?> destClass) {
- float cost = 0.0f;
- while (srcClass != null && !destClass.equals(srcClass)) {
- if (destClass.isPrimitive()) {
- final Class<?> destClassWrapperClazz =
ClassUtils.wrapperToPrimitive(destClass);
- if (destClassWrapperClazz != null &&
destClassWrapperClazz.equals(srcClass)) {
- cost += 0.25f;
- break;
- }
- }
- final Class<?> cls = srcClass;
- if (destClass.isInterface() && ClassUtils.isAssignable(cls,
destClass)) {
- // slight penalty for interface match.
- // we still want an exact match to override an interface
match, but
- // an interface match should override anything where we have
to get a
- // superclass.
- cost += 0.25f;
- break;
- }
- cost++;
- srcClass = srcClass.getSuperclass();
- }
-
- /*
- * If the destination class is null, we've traveled all the way up to
an Object match. We'll penalize this by adding 1.5 to the cost.
- */
- if (srcClass == null) {
- cost += 1.5f;
- }
-
- return cost;
- }
-
- /**
- * Gets the sum of the object transformation cost for each class in the
source argument list.
- *
- * @param srcArgs The source arguments.
- * @param destArgs The destination arguments.
- * @return The total transformation cost.
- */
- private static float getTotalTransformationCost(final Class<?>[] srcArgs,
final Class<?>[] destArgs) {
- float totalCost = 0.0f;
- for (int i = 0; i < srcArgs.length; i++) {
- Class<?> srcClass, destClass;
- srcClass = srcArgs[i];
- destClass = destArgs[i];
- totalCost += getObjectTransformationCost(srcClass, destClass);
- }
- return totalCost;
+ });
}
/**
@@ -496,9 +341,9 @@ public final class MethodUtils {
* @since 1.8.0
*/
public static synchronized void setCacheMethods(final boolean
cacheMethods) {
- CACHE_METHODS = cacheMethods;
- if (!CACHE_METHODS) {
- clearCache();
+ CACHE_ENABLED = cacheMethods;
+ if (!CACHE_ENABLED) {
+ CACHE.clear();
}
}
