From 1f56abb341d49c29ea4d7a4f79fa8cf7c6950bbd Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Mon, 6 Oct 2014 22:54:33 -0500 Subject: [PATCH] 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. --- core/src/main/java/org/jruby/Ruby.java | 31 +++++++-- .../java/org/jruby/compiler/ASTCompiler.java | 15 +++-- .../java/org/jruby/compiler/BodyCompiler.java | 13 ++-- .../jruby/compiler/impl/BaseBodyCompiler.java | 66 +++++++++---------- .../org/jruby/ext/coverage/CoverageData.java | 24 +++---- .../jruby/ext/coverage/CoverageModule.java | 8 +-- .../runtime/methods/DefaultMethod.java | 11 +++- .../org/jruby/parser/ParserConfiguration.java | 13 ++-- .../main/java/org/jruby/runtime/Helpers.java | 4 +- 9 files changed, 106 insertions(+), 79 deletions(-) diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java index 0077792369d..1c3aae284ba 100644 --- 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 eventHooks = new CopyOnWriteArrayList(); - 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 index 79d9e59d6fb..59feb1ce748 100644 --- 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 index 8d6258cd116..cea7356c731 100644 --- 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 index e8b4f90408e..00c596ad452 100644 --- 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,7 +2477,7 @@ 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); @@ -2484,7 +2485,7 @@ public void defineClass( 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())); @@ -2492,7 +2493,7 @@ public void defineClass( 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 index bae6b1231d0..a4093f51351 100644 --- 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,7 +36,7 @@ import org.jruby.runtime.builtin.IRubyObject; public class CoverageData { - private volatile Map coverage; + private volatile Map coverage; public boolean isCoverageEnabled() { return coverage != null; @@ -43,25 +44,25 @@ public boolean isCoverageEnabled() { public synchronized void setCoverageEnabled(Ruby runtime, boolean enabled) { if (enabled) { - coverage = new HashMap(); + coverage = new HashMap(); runtime.addEventHook(COVERAGE_HOOK); } else { coverage = null; } } - public synchronized Map resetCoverage(Ruby runtime) { - Map coverage = this.coverage; + public synchronized Map resetCoverage(Ruby runtime) { + Map coverage = this.coverage; runtime.removeEventHook(COVERAGE_HOOK); this.coverage = null; return coverage; } - public synchronized Map prepareCoverage(String filename, Integer[] lines) { + public synchronized Map prepareCoverage(String filename, int[] lines) { assert lines != null; - Map coverage = this.coverage; + Map 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 index 3a95f91c9cc..ccb1f4d1181 100644 --- 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 coverage = runtime.getCoverageData().resetCoverage(runtime); + Map coverage = runtime.getCoverageData().resetCoverage(runtime); // populate a Ruby Hash with coverage data RubyHash covHash = RubyHash.newHash(runtime); - for (Map.Entry entry : coverage.entrySet()) { + for (Map.Entry 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 index 6d5baac8fa7..b42fe2844f9 100644 --- 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 index dd46f2a400b..e3dfbb6cc08 100644 --- 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 index 5bbc57eca2f..54041902838 100644 --- 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) {