Interceptor optimization

[brucelwl]

If there are multiple interceptors intercepting the same method, the same number of dynamic proxy classes will be created. However, the dynamic proxy has certain performance loss. I hope that it can be optimized to avoid the creation of multiple dynamic proxy classes.
example：

@Intercepts({
        @Signature(type = Executor.class, method = "update", args = {MappedStatement.class, Object.class})

})
public class MyInterceptor implements Interceptor {
    private static final Logger logger = LoggerFactory.getLogger(MyInterceptor.class);

    private Properties properties;

    @Override
    public void setProperties(Properties properties) {
        this.properties = properties;
    }

    @Override
    public Object intercept(Invocation invocation) throws Throwable {
        logger.info("invoke");
        return invocation.proceed();
    }
}

@Intercepts({
        @Signature(type = Executor.class, method = "update", args = {MappedStatement.class, Object.class})

})
public class MyInterceptor2 implements Interceptor {
    
}

This will cause two dynamic proxy classes to be created !!!

At present, Mybatis supports four types of interface interception
ParameterHandler, ResultSetHandler, StatementHandler, Executor

Proposal 1: change to responsibility chain mode，developers can provide specific implementation classes of these interfaces to intercept corresponding methods

Proposal 2: Only a proxy class can be used to intercept interface methods，The advantage of this approach is that it does not change the existing interceptor implementation logic

[brucelwl]

@harawata Can you reply to my issue?

[harawata]

Hello @brucelwl ,

I'm sorry, but I am not so familiar with this topic.
Could you post some data showing the 'certain performance loss', please?

[brucelwl]

@harawata
I did a test. If an interface is proxy only once, the performance is much higher than that of proxy multiple times, and the performance increases in proportion to the number of agents

environment：
windows 10
JDK：JDK 1.8.0_171, Java HotSpot(TM) 64-Bit Server VM, 25.171-b11
CPU: i5
pc memory：16G
Other: all defaults
Benchmarking tools：JMH

Benchmark code

public interface ExecutorHandler {
    int getProxyCount();

    int update(MappedStatement ms, Object parameter) throws SQLException;

    <E> List<E> query(MappedStatement ms, Object parameter);
}

public class ExecutorHandlerImpl implements ExecutorHandler {

    private int count;

    public ExecutorHandlerImpl(int count) {
        this.count = count;
    }

    @Override
    public int getProxyCount() {
        return count;
    }

    @Override
    public int update(MappedStatement ms, Object parameter) throws SQLException {
        return 10 * 20 / 5 * 2 + 1 + 15;
    }

    @Override
    public <E> List<E> query(MappedStatement ms, Object parameter) {
        return null;
    }
}

public class Plugin implements InvocationHandler {

    private final Object target;

    private Plugin(Object target) {
        this.target = target;
    }

    public static Object wrap(Object target) {
        Class<?> type = target.getClass();
        Class<?>[] interfaces = getAllInterfaces(type);
        if (interfaces.length > 0) {
            return Proxy.newProxyInstance(type.getClassLoader(), interfaces, new Plugin(target));
        }
        return target;
    }

    private static Class<?>[] getAllInterfaces(Class<?> type) {
        Set<Class<?>> interfaces = new HashSet<>();
        while (type != null) {
            interfaces.addAll(Arrays.asList(type.getInterfaces()));
            type = type.getSuperclass();
        }
        return interfaces.toArray(new Class<?>[0]);
    }

    @Override
    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
        return method.invoke(target, args);
    }
}


public class InterceptorChain {

    public Object pluginAll(ExecutorHandler target) {
        int proxyCount = target.getProxyCount();

        for (int i = 0; i < proxyCount; i++) {
            target = (ExecutorHandler) Plugin.wrap(target);
        }
        return target;
    }
}

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 1) 
@Threads(15)
@State(Scope.Benchmark)
@Measurement(iterations = 2, time = 5, timeUnit = TimeUnit.SECONDS)
@OutputTimeUnit(TimeUnit.NANOSECONDS) 
public class ProxyBenchmarkTest {
    private static final Logger logger = LoggerFactory.getLogger(ProxyBenchmarkTest.class);

    ExecutorHandler executorHandler;
    ExecutorHandler executorHandler2;

    @Setup
    public void setup() throws Exception {
        InterceptorChain interceptorChain = new InterceptorChain();
        executorHandler = (ExecutorHandler) interceptorChain.pluginAll(new ExecutorHandlerImpl(1));
        executorHandler2 = (ExecutorHandler) interceptorChain.pluginAll(new ExecutorHandlerImpl(15));
    }

    @Benchmark
    public void jdkProxyN(Blackhole blackhole) throws Exception {
        MappedStatement mappedStatement = new MappedStatement(
                "select * from user info", 5,500,"c://aaa.mapper");

        List<Object> query = executorHandler2.query(mappedStatement, 45);
        blackhole.consume(query);
    }

    @Benchmark
    public void jdkProxy1(Blackhole blackhole) throws Exception {
        MappedStatement mappedStatement = new MappedStatement(
                "select * from user info", 5,500,"c://aaa.mapper");

        List<Object> query = executorHandler.query(mappedStatement, 45);
        blackhole.consume(query);
    }

    public static void main(String[] args) throws Exception {
        Options options = new OptionsBuilder().include(ProxyBenchmarkTest.class.getName())
                //.output("benchmark/jedis-Throughput.log")
                .forks(0)
                .build();
        new Runner(options).run();

    }
}

Proxy 1 time vs Proxy 3 times

