Pulled varargs test out of the loop to avoid checking on each parameter

The Method#isVarargs method is now referenced statically to avoid costs of looking it up on each native method call with at least one parameter. This also can save a lot of memory, since on every Class#getMethod() call a new Method object is returned. Added null check uses the double-checked locking to speed up subsequent calls to the same function. Made Function$Handler$FunctionInfo immutable class to make double check locking work correctly. Added description of changes to CHANGES.md
java-native-access · Jul 24, 2014 · 7f2c689 · 7f2c689
1 parent 80d4c13
commit 7f2c689
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 34 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -18,6 +18,7 @@ Features
 * [#286](https://github.com/twall/jna/pull/286): Added in com.sun.jna.platform.win32.Kernel32: CreateRemoteThread, WritePocessMemory and ReadProcessMemory - [@sstokic-tgm](https://github.com/sstokic-tgm).
 * [#350](https://github.com/twall/jna/pull/350): Added `jnacontrib.x11.api.X.Window.getSubwindows` - [@rm5248](https://github.com/rm5248).
 * Improved `contrib/msoffice` sample - [@wolftobias](https://github.com/wolftobias).
+* [#352](https://github.com/twall/jna/pull/352): Performance improvements due to reduced locking in `com.sun.jna.Library$Handler` and fewer vararg checks in `com.sun.jna.Function` - [@Boereck](https://github.com/Boereck).
 
 Bug Fixes
 ---------

diff --git a/src/com/sun/jna/Function.java b/src/com/sun/jna/Function.java
@@ -288,9 +288,10 @@ public Object invoke(Class returnType, Object[] inArgs, Map options) {
         Method invokingMethod = (Method)options.get(OPTION_INVOKING_METHOD);
         Class[] paramTypes = invokingMethod != null ? invokingMethod.getParameterTypes() : null;
         boolean allowObjects = Boolean.TRUE.equals(options.get(Library.OPTION_ALLOW_OBJECTS));
+        boolean isVarArgs = args.length > 0 && invokingMethod != null ? isVarArgs(invokingMethod) : false;
         for (int i=0; i < args.length; i++) {
             Class paramType = invokingMethod != null
-                ? (isVarArgs(invokingMethod) && i >= paramTypes.length-1
+                ? (isVarArgs && i >= paramTypes.length-1
                    ? paramTypes[paramTypes.length-1].getComponentType()
                    : paramTypes[i])
                 : null;
@@ -776,21 +777,34 @@ static Object[] concatenateVarArgs(Object[] inArgs) {
         return inArgs;
     }
 
-    /** Varargs are only supported on 1.5+. */
-    static boolean isVarArgs(Method m) {
+    /**
+     * Reference to Method.isVarArgs
+     */
+    private static Method isVarArgsMethod = getIsVarArgsMethod();
+
+    /**
+     * If possible returns the Method.isVarArgs method via reflection
+     * @return Method.isVarArgs method
+     */
+    private static Method getIsVarArgsMethod() {
         try {
-            Method v = m.getClass().getMethod("isVarArgs", new Class[0]);
-            return Boolean.TRUE.equals(v.invoke(m, new Object[0]));
-        }
-        catch (SecurityException e) {
+            return Method.class.getMethod("isVarArgs", new Class[0]);
+        } catch (SecurityException e) {
+            return null;
+        } catch (NoSuchMethodException e) {
+            return null;
         }
-        catch (NoSuchMethodException e) {
-        }
-        catch (IllegalArgumentException e) {
-        }
-        catch (IllegalAccessException e) {
-        }
-        catch (InvocationTargetException e) {
+    }
+
+    /** Varargs are only supported on 1.5+. */
+    static boolean isVarArgs(Method m) {
+        if(isVarArgsMethod != null) {
+            try {
+                return Boolean.TRUE.equals(isVarArgsMethod.invoke(m, new Object[0]));
+            } catch (IllegalArgumentException e) {
+            } catch (IllegalAccessException e) {
+            } catch (InvocationTargetException e) {
+            }
         }
         return false;
     }

diff --git a/src/com/sun/jna/Library.java b/src/com/sun/jna/Library.java
@@ -160,11 +160,25 @@ public Class getInterfaceClass() {
             return interfaceClass;
         }
 
-        private static class FunctionInfo {
-            InvocationHandler handler;
-            Function function;
-            boolean isVarArgs;
-            Map options;
+        /**
+         * FunctionInfo has to be immutable to to make the object visible 
+         * to other threads fully initialized. This is a prerequisite for
+         * using the class in the double checked locking scenario of {@link Handler#invoke(Object, Method, Object[])}
+         */
+        private static final class FunctionInfo {
+
+            FunctionInfo(InvocationHandler handler, Function function, boolean isVarArgs, Map options) {
+                super();
+                this.handler = handler;
+                this.function = function;
+                this.isVarArgs = isVarArgs;
+                this.options = options;
+            }
+
+            final InvocationHandler handler;
+            final Function function;
+            final boolean isVarArgs;
+            final Map options;
         }
 
         public Object invoke(Object proxy, Method method, Object[] inArgs)
@@ -185,22 +199,28 @@ else if (OBJECT_EQUALS.equals(method)) {
                 return Boolean.FALSE;
             }
 
-            FunctionInfo f = null;
-            synchronized(functions) {
-                f = (FunctionInfo)functions.get(method);
-                if (f == null) {
-                    f = new FunctionInfo();
-                    f.isVarArgs = Function.isVarArgs(method);
-                    if (invocationMapper != null) {
-                        f.handler = invocationMapper.getInvocationHandler(nativeLibrary, method);
-                    }
-                    if (f.handler == null) {
-                        // Find the function to invoke
-                        f.function = nativeLibrary.getFunction(method.getName(), method);
-                        f.options = new HashMap(this.options);
-                        f.options.put(Function.OPTION_INVOKING_METHOD, method);
+            // Using the double-checked locking pattern to speed up function calls
+            FunctionInfo f = (FunctionInfo)functions.get(method);
+            if(f == null) {
+                synchronized(functions) {
+                    f = (FunctionInfo)functions.get(method);
+                    if (f == null) {
+                        boolean isVarArgs = Function.isVarArgs(method);
+                        InvocationHandler handler = null;
+                        if (invocationMapper != null) {
+                            handler = invocationMapper.getInvocationHandler(nativeLibrary, method);
+                        }
+                        Function function = null;
+                        Map options = null;
+                        if (handler == null) {
+                            // Find the function to invoke
+                            function = nativeLibrary.getFunction(method.getName(), method);
+                            options = new HashMap(this.options);
+                            options.put(Function.OPTION_INVOKING_METHOD, method);
+                        }
+                        f = new FunctionInfo(handler, function, isVarArgs, options);
+                        functions.put(method, f);
                     }
-                    functions.put(method, f);
                 }
             }
             if (f.isVarArgs) {