From c7312d31f84832256c6063456613da08694101d6 Mon Sep 17 00:00:00 2001
From: James Baker <jbaker@palantir.com>
Date: Sat, 13 Aug 2022 15:19:30 +0100
Subject: [PATCH] TypeSafeMatching no longer iterates over class methods
 inefficiently

Fixes #2723.

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).
---
 .../internal/invocation/TypeSafeMatching.java | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/mockito/internal/invocation/TypeSafeMatching.java b/src/main/java/org/mockito/internal/invocation/TypeSafeMatching.java
index b4ca07aca6..8f8af6dbdb 100644
--- a/src/main/java/org/mockito/internal/invocation/TypeSafeMatching.java
+++ b/src/main/java/org/mockito/internal/invocation/TypeSafeMatching.java
@@ -4,15 +4,26 @@
  */
 package org.mockito.internal.invocation;
 
-import java.lang.reflect.Method;
-
 import org.mockito.ArgumentMatcher;
 
+import java.lang.reflect.Method;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
 @SuppressWarnings({"unchecked", "rawtypes"})
 public class TypeSafeMatching implements ArgumentMatcherAction {
 
     private static final ArgumentMatcherAction TYPE_SAFE_MATCHING_ACTION = new TypeSafeMatching();
 
+    /**
+     * This cache is in theory unbounded. However, its max size is bounded by the number of types of argument matchers
+     * that are both in the system and being used, which is expected to bound the cache's size to a low number
+     * (<200) in all but the most contrived cases, and form a small percentage of the overall memory usage of those
+     * classes.
+     */
+    private static final ConcurrentMap<Class<?>, Class<?>> argumentTypeCache =
+            new ConcurrentHashMap<>();
+
     private TypeSafeMatching() {}
 
     public static ArgumentMatcherAction matchesTypeSafe() {
@@ -39,11 +50,23 @@ private static boolean isCompatible(ArgumentMatcher<?> argumentMatcher, Object a
         return expectedArgumentType.isInstance(argument);
     }
 
+    private static Class<?> getArgumentType(ArgumentMatcher<?> matcher) {
+        Class<?> argumentMatcherType = matcher.getClass();
+        Class<?> cached = argumentTypeCache.get(argumentMatcherType);
+        // Avoids a lambda allocation on invocations >=2 for worse perf on invocation 1.
+        if (cached != null) {
+            return cached;
+        } else {
+            return argumentTypeCache.computeIfAbsent(
+                    argumentMatcherType, unusedKey -> getArgumentTypeUncached(matcher));
+        }
+    }
+
     /**
      * Returns the type of {@link ArgumentMatcher#matches(Object)} of the given
      * {@link ArgumentMatcher} implementation.
      */
-    private static Class<?> getArgumentType(ArgumentMatcher<?> argumentMatcher) {
+    private static Class<?> getArgumentTypeUncached(ArgumentMatcher<?> argumentMatcher) {
         Method[] methods = argumentMatcher.getClass().getMethods();
 
         for (Method method : methods) {