Proxy 1 time vs Proxy 5 times

Proxy 1 time vs Proxy 8 times

[brucelwl]

@harawata
see PR https://github.com/mybatis/mybatis-3/pull/2001/files please
This PR has two main purposes:

1. The first goal is performance improvement
2. The second goal is to make the generated proxy class structure clearer

The current implementation code will make the proxy class nest another proxy. The more interceptors, the more nested the proxy class. But in fact, if only one proxy class is used, the class structure will be clearer

[harawata]

Thank you, @brucelwl .

I will look into it, but it may take some time as I don't have much spare time lately.

[brucelwl]

@harawata
Another more optimized way is to use the decorator pattern to form an interceptor chain, so that there is no need to use a dynamic proxy to implement the interceptor, If you agree, I can submit a pr

[harawata]

@brucelwl ,

Just curious, how many plugins do you use in your project?
I don't use plugins very often and have never had any performance issue myself.

Anyway, I need some time to understand the current design before answering your question.
It would be easier for us to understand your proposals if you submit the change as a PR indeed, but please do understand there is no guarantee that your PRs will be merged.

[brucelwl]

@harawata The performance problem is only relative. I just want to have an optimized solution

[wushp]

@harawata ありがとう！

[harawata]

I finally took some time to look into it and my conclusion is : the number of dynamic proxies does not have significant impact on performance.

Here is the JMH project I used.
https://github.com/harawata/mybatis-gh1993-jmh

There are three methods.

• zeroInterceptor
• fiveInterceptors
• fiveFakes

zeroInterceptor is a baseline, so to speak.
There is no Interceptor instance in the InterceptorChain, so there will be no dynamic proxy, obviously.
On my machine, the throughput was about 200-230.

fiveInterceptors adds five Interceptor instances to the InterceptorChain, so there will be five dynamic proxies created for every update() invocation.
The throughput was about 1.4-1.7, so it's much slower than zeroInterceptor.

fiveFakes also adds five Interceptor instances to the InterceptorChain, however, their @Signature references unsupported method, so no dynamic proxies will be created.
If the number of dynamic proxies affects performance, as you suggested, the throughput of fiveFakes should be much higher than fiveInterceptors, but the actual throughput is pretty much the same as the throughput of fiveInterceptors.

Benchmark	Mode	Cnt	Score	Error	Units
MyBenchmark.fiveFakes	thrpt	25	1.750	± 0.136	ops/s
MyBenchmark.fiveInterceptors	thrpt	25	1.617	± 0.127	ops/s
MyBenchmark.zeroInterceptor	thrpt	25	229.677	± 2.956	ops/s

• The difference between fiveInterceptors and fiveFakes (or the absence thereof) shows that the number of dynamic proxies does not affect the performance significantly.
• The difference between zeroInterceptor and fiveFakes shows that the bottleneck exists somewhere between InterceptorChain#pluginAll() and Plugin#wrap(). [1]

Please try the benchmark yourself and see if you reach the same conclusion.
And please let me know if there is any issue with my benchmark code/config (it's possible).

---

Just to be sure, I ran the same benchmarks against PR #2001 (master branch merged locally).
As you can see, there is no significant difference.

Benchmark	Mode	Cnt	Score	Error	Units
MyBenchmark.fiveFakes	thrpt	25	1.774	± 0.135	ops/s
MyBenchmark.fiveInterceptors	thrpt	25	1.655	± 0.132	ops/s
MyBenchmark.zeroInterceptor	thrpt	25	230.693	± 4.927	ops/s

---

I also ran the benchmarks against PR #3396 .
The throughput increased a little bit, but not much.

Benchmark	Mode	Cnt	Score	Error	Units
MyBenchmark.fiveFakes	thrpt	25	3.944	± 0.310	ops/s
MyBenchmark.fiveInterceptors	thrpt	12	3.426	± 0.435	ops/s
MyBenchmark.zeroInterceptor	thrpt	25	232.382	± 3.758	ops/s

---

[1] This is just an experiment, but after applying the following patch to the master, the throughput of fiveFakes was about 10 which is better, but not as good as I thought it would be.

Benchmark	Mode	Cnt	Score	Error	Units
MyBenchmark.fiveFakes	thrpt	25	10.229	± 1.007	ops/s

diff --git a/src/main/java/org/apache/ibatis/plugin/Plugin.java b/src/main/java/org/apache/ibatis/plugin/Plugin.java
index feae8724c8..8da6166095 100644
--- a/src/main/java/org/apache/ibatis/plugin/Plugin.java
+++ b/src/main/java/org/apache/ibatis/plugin/Plugin.java
@@ -41,12 +41,6 @@ public class Plugin implements InvocationHandler {
   }
 
   public static Object wrap(Object target, Interceptor interceptor) {
-    Map<Class<?>, Set<Method>> signatureMap = getSignatureMap(interceptor);
-    Class<?> type = target.getClass();
-    Class<?>[] interfaces = getAllInterfaces(type, signatureMap);
-    if (interfaces.length > 0) {
-      return Proxy.newProxyInstance(type.getClassLoader(), interfaces, new Plugin(target, interceptor, signatureMap));
-    }
     return target;
   }
 

[brucelwl]

@harawata Although this optimization does not approach the performance of zero interceptors, even with a small amount of optimization, it is still a worthwhile thing to do, and it has not made things worse than before
Due to the long time, my original PR has been discarded. If you are willing to merge this optimization, I can submit a new PR and resubmit the optimization proposed by 3396 together