Skip to content

Commit 89698ba

Browse files
authored
Optimize TypeSafeMatching iteration over class methods (#2729)
`Class.getMethods` is an inefficient method call which is being called on each mock invocation. It ends up constructing new `Method` objects for each method on the class, and this can dominate the overall performance of Mockito mocks. This commit caches the result of the computation. Once concern is that this change uses some static state. I considered: - Instance state - based on where this type is constructed it seemed like it'd be a big imposition on code readability elsewhere. - Weakly referenced map. Mockito has a type for this, but the constructor of that type produces a Thread with which to clean up. Both of these seemed like overkill compared to the overhead expected in the real world (which should be on the order of a few kilobytes of RAM at most). Fixes #2723
1 parent 70c1fe9 commit 89698ba

File tree

1 file changed

+26
-3
lines changed

1 file changed

+26
-3
lines changed

src/main/java/org/mockito/internal/invocation/TypeSafeMatching.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,26 @@
44
*/
55
package org.mockito.internal.invocation;
66

7-
import java.lang.reflect.Method;
8-
97
import org.mockito.ArgumentMatcher;
108

9+
import java.lang.reflect.Method;
10+
import java.util.concurrent.ConcurrentHashMap;
11+
import java.util.concurrent.ConcurrentMap;
12+
1113
@SuppressWarnings({"unchecked", "rawtypes"})
1214
public class TypeSafeMatching implements ArgumentMatcherAction {
1315

1416
private static final ArgumentMatcherAction TYPE_SAFE_MATCHING_ACTION = new TypeSafeMatching();
1517

18+
/**
19+
* This cache is in theory unbounded. However, its max size is bounded by the number of types of argument matchers
20+
* that are both in the system and being used, which is expected to bound the cache's size to a low number
21+
* (<200) in all but the most contrived cases, and form a small percentage of the overall memory usage of those
22+
* classes.
23+
*/
24+
private static final ConcurrentMap<Class<?>, Class<?>> argumentTypeCache =
25+
new ConcurrentHashMap<>();
26+
1627
private TypeSafeMatching() {}
1728

1829
public static ArgumentMatcherAction matchesTypeSafe() {
@@ -39,11 +50,23 @@ private static boolean isCompatible(ArgumentMatcher<?> argumentMatcher, Object a
3950
return expectedArgumentType.isInstance(argument);
4051
}
4152

53+
private static Class<?> getArgumentType(ArgumentMatcher<?> matcher) {
54+
Class<?> argumentMatcherType = matcher.getClass();
55+
Class<?> cached = argumentTypeCache.get(argumentMatcherType);
56+
// Avoids a lambda allocation on invocations >=2 for worse perf on invocation 1.
57+
if (cached != null) {
58+
return cached;
59+
} else {
60+
return argumentTypeCache.computeIfAbsent(
61+
argumentMatcherType, unusedKey -> getArgumentTypeUncached(matcher));
62+
}
63+
}
64+
4265
/**
4366
* Returns the type of {@link ArgumentMatcher#matches(Object)} of the given
4467
* {@link ArgumentMatcher} implementation.
4568
*/
46-
private static Class<?> getArgumentType(ArgumentMatcher<?> argumentMatcher) {
69+
private static Class<?> getArgumentTypeUncached(ArgumentMatcher<?> argumentMatcher) {
4770
Method[] methods = argumentMatcher.getClass().getMethods();
4871

4972
for (Method method : methods) {

0 commit comments

Comments
 (0)