You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2019/09/21 14:23:21 UTC

[isis] branch v2 updated: ISIS-2158: performance improvement for MethodFinderUtils

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch v2
in repository https://gitbox.apache.org/repos/asf/isis.git


The following commit(s) were added to refs/heads/v2 by this push:
     new a7a4586  ISIS-2158: performance improvement for MethodFinderUtils
a7a4586 is described below

commit a7a4586d5251605d6f383977584be53680ba7fa2
Author: Andi Huber <ah...@apache.org>
AuthorDate: Sat Sep 21 16:23:11 2019 +0200

    ISIS-2158: performance improvement for MethodFinderUtils
    
    - JDK reflection API has no Class.getMethod(name, ...) variant that does
    not produce an expensive stacktrace, when no such method exists; by
    introducing a _MethodCache (internal API) we see the following
    improvements with a simple headless smoketest context:
    
    - previously 44k calls to Class.getMethod taking 500ms
    - with the new _MethodCache the MethodFinderUtils take only 16ms
---
 .../isis/commons/internal/collections/_Arrays.java |  4 +-
 .../commons/internal/reflection/_MethodCache.java  | 76 ++++++++++++++++++++++
 .../isis/metamodel/facets/MethodFinderUtils.java   | 16 +++--
 ...tionParameterDefaultsFacetViaMethodFactory.java | 51 ++++++++-------
 4 files changed, 115 insertions(+), 32 deletions(-)

