[JEXL-343] MethodExecutor ctor has strange null check before calling MethodKey.isVarArgs()

• Type: Improvement
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 3.1
• Fix Version/s: 3.2

Environment

jdk-11.0.5_10-hotspot, GNU/Linux

• Assignee: Henri Biestro
• Reporter: David Costanzo
• Created: 01/Feb/21 19:28
• Updated: 07/Jun/21 13:14
• Resolved: 02/Feb/21 20:34

Description

When inspecting slow performance of my JEXL-enabled application, I came across a strange line in MethodExecutor's constructor that I think is a bug. Specifically the formal != null is testing a condition that should never happen because Method.getTypeParameters()  should never return null.

 

private MethodExecutor(Class<?> c, java.lang.reflect.Method m, MethodKey k) {
    ...
        Class<?>[] formal = method.getParameterTypes();
        // if the last parameter is an array, the method is considered as vararg
        if (formal != null && MethodKey.isVarArgs(method)) { // <===== THIS LINE
            vastart = formal.length - 1;
            vaclass = formal[vastart].getComponentType();
        }
    ...
    }

 

I think formal != null is supposed to be formal.length != 0.

 

Impact:

Although this change may look trivial, my application completes in 25% less time with it (one benchmark goes from 9:49 to 7:01).

 

For reference, this is the top of the call stack that takes 28% of the execution time: 

java.lang.Class.getMethod(String, Class[])
org.apache.commons.jexl3.internal.introspection.MethodKey.isVarArgs (java.lang.reflect.Method)
org.apache.commons.jexl3.internal.introspection.MethodExecutor.<init>(Class, java.lang.reflect.Method, org.apache.commons.jexl3.internal.introspection.MethodKey)
org.apache.commons.jexl3.internal.introspection.MethodExecutor.discover(org.apache.commons.jexl3.internal.introspection.Introspector, Object, String, Object[])
org.apache.commons.jexl3.internal.introspection.Uberspect.getMethod(Object, String, Object[])

 

I looked for a sanctioned way to apply the "formal.length != 0" change using the provided hooks, but I couldn't find a good one. The code I want to change is too deep within the "internal" package. My bad way of changing this code is to use a custom Uberspect which calls a custom copy of MethodExecutor which extends a custom copy AbstractExecutor.

 

Issue Links

• is fixed by: JEXL-281 MethodExecutor incorrectly tests for empty parameters list