Performance improvements to event tracing.

* Avoid CopyOnWriteArrayList iterator construction (use a simple array and iterate over that) for event hook list * Use int[] to represent line number hit counts. * Fix int[] size checking to avoid excessive allocation. * Fixes to trace correctly in compiler without Backtrace.
jruby · Oct 7, 2014 · 8c3c3f0 · 8c3c3f0
1 parent 87bf291
commit 8c3c3f0
Showing 9 changed files with 106 additions and 79 deletions.
diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -3018,18 +3018,34 @@ public boolean isInterestedInEvent(RubyEvent event) {
 
     private final CallTraceFuncHook callTraceFuncHook = new CallTraceFuncHook();
 
-    public void addEventHook(EventHook hook) {
+    public synchronized void addEventHook(EventHook hook) {
         if (!RubyInstanceConfig.FULL_TRACE_ENABLED) {
             // without full tracing, many events will not fire
             getWarnings().warn("tracing (e.g. set_trace_func) will not capture all events without --debug flag");
         }
-        eventHooks.add(hook);
+
+        EventHook[] hooks = eventHooks;
+        EventHook[] newHooks = new EventHook[hooks.length + 1];
+        newHooks[hooks.length] = hook;
+        eventHooks = newHooks;
         hasEventHooks = true;
     }
 
-    public void removeEventHook(EventHook hook) {
-        eventHooks.remove(hook);
-        hasEventHooks = !eventHooks.isEmpty();
+    public synchronized void removeEventHook(EventHook hook) {
+        EventHook[] hooks = eventHooks;
+        if (hooks.length == 0) return;
+        EventHook[] newHooks = new EventHook[hooks.length - 1];
+        boolean found = false;
+        for (int i = 0, j = 0; i < hooks.length; i++) {
+            if (!found && hooks[i] == hook && !found) { // exclude first found
+                found = true;
+                continue;
+            }
+            newHooks[j] = hooks[i];
+            j++;
+        }
+        eventHooks = newHooks;
+        hasEventHooks = newHooks.length > 0;
     }
 
     public void setTraceFunction(RubyProc traceFunction) {
@@ -4658,8 +4674,9 @@ public CallbackFactory callbackFactory(Class<?> type) {
 
     private final RubySymbol.SymbolTable symbolTable = new RubySymbol.SymbolTable(this);
 
-    private final List<EventHook> eventHooks = new CopyOnWriteArrayList<EventHook>();
-    private boolean hasEventHooks;  
+    private static final EventHook[] EMPTY_HOOKS = new EventHook[0];
+    private volatile EventHook[] eventHooks = new EventHook[0];
+    private boolean hasEventHooks;
     private boolean globalAbortOnExceptionEnabled = false;
     private boolean doNotReverseLookupEnabled = false;
     private volatile boolean objectSpaceEnabled;

diff --git a/core/src/main/java/org/jruby/compiler/ASTCompiler.java b/core/src/main/java/org/jruby/compiler/ASTCompiler.java
@@ -43,6 +43,7 @@
 import org.jruby.RubyString;
 import org.jruby.ast.*;
 import org.jruby.exceptions.JumpException;
+import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.runtime.Helpers;
 import org.jruby.parser.StaticScope;
 import org.jruby.runtime.Arity;
@@ -902,7 +903,7 @@ public void call(BodyCompiler context) {
             final WhenNode whenNode = (WhenNode)node;
             CompilerCallback body = new CompilerCallback() {
                 public void call(BodyCompiler context) {
-                    if (RubyInstanceConfig.FULL_TRACE_ENABLED) context.traceLine();
+                    if (RubyInstanceConfig.FULL_TRACE_ENABLED) context.traceLine(whenNode.getPosition());
                     compile(whenNode.getBodyNode(), context, expr);
                 }
             };
@@ -1001,6 +1002,8 @@ public void call(BodyCompiler context) {
             superCallback = null;
         }
 
+        ISourcePosition[] lastPosition = new ISourcePosition[1];
+
         CompilerCallback bodyCallback = new CompilerCallback() {
 
                     public void call(BodyCompiler context) {
@@ -1040,7 +1043,7 @@ public void call(BodyCompiler context) {
         ASTInspector inspector = new ASTInspector();
         inspector.inspect(classNode.getBodyNode());
 
-        context.defineClass(classNode.getCPath().getName(), classNode.getScope(), superCallback, pathCallback, bodyCallback, null, inspector);
+        context.defineClass(classNode.getCPath().getName(), classNode.getScope(), superCallback, pathCallback, bodyCallback, null, inspector, classNode.getPosition());
         // TODO: don't require pop
         if (!expr) context.consumeCurrentValue();
     }
@@ -1072,7 +1075,7 @@ public void call(BodyCompiler context) {
         ASTInspector inspector = new ASTInspector();
         inspector.inspect(sclassNode.getBodyNode());
 
-        context.defineClass("SCLASS", sclassNode.getScope(), null, null, bodyCallback, receiverCallback, inspector);
+        context.defineClass("SCLASS", sclassNode.getScope(), null, null, bodyCallback, receiverCallback, inspector, sclassNode.getPosition());
         // TODO: don't require pop
         if (!expr) context.consumeCurrentValue();
     }
@@ -2714,7 +2717,7 @@ public void call(BodyCompiler context) {
         ASTInspector inspector = new ASTInspector();
         inspector.inspect(moduleNode.getBodyNode());
 
-        context.defineModule(moduleNode.getCPath().getName(), moduleNode.getScope(), pathCallback, bodyCallback, inspector);
+        context.defineModule(moduleNode.getCPath().getName(), moduleNode.getScope(), pathCallback, bodyCallback, inspector, moduleNode.getPosition());
         // TODO: don't require pop
         if (!expr) context.consumeCurrentValue();
     }
@@ -2864,10 +2867,10 @@ public void compileNewline(Node node, BodyCompiler context, boolean expr) {
 
         context.setLinePosition(node.getPosition());
 
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) context.traceLine();
-
         NewlineNode newlineNode = (NewlineNode) node;
 
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) context.traceLine(newlineNode.getPosition());
+
         compile(newlineNode.getNextNode(), context, expr);
     }
 

diff --git a/core/src/main/java/org/jruby/compiler/BodyCompiler.java b/core/src/main/java/org/jruby/compiler/BodyCompiler.java
@@ -235,8 +235,6 @@ public interface BodyCompiler {
     public void createNewLiteralHash(Object elements, ArrayCallback callback, int keyCount);
 
     /**
-    * @see createNewHash
-    *
     * Create new hash running in ruby 1.9 compat version.
     */
     public void createNewHash19(Object elements, ArrayCallback callback, int keyCount);
@@ -670,8 +668,8 @@ public void defineNewMethod(String name, int methodArity, StaticScope scope,
     public void toJavaString();
     public void aliasGlobal(String newName, String oldName);
     public void undefMethod(CompilerCallback nameArg);
-    public void defineClass(String name, StaticScope staticScope, CompilerCallback superCallback, CompilerCallback pathCallback, CompilerCallback bodyCallback, CompilerCallback receiverCallback, ASTInspector inspector);
-    public void defineModule(String name, StaticScope staticScope, CompilerCallback pathCallback, CompilerCallback bodyCallback, ASTInspector inspector);
+    public void defineClass(String name, StaticScope staticScope, CompilerCallback superCallback, CompilerCallback pathCallback, CompilerCallback bodyCallback, CompilerCallback receiverCallback, ASTInspector inspector, ISourcePosition startPosition);
+    public void defineModule(String name, StaticScope staticScope, CompilerCallback pathCallback, CompilerCallback bodyCallback, ASTInspector inspector, ISourcePosition startPosition);
     public void unwrapPassedBlock();
     public void performBackref(char type);
     public void callZSuper(CompilerCallback closure);
@@ -682,7 +680,6 @@ public void defineNewMethod(String name, int methodArity, StaticScope scope,
     public void loadStandardError();
     public void unwrapRaiseException();
     public void loadException();
-    public void setFilePosition(ISourcePosition position);
     public void setLinePosition(ISourcePosition position);
     public void checkWhenWithSplat();
     public void createNewEndBlock(CompilerCallback body);
@@ -730,9 +727,9 @@ public void compileSequencedConditional(
 
     public void raiseTypeError(String string);
 
-    public void traceLine();
-    public void traceClass();
-    public void traceEnd();
+    public void traceLine(ISourcePosition position);
+    public void traceClass(ISourcePosition position);
+    public void traceEnd(int line);
 
     public String getNativeMethodName();
 

diff --git a/core/src/main/java/org/jruby/compiler/impl/BaseBodyCompiler.java b/core/src/main/java/org/jruby/compiler/impl/BaseBodyCompiler.java
@@ -2399,7 +2399,8 @@ public void defineClass(
             final CompilerCallback pathCallback,
             final CompilerCallback bodyCallback,
             final CompilerCallback receiverCallback,
-            final ASTInspector inspector) {
+            final ASTInspector inspector,
+            final ISourcePosition startPosition) {
         String classMethodName = null;
         String rubyName;
         if (receiverCallback == null) {
@@ -2476,23 +2477,23 @@ public void defineClass(
 
         classBody.beginMethod(null, staticScope);
 
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceClass();
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceClass(startPosition);
 
         classBody.method.label(start);
 
         bodyCallback.call(classBody);
 
         classBody.method.label(end);
         // finally with no exception
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd();
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd(lastPositionLine);
         classBody.loadThreadContext();
         classBody.invokeThreadContext("postCompiledClass", sig(Void.TYPE, params()));
 
         classBody.method.go_to(noException);
 
         classBody.method.label(after);
         // finally with exception
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd();
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd(lastPositionLine);
         classBody.loadThreadContext();
         classBody.invokeThreadContext("postCompiledClass", sig(Void.TYPE, params()));
         classBody.method.athrow();
@@ -2502,7 +2503,12 @@ public void defineClass(
         classBody.endBody();
     }
 
-    public void defineModule(final String name, final StaticScope staticScope, final CompilerCallback pathCallback, final CompilerCallback bodyCallback, final ASTInspector inspector) {
+    public void defineModule(final String name,
+                             final StaticScope staticScope,
+                             final CompilerCallback pathCallback,
+                             final CompilerCallback bodyCallback,
+                             final ASTInspector inspector,
+                             final ISourcePosition startPosition) {
         String mangledName = JavaNameMangler.mangleMethodName(name);
         String moduleMethodName = "module__" + script.getAndIncrementMethodIndex() + "$RUBY$" + mangledName;
 
@@ -2548,7 +2554,7 @@ public void defineModule(final String name, final StaticScope staticScope, final
 
         classBody.beginMethod(null, staticScope);
 
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceClass();
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceClass(startPosition);
 
         classBody.method.label(start);
 
@@ -2558,13 +2564,13 @@ public void defineModule(final String name, final StaticScope staticScope, final
         classBody.method.go_to(noException);
 
         classBody.method.label(after);
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd();
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd(lastPositionLine);
         classBody.loadThreadContext();
         classBody.invokeThreadContext("postCompiledClass", sig(Void.TYPE, params()));
         classBody.method.athrow();
 
         classBody.method.label(noException);
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd();
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) classBody.traceEnd(lastPositionLine);
         classBody.loadThreadContext();
         classBody.invokeThreadContext("postCompiledClass", sig(Void.TYPE, params()));
 
@@ -2658,26 +2664,8 @@ public void loadException() {
         method.aload(getExceptionIndex());
     }
 
-    public void setFilePosition(ISourcePosition position) {
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) {
-            loadThreadContext();
-            method.ldc(position.getFile());
-            invokeThreadContext("setFile", sig(void.class, params(String.class)));
-        }
-    }
-
     public void setLinePosition(ISourcePosition position) {
-        if (RubyInstanceConfig.FULL_TRACE_ENABLED) {
-            if (lastPositionLine == position.getStartLine()) {
-                // updating position for same line; skip
-                return;
-            } else {
-                lastPositionLine = position.getStartLine();
-                loadThreadContext();
-                method.pushInt(position.getStartLine());
-                method.invokestatic(script.getClassname(), "setPosition", sig(void.class, params(ThreadContext.class, int.class)));
-            }
-        }
+        lastPositionLine = position.getStartLine();
     }
 
     public void checkWhenWithSplat() {
@@ -2938,19 +2926,31 @@ public void call(BodyCompiler context) {
         getVariableCompiler().releaseTempLocal();
     }
 
-    public void traceLine() {
+    public void traceLine(ISourcePosition position) {
         loadThreadContext();
-        invokeUtilityMethod("traceLine", sig(void.class, ThreadContext.class));
+        // load filename since we share bytecode in some scenarios
+        loadThis();
+        method.getfield(getScriptCompiler().getClassname(), "filename", ci(String.class));
+        method.pushInt(position.getStartLine());
+        invokeUtilityMethod("traceLine", sig(void.class, ThreadContext.class, String.class, int.class));
     }
 
-    public void traceClass() {
+    public void traceClass(ISourcePosition position) {
         loadThreadContext();
-        invokeUtilityMethod("traceClass", sig(void.class, ThreadContext.class));
+        // load filename since we share bytecode in some scenarios
+        loadThis();
+        method.getfield(getScriptCompiler().getClassname(), "filename", ci(String.class));
+        method.pushInt(position.getStartLine());
+        invokeUtilityMethod("traceClass", sig(void.class, ThreadContext.class, String.class, int.class));
     }
 
-    public void traceEnd() {
+    public void traceEnd(int line) {
         loadThreadContext();
-        invokeUtilityMethod("traceEnd", sig(void.class, ThreadContext.class));
+        // load filename since we share bytecode in some scenarios
+        loadThis();
+        method.getfield(getScriptCompiler().getClassname(), "filename", ci(String.class));
+        method.pushInt(line);
+        invokeUtilityMethod("traceEnd", sig(void.class, ThreadContext.class, String.class, int.class));
     }
 
     public void preMultiAssign(int head, boolean args) {

diff --git a/core/src/main/java/org/jruby/ext/coverage/CoverageData.java b/core/src/main/java/org/jruby/ext/coverage/CoverageData.java
@@ -26,6 +26,7 @@
 
 package org.jruby.ext.coverage;
 
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import org.jruby.Ruby;
@@ -35,33 +36,33 @@
 import org.jruby.runtime.builtin.IRubyObject;
 
 public class CoverageData {
-    private volatile Map<String, Integer[]> coverage;
+    private volatile Map<String, int[]> coverage;
 
     public boolean isCoverageEnabled() {
         return coverage != null;
     }
 
     public synchronized void setCoverageEnabled(Ruby runtime, boolean enabled) {
         if (enabled) {
-            coverage = new HashMap<String, Integer[]>();
+            coverage = new HashMap<String, int[]>();
             runtime.addEventHook(COVERAGE_HOOK);
         } else {
             coverage = null;
         }
     }
 
-    public synchronized Map<String, Integer[]> resetCoverage(Ruby runtime) {
-        Map<String, Integer[]> coverage = this.coverage;
+    public synchronized Map<String, int[]> resetCoverage(Ruby runtime) {
+        Map<String, int[]> coverage = this.coverage;
         runtime.removeEventHook(COVERAGE_HOOK);
         this.coverage = null;
 
         return coverage;
     }
 
-    public synchronized Map<String, Integer[]> prepareCoverage(String filename, Integer[] lines) {
+    public synchronized Map<String, int[]> prepareCoverage(String filename, int[] lines) {
         assert lines != null;
 
-        Map<String, Integer[]> coverage = this.coverage;
+        Map<String, int[]> coverage = this.coverage;
 
         if (coverage != null) {
             coverage.put(filename, lines);
@@ -78,21 +79,22 @@ public synchronized void eventHandler(ThreadContext context, String eventName, S
             }
 
             // make sure we have a lines array of acceptable length for the given file
-            Integer[] lines = coverage.get(file);
+            int[] lines = coverage.get(file);
             if (lines == null) {
                 // loaded before coverage; skip
                 return;
-            } else if (lines.length <= line) {
+            } else if (lines.length < line) {
                 // can this happen? shouldn't all coverable lines be here already (from parse time)?
-                Integer[] newLines = new Integer[line];
+                int[] newLines = new int[line];
+                Arrays.fill(newLines, lines.length, line, -1); // mark unknown lines as -1
                 System.arraycopy(lines, 0, newLines, 0, lines.length);
                 lines = newLines;
                 coverage.put(file, lines);
             }
 
             // increment the line's count or set it to 1
-            Integer count = lines[line - 1];
-            if (count == null) {
+            int count = lines[line - 1];
+            if (count == -1) {
                 lines[line - 1] = 1;
             } else {
                 lines[line - 1] = count + 1;

diff --git a/core/src/main/java/org/jruby/ext/coverage/CoverageModule.java b/core/src/main/java/org/jruby/ext/coverage/CoverageModule.java
@@ -58,15 +58,15 @@ public static IRubyObject result(ThreadContext context, IRubyObject self) {
             throw runtime.newRuntimeError("coverage measurement is not enabled");
         }
 
-        Map<String, Integer[]> coverage = runtime.getCoverageData().resetCoverage(runtime);
+        Map<String, int[]> coverage = runtime.getCoverageData().resetCoverage(runtime);
 
         // populate a Ruby Hash with coverage data
         RubyHash covHash = RubyHash.newHash(runtime);
-        for (Map.Entry<String, Integer[]> entry : coverage.entrySet()) {
+        for (Map.Entry<String, int[]> entry : coverage.entrySet()) {
             RubyArray ary = RubyArray.newArray(runtime, entry.getValue().length);
             for (int i = 0; i < entry.getValue().length; i++) {
-                Integer integer = entry.getValue()[i];
-                ary.store(i, integer == null ? runtime.getNil() : runtime.newFixnum(integer));
+                int integer = entry.getValue()[i];
+                ary.store(i, integer == -1 ? context.nil : runtime.newFixnum(integer));
                 covHash.fastASetCheckString(runtime, RubyString.newString(runtime, entry.getKey()), ary);
             }
         }

diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/DefaultMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/DefaultMethod.java
@@ -74,9 +74,14 @@ private static class DynamicMethodBox {
     public DefaultMethod(RubyModule implementationClass, StaticScope staticScope, Node body,
             String name, ArgsNode argsNode, Visibility visibility, ISourcePosition position) {
         super(implementationClass, visibility, CallConfiguration.FrameFullScopeFull, name);
-        this.interpretedMethod = DynamicMethodFactory.newInterpretedMethod(
-                implementationClass.getRuntime(), implementationClass, staticScope,
-                body, name, argsNode, visibility, position);
+        if (RubyInstanceConfig.FULL_TRACE_ENABLED) {
+            this.interpretedMethod = new TraceableInterpretedMethod(implementationClass, staticScope, body, name, argsNode,
+                    visibility, position);
+        } else {
+            this.interpretedMethod = DynamicMethodFactory.newInterpretedMethod(
+                    implementationClass.getRuntime(), implementationClass, staticScope,
+                    body, name, argsNode, visibility, position);
+        }
         this.interpretedMethod.serialNumber = this.serialNumber;
         this.box.actualMethod = interpretedMethod;
         this.argsNode = argsNode;

diff --git a/core/src/main/java/org/jruby/parser/ParserConfiguration.java b/core/src/main/java/org/jruby/parser/ParserConfiguration.java
@@ -41,6 +41,8 @@
 import org.jruby.util.ByteList;
 import org.jruby.util.KCode;
 
+import java.util.Arrays;
+
 public class ParserConfiguration {
     private DynamicScope existingScope = null;
     private boolean asBlock = false;
@@ -62,9 +64,9 @@ public class ParserConfiguration {
     private Encoding defaultEncoding;
     private Ruby runtime;
 
-    private Integer[] coverage = EMPTY_COVERAGE;
+    private int[] coverage = EMPTY_COVERAGE;
 
-    private static final Integer[] EMPTY_COVERAGE = new Integer[0];
+    private static final int[] EMPTY_COVERAGE = new int[0];
 
     public ParserConfiguration(Ruby runtime, int lineNumber, boolean inlineSource,
             CompatVersion version) {
@@ -231,9 +233,10 @@ public void coverLine(int i) {
 
         if (runtime.getCoverageData().isCoverageEnabled()) {
             if (coverage == null) {
-                coverage = new Integer[i + 1];
+                coverage = new int[i + 1];
             } else if (coverage.length <= i) {
-                Integer[] newCoverage = new Integer[i + 1];
+                int[] newCoverage = new int[i + 1];
+                Arrays.fill(newCoverage, -1);
                 System.arraycopy(coverage, 0, newCoverage, 0, coverage.length);
                 coverage = newCoverage;
             }
@@ -246,7 +249,7 @@ public void coverLine(int i) {
     /**
      * Get the coverage array, indicating all coverable lines
      */
-    public Integer[] getCoverage() {
+    public int[] getCoverage() {
         return coverage;
     }
 }
diff --git a/core/src/main/java/org/jruby/runtime/Helpers.java b/core/src/main/java/org/jruby/runtime/Helpers.java
@@ -2439,10 +2439,10 @@ public static RubyBoolean rbEql(ThreadContext context, IRubyObject a, IRubyObjec
         return runtime.newBoolean(res.isTrue());
     }
 
-    public static void traceLine(ThreadContext context) {
+    public static void traceLine(ThreadContext context, String file, int line) {
         String name = context.getFrameName();
         RubyModule type = context.getFrameKlazz();
-        context.runtime.callEventHooks(context, RubyEvent.LINE, context.getFile(), context.getLine(), name, type);
+        context.runtime.callEventHooks(context, RubyEvent.LINE, file, line, name, type);
     }
 
     public static void traceClass(ThreadContext context) {