diff --git a/core/commons/src/main/java/org/apache/isis/commons/internal/collections/_Arrays.java b/core/commons/src/main/java/org/apache/isis/commons/internal/collections/_Arrays.java
index c4f1dc6..49e28b3 100644
--- a/core/commons/src/main/java/org/apache/isis/commons/internal/collections/_Arrays.java
+++ b/core/commons/src/main/java/org/apache/isis/commons/internal/collections/_Arrays.java
@@ -233,10 +233,10 @@ public final class _Arrays {
      * @return
      */
     public static <T> T[] removeByIndex(T[] array, int index) {
-        if(array==null || array.length<=1) {
+        if(array==null || array.length<1) {
             throw new IllegalArgumentException("Array must be of lenght 1 or larger.");
         }
-        if(index>=array.length) {
+        if(index<0 || index>=array.length) {
             val msg = String.format("Array index %d is out of bounds [0, %d]", index, array.length-1);
             throw new IllegalArgumentException(msg);
         }
diff --git a/core/commons/src/main/java/org/apache/isis/commons/internal/reflection/_MethodCache.java b/core/commons/src/main/java/org/apache/isis/commons/internal/reflection/_MethodCache.java
new file mode 100644
index 0000000..14a4c5f
--- /dev/null
+++ b/core/commons/src/main/java/org/apache/isis/commons/internal/reflection/_MethodCache.java
@@ -0,0 +1,76 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.isis.commons.internal.reflection;
+
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
+import lombok.val;
+
+public final class _MethodCache {
+
+    public Method lookupMethod(Class<?> type, String name, Class<?>[] paramTypes) {
+        
+        if(!inspectedTypes.contains(type)) {
+            for(val method : type.getDeclaredMethods()) {
+                methodsByKey.put(Key.of(type, method), method);
+            }
+            inspectedTypes.add(type);
+        }
+        
+        return methodsByKey.get(Key.of(type, name, nullify(paramTypes)));
+    }
+
+    public int size() {
+        return methodsByKey.size();
+    }
+
+    // -- IMPLEMENATION DETAILS
+    
+    private Map<Key, Method> methodsByKey = new HashMap<>();
+    private Set<Class<?>> inspectedTypes = new HashSet<>();
+    
+    @AllArgsConstructor(staticName = "of") @EqualsAndHashCode
+    private final static class Key {
+        private final Class<?> type;
+        private final String name;
+        private final Class<?>[] paramTypes;
+        
+        public static Key of(Class<?> type, Method method) {
+            return Key.of(type, method.getName(), nullify(method.getParameterTypes()));
+        }
+        
+        
+    }
+    
+    private static Class<?>[] nullify(Class<?>[] x) {
+        if(x!=null && x.length==0) {
+            return null;
+        }
+        return x;
+    }
+    
+    
+    
+}
diff --git a/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/MethodFinderUtils.java b/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/MethodFinderUtils.java
index a52d557..92d5a9e 100644
--- a/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/MethodFinderUtils.java
+++ b/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/MethodFinderUtils.java
@@ -26,9 +26,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
+import org.apache.isis.commons.internal.reflection._MethodCache;
 import org.apache.isis.metamodel.facetapi.MethodRemover;
 import org.apache.isis.metamodel.methodutils.MethodScope;
 
+import lombok.val;
+
 public final class MethodFinderUtils {
 
     private MethodFinderUtils() {
@@ -41,6 +44,8 @@ public final class MethodFinderUtils {
         }
         return method;
     }
+    
+    private static final _MethodCache methodCache = new _MethodCache(); //TODO could be share on the context
 
     /**
      * Returns a specific public methods that: have the specified prefix; have
@@ -60,13 +65,9 @@ public final class MethodFinderUtils {
             final String name,
             final Class<?> returnType,
             final Class<?>[] paramTypes) {
-
-        Method method;
-        try {
-            method = type.getMethod(name, paramTypes);
-        } catch (final SecurityException e) {
-            return null;
-        } catch (final NoSuchMethodException e) {
+        
+        val method = methodCache.lookupMethod(type, name, paramTypes);
+        if(method == null) {
             return null;
         }
 
@@ -104,6 +105,7 @@ public final class MethodFinderUtils {
 
         return method;
     }
+    
 
     public static Method findMethod(
             final Class<?> type,
diff --git a/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/param/defaults/methodnum/ActionParameterDefaultsFacetViaMethodFactory.java b/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/param/defaults/methodnum/ActionParameterDefaultsFacetViaMethodFactory.java
index 85f3b92..4dd534a 100644
--- a/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/param/defaults/methodnum/ActionParameterDefaultsFacetViaMethodFactory.java
+++ b/core/metamodel/src/main/java/org/apache/isis/metamodel/facets/param/defaults/methodnum/ActionParameterDefaultsFacetViaMethodFactory.java
@@ -22,7 +22,7 @@ package org.apache.isis.metamodel.facets.param.defaults.methodnum;
 import java.lang.reflect.Method;
 import java.util.List;
 
-import org.apache.isis.metamodel.commons.ListExtensions;
+import org.apache.isis.commons.internal.collections._Arrays;
 import org.apache.isis.metamodel.commons.StringExtensions;
 import org.apache.isis.metamodel.exceptions.MetaModelException;
 import org.apache.isis.metamodel.facetapi.Facet;
@@ -36,6 +36,8 @@ import org.apache.isis.metamodel.facets.MethodPrefixConstants;
 import org.apache.isis.metamodel.facets.actions.defaults.ActionDefaultsFacet;
 import org.apache.isis.metamodel.methodutils.MethodScope;
 
+import lombok.val;
+
 /**
  * Sets up all the {@link Facet}s for an action in a single shot.
  */
@@ -94,38 +96,41 @@ public class ActionParameterDefaultsFacetViaMethodFactory extends MethodPrefixBa
             FacetUtil.addFacet(new ActionParameterDefaultsFacetViaMethod(defaultMethod, i, paramAsHolder));
         }
     }
-
+    
     /**
      * search successively for the default method, trimming number of param types each loop
      */
     private static Method findDefaultNumMethod(ProcessMethodContext processMethodContext, int n) {
-
-        final Method actionMethod = processMethodContext.getMethod();
-        final List<Class<?>> paramTypes = ListExtensions.mutableCopy(actionMethod.getParameterTypes());
-
-        final int numParamTypes = paramTypes.size();
-
-        for(int i=0; i< numParamTypes+1; i++) {
-            final Method method = findDefaultNumMethod(processMethodContext, n, paramTypes.toArray(new Class<?>[]{}));
+        
+        val cls = processMethodContext.getCls();
+        val actionMethod = processMethodContext.getMethod();
+        Class<?>[] paramTypes = actionMethod.getParameterTypes();
+        val returnType = paramTypes[n];
+        val capitalizedName =
+                MethodPrefixConstants.DEFAULT_PREFIX + n +
+                StringExtensions.asCapitalizedName(actionMethod.getName());
+
+        for(;;) {
+            val method = MethodFinderUtils.findMethod(
+                    cls, 
+                    MethodScope.OBJECT, 
+                    capitalizedName, 
+                    returnType, 
+                    paramTypes);
+            
             if(method != null) {
                 return method;
             }
-            // remove last, and search again
-            if(!paramTypes.isEmpty()) {
-                paramTypes.remove(paramTypes.size()-1);
+            
+            if(paramTypes.length==0) {
+                break;
             }
+            
+            // remove last, and search again
+            paramTypes = _Arrays.removeByIndex(paramTypes, paramTypes.length-1);
         }
-
+        
         return null;
     }
 
-    private static Method findDefaultNumMethod(final ProcessMethodContext processMethodContext, int n, Class<?>[] paramTypes) {
-        final Class<?> cls = processMethodContext.getCls();
-        final Method actionMethod = processMethodContext.getMethod();
-        final Class<?> returnType = actionMethod.getParameterTypes()[n];
-        final String capitalizedName = StringExtensions.asCapitalizedName(actionMethod.getName());
-        return MethodFinderUtils.findMethod(cls, MethodScope.OBJECT, MethodPrefixConstants.DEFAULT_PREFIX + n + capitalizedName, returnType, paramTypes);
-    }
-
-
 }