Merge pull request mybatis#1758 from harawata/cache-methodhandle

Reuse MethodHandle for default methods
mnisummer · Dec 2, 2019 · 10db83c · 10db83c
2 parents bf01fee + 6294c85
commit 10db83c
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 29 deletions.
diff --git a/src/main/java/org/apache/ibatis/binding/MapperProxy.java b/src/main/java/org/apache/ibatis/binding/MapperProxy.java
@@ -16,11 +16,13 @@
 package org.apache.ibatis.binding;
 
 import java.io.Serializable;
+import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.Map;
 
@@ -33,16 +35,16 @@
  */
 public class MapperProxy<T> implements InvocationHandler, Serializable {
 
-  private static final long serialVersionUID = -6424540398559729838L;
+  private static final long serialVersionUID = -4724728412955527868L;
   private static final int ALLOWED_MODES = MethodHandles.Lookup.PRIVATE | MethodHandles.Lookup.PROTECTED
       | MethodHandles.Lookup.PACKAGE | MethodHandles.Lookup.PUBLIC;
   private static final Constructor<Lookup> lookupConstructor;
   private static final Method privateLookupInMethod;
   private final SqlSession sqlSession;
   private final Class<T> mapperInterface;
-  private final Map<Method, MapperMethod> methodCache;
+  private final Map<Method, MapperMethodInvoker> methodCache;
 
-  public MapperProxy(SqlSession sqlSession, Class<T> mapperInterface, Map<Method, MapperMethod> methodCache) {
+  public MapperProxy(SqlSession sqlSession, Class<T> mapperInterface, Map<Method, MapperMethodInvoker> methodCache) {
     this.sqlSession = sqlSession;
     this.mapperInterface = mapperInterface;
     this.methodCache = methodCache;
@@ -67,7 +69,7 @@ public MapperProxy(SqlSession sqlSession, Class<T> mapperInterface, Map<Method,
         throw new IllegalStateException(
             "There is neither 'privateLookupIn(Class, Lookup)' nor 'Lookup(Class, int)' method in java.lang.invoke.MethodHandles.",
             e);
-      } catch (Throwable t) {
+      } catch (Exception e) {
         lookup = null;
       }
     }
@@ -79,38 +81,81 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
     try {
       if (Object.class.equals(method.getDeclaringClass())) {
         return method.invoke(this, args);
-      } else if (method.isDefault()) {
-        if (privateLookupInMethod == null) {
-          return invokeDefaultMethodJava8(proxy, method, args);
-        } else {
-          return invokeDefaultMethodJava9(proxy, method, args);
-        }
+      } else {
+        return cachedInvoker(proxy, method, args).invoke(proxy, method, args, sqlSession);
       }
     } catch (Throwable t) {
       throw ExceptionUtil.unwrapThrowable(t);
     }
-    final MapperMethod mapperMethod = cachedMapperMethod(method);
-    return mapperMethod.execute(sqlSession, args);
   }
 
-  private MapperMethod cachedMapperMethod(Method method) {
-    return methodCache.computeIfAbsent(method,
-        k -> new MapperMethod(mapperInterface, method, sqlSession.getConfiguration()));
+  private MapperMethodInvoker cachedInvoker(Object proxy, Method method, Object[] args) throws Throwable {
+    try {
+      return methodCache.computeIfAbsent(method, m -> {
+        if (m.isDefault()) {
+          try {
+            if (privateLookupInMethod == null) {
+              return new DefaultMethodInvoker(getMethodHandleJava8(method));
+            } else {
+              return new DefaultMethodInvoker(getMethodHandleJava9(method));
+            }
+          } catch (IllegalAccessException | InstantiationException | InvocationTargetException
+              | NoSuchMethodException e) {
+            throw new RuntimeException(e);
+          }
+        } else {
+          return new PlainMethodInvoker(new MapperMethod(mapperInterface, method, sqlSession.getConfiguration()));
+        }
+      });
+    } catch (RuntimeException re) {
+      Throwable cause = re.getCause();
+      throw cause == null ? re : cause;
+    }
   }
 
-  private Object invokeDefaultMethodJava9(Object proxy, Method method, Object[] args)
-      throws Throwable {
+  private MethodHandle getMethodHandleJava9(Method method)
+      throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {
     final Class<?> declaringClass = method.getDeclaringClass();
-    return ((Lookup) privateLookupInMethod.invoke(null, declaringClass, MethodHandles.lookup()))
-        .findSpecial(declaringClass, method.getName(),
-            MethodType.methodType(method.getReturnType(), method.getParameterTypes()), declaringClass)
-        .bindTo(proxy).invokeWithArguments(args);
+    return ((Lookup) privateLookupInMethod.invoke(null, declaringClass, MethodHandles.lookup())).findSpecial(
+        declaringClass, method.getName(), MethodType.methodType(method.getReturnType(), method.getParameterTypes()),
+        declaringClass);
   }
 
-  private Object invokeDefaultMethodJava8(Object proxy, Method method, Object[] args)
-      throws Throwable {
+  private MethodHandle getMethodHandleJava8(Method method)
+      throws IllegalAccessException, InstantiationException, InvocationTargetException {
     final Class<?> declaringClass = method.getDeclaringClass();
-    return lookupConstructor.newInstance(declaringClass, ALLOWED_MODES).unreflectSpecial(method, declaringClass)
-        .bindTo(proxy).invokeWithArguments(args);
+    return lookupConstructor.newInstance(declaringClass, ALLOWED_MODES).unreflectSpecial(method, declaringClass);
+  }
+
+  interface MapperMethodInvoker {
+    Object invoke(Object proxy, Method method, Object[] args, SqlSession sqlSession) throws Throwable;
+  }
+
+  private static class PlainMethodInvoker implements MapperMethodInvoker {
+    private final MapperMethod mapperMethod;
+
+    public PlainMethodInvoker(MapperMethod mapperMethod) {
+      super();
+      this.mapperMethod = mapperMethod;
+    }
+
+    @Override
+    public Object invoke(Object proxy, Method method, Object[] args, SqlSession sqlSession) throws Throwable {
+      return mapperMethod.execute(sqlSession, args);
+    }
+  }
+
+  private static class DefaultMethodInvoker implements MapperMethodInvoker {
+    private final MethodHandle methodHandle;
+
+    public DefaultMethodInvoker(MethodHandle methodHandle) {
+      super();
+      this.methodHandle = methodHandle;
+    }
+
+    @Override
+    public Object invoke(Object proxy, Method method, Object[] args, SqlSession sqlSession) throws Throwable {
+      return methodHandle.bindTo(proxy).invokeWithArguments(args);
+    }
   }
 }
diff --git a/src/main/java/org/apache/ibatis/binding/MapperProxyFactory.java b/src/main/java/org/apache/ibatis/binding/MapperProxyFactory.java
@@ -1,5 +1,5 @@
 /**
- *    Copyright 2009-2018 the original author or authors.
+ *    Copyright 2009-2019 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+import org.apache.ibatis.binding.MapperProxy.MapperMethodInvoker;
 import org.apache.ibatis.session.SqlSession;
 
 /**
@@ -28,7 +29,7 @@
 public class MapperProxyFactory<T> {
 
   private final Class<T> mapperInterface;
-  private final Map<Method, MapperMethod> methodCache = new ConcurrentHashMap<>();
+  private final Map<Method, MapperMethodInvoker> methodCache = new ConcurrentHashMap<>();
 
   public MapperProxyFactory(Class<T> mapperInterface) {
     this.mapperInterface = mapperInterface;
@@ -38,7 +39,7 @@ public Class<T> getMapperInterface() {
     return mapperInterface;
   }
 
-  public Map<Method, MapperMethod> getMethodCache() {
+  public Map<Method, MapperMethodInvoker> getMethodCache() {
     return methodCache;
   }
 

diff --git a/src/test/java/org/apache/ibatis/binding/BindingTest.java b/src/test/java/org/apache/ibatis/binding/BindingTest.java
@@ -37,6 +37,7 @@
 
 import org.apache.ibatis.BaseDataTest;
 import org.apache.ibatis.builder.BuilderException;
+import org.apache.ibatis.binding.MapperProxy.MapperMethodInvoker;
 import org.apache.ibatis.cursor.Cursor;
 import org.apache.ibatis.domain.blog.Author;
 import org.apache.ibatis.domain.blog.Blog;
@@ -590,7 +591,7 @@ void shouldCacheMapperMethod() throws Exception {
       mapper.selectBlog(1);
       assertEquals(1, mapperProxyFactory.getMethodCache().size());
       assertTrue(mapperProxyFactory.getMethodCache().containsKey(selectBlog));
-      final MapperMethod cachedSelectBlog = mapperProxyFactory.getMethodCache().get(selectBlog);
+      final MapperMethodInvoker cachedSelectBlog = mapperProxyFactory.getMethodCache().get(selectBlog);
 
       // Call mapper method again and verify the cache is unchanged:
       session.clearCache();