Merge pull request #6031 from kares/fix-rescue-nil-perf

[fix] restore rescue nil performance
jruby · Jan 13, 2020 · 77d5442 · 77d5442
2 parents 6727556 + 6acc73d
commit 77d5442
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 8 deletions.
diff --git a/bench/core/kernel/bench_raise.rb b/bench/core/kernel/bench_raise.rb
@@ -0,0 +1,48 @@
+require 'benchmark/ips'
+
+Benchmark.ips do |x|
+
+  x.config(:time => 10, :warmup => 10)
+
+  x.report("raise(msg) default") do
+    begin
+      raise 'default'
+    rescue => ex
+      ex && true
+    end
+  end
+
+  x.report("raise(class, msg)") do
+    begin
+      raise RuntimeError, 'def'
+    rescue => ex
+      ex && true
+    end
+  end
+
+  x.report("raise(class, msg, nil)") do
+    begin
+      raise RuntimeError, 'nil', nil
+    rescue => ex
+      ex && true
+    end
+  end
+
+  trace = caller.dup
+  x.report("raise(class, msg, trace)") do
+    begin
+      raise RuntimeError, 'nil', backtrace
+    rescue => ex
+      ex && true
+    end
+  end
+
+  x.report("raise(class, msg) rescue nil") do
+    (raise RuntimeError, 'def') rescue nil
+  end
+
+  x.report("raise(class, msg, trace) rescue nil") do
+    (raise RuntimeError, 'def', trace) rescue nil
+  end
+
+end
diff --git a/core/src/main/java/org/jruby/exceptions/RaiseException.java b/core/src/main/java/org/jruby/exceptions/RaiseException.java
@@ -43,6 +43,7 @@
 import org.jruby.RubyClass;
 import org.jruby.RubyException;
 import org.jruby.RubyInstanceConfig;
+import org.jruby.RubyStandardError;
 import org.jruby.RubyString;
 import org.jruby.runtime.Helpers;
 import org.jruby.runtime.JavaSites;
@@ -111,13 +112,13 @@ private void preRaise(ThreadContext context, IRubyObject backtrace, boolean capt
 
         if (backtrace == null) {
             if (capture) { // only false to support legacy RaiseException construction (not setting trace)
-                exception.captureBacktrace(context);
+                if (requiresBacktrace(context)) exception.captureBacktrace(context);
                 setStackTraceFromException();
             }
         } else {
             exception.setBacktrace(backtrace);
             if (!backtrace.isNil() && !isEmptyArray(backtrace)) {
-                exception.captureBacktrace(context);
+                if (requiresBacktrace(context)) exception.captureBacktrace(context);
             }
             setStackTraceFromException();
         }
@@ -270,12 +271,9 @@ private void preRaise(ThreadContext context, StackTraceElement[] javaTrace) {
         }
     }
 
-    @Deprecated
     private boolean requiresBacktrace(ThreadContext context) {
         // We can only omit backtraces of descendents of Standard error for 'foo rescue nil'
-        return context.exceptionRequiresBacktrace ||
-                !context.runtime.getStandardError().isInstance(exception) ||
-                context.runtime.isDebug();
+        return context.exceptionRequiresBacktrace || !(exception instanceof RubyStandardError);
     }
 
     private static boolean isEmptyArray(final IRubyObject ary) {

diff --git a/core/src/main/java/org/jruby/runtime/ThreadContext.java b/core/src/main/java/org/jruby/runtime/ThreadContext.java
@@ -1234,17 +1234,18 @@ public void setRecursiveSet(Set<RecursiveComparator.Pair> recursiveSet) {
     }
 
     public void setExceptionRequiresBacktrace(boolean exceptionRequiresBacktrace) {
+        if (runtime.isDebug()) return; // leave true default
         this.exceptionRequiresBacktrace = exceptionRequiresBacktrace;
     }
 
     @JIT
     public void exceptionBacktraceOn() {
-        this.exceptionRequiresBacktrace = true;
+        setExceptionRequiresBacktrace(true);
     }
 
     @JIT
     public void exceptionBacktraceOff() {
-        this.exceptionRequiresBacktrace = false;
+        setExceptionRequiresBacktrace(false);
     }
 
     private Map<String, Map<IRubyObject, IRubyObject>> symToGuards;