You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Henri Biestro (Jira)" <ji...@apache.org> on 2021/06/07 13:15:06 UTC

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

     [ https://issues.apache.org/jira/browse/JEXL-343?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henri Biestro closed JEXL-343.
------------------------------

> MethodExecutor ctor has strange null check before calling MethodKey.isVarArgs()
> -------------------------------------------------------------------------------
>
>                 Key: JEXL-343
>                 URL: https://issues.apache.org/jira/browse/JEXL-343
>             Project: Commons JEXL
>          Issue Type: Improvement
>    Affects Versions: 3.1
>         Environment: jdk-11.0.5_10-hotspot, GNU/Linux
>            Reporter: David Costanzo
>            Assignee: Henri Biestro
>            Priority: Minor
>             Fix For: 3.2
>
>
> 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()|https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/reflect/Method.html#getTypeParameters()]  should never return null.
>  
> {noformat}
> 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();
>         }
>     ...
>     }
> {noformat}
>  
> 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: 
> {noformat}
> 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[]){noformat}
>  
> 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}}.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)