Add c-call, c-return tracing to handle-based methods.

This mechanism is slightly heavier than the generated invokers mostly because it ends up checking whether there's events twice. However, this appears to be wrong logic; MRI reports the return of `set_trace_func` for example, and we don't. This commit also makes the following improvements: * Pass all DynamicMethod.call args into the handle chain; this is necessary for tracing and helpful for reporting arity errors with the actually-invoked name. * Don't use an iterator when iterating over event hooks, to avoid allocation on that hot path. This should help all event hooks to be cheaper.
jruby · Jul 5, 2015 · 9425fcc · 9425fcc
1 parent ba22379
commit 9425fcc
Showing 4 changed files with 52 additions and 32 deletions.
diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -3109,7 +3109,9 @@ public void setTraceFunction(RubyProc traceFunction) {
 
     public void callEventHooks(ThreadContext context, RubyEvent event, String file, int line, String name, IRubyObject type) {
         if (context.isEventHooksEnabled()) {
-            for (EventHook eventHook : eventHooks) {
+            for (int i = 0; i < eventHooks.length; i++) {
+                EventHook eventHook = eventHooks[i];
+
                 if (eventHook.isInterestedInEvent(event)) {
                     eventHook.event(context, event, file, line, name, type);
                 }

diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/HandleMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/HandleMethod.java
@@ -123,7 +123,7 @@ private void ensureTargets() {
     public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
         ensureTargets();
         try {
-            return (IRubyObject) target4.invokeExact(context, self, args, block);
+            return (IRubyObject) target4.invokeExact(context, self, clazz, name, args, block);
         } catch (Throwable t) {
             Helpers.throwException(t);
             return null;
@@ -137,7 +137,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
             return call(context, self, clazz, name, IRubyObject.NULL_ARRAY, block);
         }
         try {
-            return (IRubyObject) target0.invokeExact(context, self, block);
+            return (IRubyObject) target0.invokeExact(context, self, clazz, name, block);
         } catch (Throwable t) {
             Helpers.throwException(t);
             return null;
@@ -151,7 +151,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
             return call(context, self, clazz, name, new IRubyObject[]{arg0}, block);
         }
         try {
-            return (IRubyObject) target1.invokeExact(context, self, arg0, block);
+            return (IRubyObject) target1.invokeExact(context, self, clazz, name, arg0, block);
         } catch (Throwable t) {
             Helpers.throwException(t);
             return null;
@@ -165,7 +165,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
             return call(context, self, clazz, name, new IRubyObject[]{arg0, arg1}, block);
         }
         try {
-            return (IRubyObject) target2.invokeExact(context, self, arg0, arg1, block);
+            return (IRubyObject) target2.invokeExact(context, self, clazz, name, arg0, arg1, block);
         } catch (Throwable t) {
             Helpers.throwException(t);
             return null;
@@ -179,7 +179,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
             return call(context, self, clazz, name, new IRubyObject[]{arg0, arg1, arg2}, block);
         }
         try {
-            return (IRubyObject) target3.invokeExact(context, self, arg0, arg1, arg2, block);
+            return (IRubyObject) target3.invokeExact(context, self, clazz, name, arg0, arg1, arg2, block);
         } catch (Throwable t) {
             Helpers.throwException(t);
             return null;

diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/InvokeDynamicMethodFactory.java b/core/src/main/java/org/jruby/internal/runtime/methods/InvokeDynamicMethodFactory.java
@@ -34,6 +34,7 @@
 import org.jruby.RubyModule;
 import org.jruby.runtime.Arity;
 import org.jruby.runtime.Block;
+import org.jruby.runtime.RubyEvent;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
 import java.lang.invoke.MethodHandle;
@@ -47,6 +48,7 @@
 import org.jruby.Ruby;
 import org.jruby.anno.JavaMethodDescriptor;
 import org.jruby.runtime.invokedynamic.InvocationLinker;
+import org.jruby.util.cli.Options;
 import org.jruby.util.log.Logger;
 import org.jruby.util.log.LoggerFactory;
 
@@ -192,6 +194,10 @@ private MethodHandle[] buildAnnotatedMethodHandles(Ruby runtime, List<JavaMethod
             }
 
             targetBinder = SmartBinder.from(baseSignature);
+
+            // unused by Java-based methods
+            targetBinder = targetBinder
+                    .exclude("class", "name");
 
             MethodHandle returnFilter = null;
             boolean castReturn = false;
@@ -280,13 +286,13 @@ private MethodHandle[] buildAnnotatedMethodHandles(Ruby runtime, List<JavaMethod
                 if (targets[i] == null) continue; // will never be retrieved; arity check will error first
 
                 if (i == 0) {
-                    varargsTargets[i] = MethodHandles.dropArguments(targets[i], 2, IRubyObject[].class);
+                    varargsTargets[i] = MethodHandles.dropArguments(targets[i], 4, IRubyObject[].class);
                 } else {
                     varargsTargets[i] = SmartBinder
                             .from(VARIABLE_ARITY_SIGNATURE)
-                            .permute("context", "self", "block", "args")
+                            .permute("context", "self", "class", "name", "block", "args")
                             .spread("arg", i)
-                            .permute("context", "self", "arg*", "block")
+                            .permute("context", "self", "class", "name", "arg*", "block")
                             .invoke(targets[i]).handle();
                 }
             }
@@ -309,18 +315,44 @@ private MethodHandle[] buildAnnotatedMethodHandles(Ruby runtime, List<JavaMethod
             targets[4] = variableCall.handle();
         }
 
+        // Add arity check of varargs path
         targets[4] = SmartBinder
                 .from(VARIABLE_ARITY_SIGNATURE)
                 .foldVoid(SmartBinder
                         .from(VARIABLE_ARITY_SIGNATURE.changeReturn(int.class))
-                        .permute("context", "args")
-                        .append(arrayOf("min", "max", "name"), arrayOf(int.class, int.class, String.class), min, max, rubyName)
+                        .permute("context", "name", "args")
+                        .append(arrayOf("min", "max"), arrayOf(int.class, int.class), min, max)
                         .invokeStaticQuiet(LOOKUP, Arity.class, "checkArgumentCount")
                         .handle())
                 .invoke(targets[4])
                 .handle();
 
-        // TODO: tracing
+        // Add tracing of "C" call/return
+        if (Options.DEBUG_FULLTRACE.load()) {
+            for (int i = 0; i < targets.length; i++) {
+                MethodHandle target = targets[i];
+
+                if (target == null) continue;
+
+                MethodHandle traceCall = Binder
+                        .from(target.type().changeReturnType(void.class))
+                        .permute(0, 3, 2) // context, name, class
+                        .insert(1, RubyEvent.C_CALL)
+                        .invokeVirtualQuiet(LOOKUP, "trace");
+
+                MethodHandle traceReturn = Binder
+                        .from(target.type().changeReturnType(void.class))
+                        .permute(0, 3, 2) // context, name, class
+                        .insert(1, RubyEvent.C_RETURN)
+                        .invokeVirtualQuiet(LOOKUP, "trace");
+
+                targets[i] = Binder
+                        .from(target.type())
+                        .foldVoid(traceCall)
+                        .tryFinally(traceReturn)
+                        .invoke(target);
+            }
+        }
 
         return targets;
     }
@@ -340,28 +372,20 @@ public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMeth
             .returning(IRubyObject.class)
             .appendArg("context", ThreadContext.class)
             .appendArg("self", IRubyObject.class)
+            .appendArg("class", RubyModule.class)
+            .appendArg("name", String.class)
             .appendArg("args", IRubyObject[].class)
             .appendArg("block", Block.class);
 
-    public static final Signature ARITY_CHECK_FOLD = Signature
-            .returning(void.class)
-            .appendArg("context", ThreadContext.class)
-            .appendArg("args", IRubyObject[].class);
-
-    public static final Signature ARITY_CHECK_SIGNATURE = Signature
-            .returning(int.class)
-            .appendArg("context", ThreadContext.class)
-            .appendArg("args", IRubyObject[].class)
-            .appendArg("min", int.class)
-            .appendArg("max", int.class);
-
     public static final Signature[] SPECIFIC_ARITY_SIGNATURES;
     static {
             Signature[] specifics = new Signature[4];
             Signature specific = Signature
                     .returning(IRubyObject.class)
                     .appendArg("context", ThreadContext.class)
-                    .appendArg("self", IRubyObject.class);
+                    .appendArg("self", IRubyObject.class)
+                    .appendArg("class", RubyModule.class)
+                    .appendArg("name", String.class);
 
             specifics[0] = specific.appendArg("block", Block.class);
 
@@ -383,10 +407,4 @@ public DynamicMethod getAnnotatedMethod(RubyModule implementationClass, JavaMeth
                     .permute("context", "self", "arg*", "block");
         }
     }
-
-    static final MethodHandle ARITY_ERROR_HANDLE = SmartBinder
-            .from(VARIABLE_ARITY_SIGNATURE.appendArgs(arrayOf("min", "max"), int.class, int.class))
-            .filterReturn(Binder.from(IRubyObject.class, int.class).drop(0).constant(null))
-            .permute(ARITY_CHECK_SIGNATURE)
-            .invokeStaticQuiet(LOOKUP, Arity.class, "checkArgumentCount").handle();
 }
diff --git a/core/src/main/java/org/jruby/runtime/Arity.java b/core/src/main/java/org/jruby/runtime/Arity.java
@@ -202,7 +202,7 @@ public static int checkArgumentCount(ThreadContext context, IRubyObject[] args,
         return checkArgumentCount(context, args.length, min, max);
     }
 
-    public static int checkArgumentCount(ThreadContext context, IRubyObject[] args, int min, int max, String name) {
+    public static int checkArgumentCount(ThreadContext context, String name, IRubyObject[] args, int min, int max) {
         return checkArgumentCount(context.runtime, name, args.length, min, max);
     }