Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc optz #3930

Merged
merged 6 commits into from Jul 9, 2016
Merged

Misc optz #3930

merged 6 commits into from Jul 9, 2016

Conversation

@headius
Copy link
Member

headius commented May 26, 2016

I'm doing a few passes over our favorite benchmarks looking for things to fix. As of creation, this PR contains the following:

  • Limit frame requirement for AttrAssign to method names that are known to need frame (specifically, just []= should require a frame. All other frame-aware method names compile as Call and not AttrAssign.
  • Remove binding requirement from UnresolvedSuper, after the success of the same change in ZSuper.
  • Replace Java-based BasicObject#!= with a Ruby version, so it caches ==.

These changes have a measurable effect on red/black, bringing average time per iter from high 0.4s to low 0.4s on 8u92.

More to come, probably.

@headius headius self-assigned this May 26, 2016
@headius headius added this to the JRuby 9.1.3.0 milestone May 26, 2016
@headius
Copy link
Member Author

headius commented May 26, 2016

After a couple more profiling runs of red/black, here's the findings I've come up with:

  • Lots and lots of Fixnums being allocated. Hard for us to do anything about until we can specialize code and object layout.
  • Lots of Object[] when running with invokedynamic, mostly from LambdaForm. This is a concern we should bring up with OpenJDK folks.
  • Lots of IRubyObject[], mostly from yield logic that has no arity-splitting yet.

The fixes in the PR have eliminated Helpers.invokedynamic (from RubyBasicObject's Java-based != impl) and framing for any of the hot methods in red/black, so that's good. Frame push/pop and scope allocation does not appear in my profiles anymore.

@headius
Copy link
Member Author

headius commented May 26, 2016

Moving on to another benchmark, bench_pythag.rb from our rubybench project.

Sampled profile shows heavy use of RubyModule.searchMethod*, almost exclusively due to type conversions in RubyNumeric.numericToFloat, which has to do respond_to? checks mostly uncached. I figured getting these type conversions better short-cutted and with better caching could have a big impact.

I was surprised how much of an impact!

A patch to simply instanceof check for a few core numeric types, dispatching to them statically, has the following effect:

BEFORE

[] ~/projects/jruby $ jruby -Xcompile.invokedynamic ../rubybench/time/bench_pythag.rb 
  3.470000   0.220000   3.690000 (  2.577027)
  2.750000   0.070000   2.820000 (  2.547048)
  2.430000   0.050000   2.480000 (  2.365870)
  2.330000   0.040000   2.370000 (  2.326562)
  2.300000   0.030000   2.330000 (  2.299861)

AFTER:

[] ~/projects/jruby $ jruby -Xcompile.invokedynamic ../rubybench/time/bench_pythag.rb 
  2.480000   0.200000   2.680000 (  1.646376)
  1.580000   0.100000   1.680000 (  1.389151)
  1.290000   0.080000   1.370000 (  1.265897)
  1.200000   0.040000   1.240000 (  1.201908)
  1.160000   0.020000   1.180000 (  1.166696)

We need better mechanisms to know whether methods like to_f have been monkey-patched, but it would also be pretty awful for someone to monkey-patch a core to_f expecting it to be called for auto-conversions like this.

Here's the patch.

diff --git a/core/src/main/java/org/jruby/RubyNumeric.java b/core/src/main/java/org/jruby/RubyNumeric.java
index f606ca9..9642153 100644
--- a/core/src/main/java/org/jruby/RubyNumeric.java
+++ b/core/src/main/java/org/jruby/RubyNumeric.java
@@ -39,6 +39,7 @@ import org.jruby.anno.JRubyMethod;
 import org.jruby.ast.util.ArgsUtil;
 import org.jruby.common.RubyWarnings;
 import org.jruby.exceptions.RaiseException;
+import org.jruby.ext.bigdecimal.RubyBigDecimal;
 import org.jruby.javasupport.JavaUtil;
 import org.jruby.runtime.Block;
 import org.jruby.runtime.ClassIndex;
@@ -275,6 +276,22 @@ public class RubyNumeric extends RubyObject {
             throw runtime.newTypeError("can't convert " + num.getType() + " into Float");
         }

+        if (num instanceof RubyFloat) {
+            return num;
+        }
+
+        if (num instanceof RubyFixnum) {
+            return ((RubyFixnum) num).to_f();
+        }
+
+        if (num instanceof RubyBignum) {
+            return ((RubyBignum) num).to_f();
+        }
+
+        if (num instanceof RubyBigDecimal) {
+            return ((RubyBigDecimal) num).to_f();
+        }
+
         return TypeConverter.convertToType(num, runtime.getFloat(), "to_f");
     }
@headius
Copy link
Member Author

headius commented May 26, 2016

The patch above for numericToFloat appears to remove it from a sampled profile completely. Uncached method lookups are a lot more expensive than we realize. Dark matter?

@headius
Copy link
Member Author

headius commented May 26, 2016

Here's some numbers on pythag compared with other impls, for a current baseline. This includes my patch for numericToFloat as well as an additional patch for num2dbl. We need a better way to do dynamic calls from Ruby, or a better way to safely check if we can statically dispatch without stepping on monkey patches.

[] ~/projects/jruby $ rvm rbx do rbx -v ../rubybench/time/bench_pythag.rb 
rubinius 2.5.7.c51 (2.1.0 1d60cbe3 2015-07-09 3.5.0 JI) [x86_64-darwin14.0.0]
  5.655909   0.024235   5.680144 (  5.163726)
  4.781444   0.010537   4.791981 (  4.790965)
  4.773716   0.010968   4.784684 (  4.771892)
  4.753539   0.010764   4.764303 (  4.763747)
  4.728906   0.010384   4.739290 (  4.738738)

[] ~/projects/jruby $ rbx -v ../rubybench/time/bench_pythag.rb 
rubinius 3.33 (2.2.2 db6f477e 2016-05-23 3.6.2) [x86_64-darwin13.4.0]
 33.741741   0.193112  33.934853 ( 33.948344)
 35.608224   0.239761  35.847985 ( 35.919885)
 36.500503   0.222454  36.722957 ( 36.766840)
 36.301038   0.206905  36.507943 ( 36.535035)
 34.700161   0.164358  34.864519 ( 34.879415)

[] ~/projects/jruby $ rvm ruby-2.3.0 do ruby -v ../rubybench/time/bench_pythag.rb 
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin14]
  2.670000   0.000000   2.670000 (  2.680018)
  2.710000   0.000000   2.710000 (  2.704229)
  2.660000   0.010000   2.670000 (  2.665836)
  2.750000   0.000000   2.750000 (  2.755212)
  2.700000   0.000000   2.700000 (  2.707388)

[] ~/projects/jruby $ rvm jruby-9.1.0.0 do ruby -v ../rubybench/time/bench_pythag.rb 
jruby 9.1.0.0 (2.3.0) 2016-05-02 a633c63 Java HotSpot(TM) 64-Bit Server VM 25.92-b14 on 1.8.0_92-b14 +jit [darwin-x86_64]
  3.690000   0.110000   3.800000 (  2.826033)
  3.090000   0.040000   3.130000 (  2.711801)
  3.040000   0.040000   3.080000 (  2.875450)
  3.010000   0.030000   3.040000 (  2.905616)
  2.810000   0.030000   2.840000 (  2.701920)

[] ~/projects/jruby $ rvm jruby-9.1.0.0 do ruby -Xcompile.invokedynamic -v ../rubybench/time/bench_pythag.rb 
jruby 9.1.0.0 (2.3.0) 2016-05-02 a633c63 Java HotSpot(TM) 64-Bit Server VM 25.92-b14 on 1.8.0_92-b14 +indy +jit [darwin-x86_64]
  3.480000   0.110000   3.590000 (  2.487788)
  2.680000   0.030000   2.710000 (  2.307357)
  2.420000   0.030000   2.450000 (  2.164667)
  2.320000   0.020000   2.340000 (  2.145938)
  2.250000   0.030000   2.280000 (  2.151271)

[] ~/projects/jruby $ jruby -v ../rubybench/time/bench_pythag.rb 
jruby 9.1.3.0-SNAPSHOT (2.3.0) 2016-05-26 eb8e2a5 Java HotSpot(TM) 64-Bit Server VM 25.92-b14 on 1.8.0_92-b14 +jit [darwin-x86_64]
  2.720000   0.230000   2.950000 (  2.090044)
  1.800000   0.090000   1.890000 (  1.758849)
  1.790000   0.060000   1.850000 (  1.738153)
  1.700000   0.030000   1.730000 (  1.702496)
  1.680000   0.050000   1.730000 (  1.692608)

[] ~/projects/jruby $ jruby -Xcompile.invokedynamic -v ../rubybench/time/bench_pythag.rb 
jruby 9.1.3.0-SNAPSHOT (2.3.0) 2016-05-26 eb8e2a5 Java HotSpot(TM) 64-Bit Server VM 25.92-b14 on 1.8.0_92-b14 +indy +jit [darwin-x86_64]
  2.590000   0.200000   2.790000 (  1.679208)
  1.860000   0.110000   1.970000 (  1.404871)
  1.280000   0.080000   1.360000 (  1.248967)
  1.170000   0.030000   1.200000 (  1.174718)
  1.200000   0.030000   1.230000 (  1.209682)
@headius
Copy link
Member Author

headius commented May 26, 2016

Er, that was "we need to find a better way to do dynamic calls from Java..."

@thedarkone
Copy link
Contributor

thedarkone commented May 27, 2016

@headius thanks for these detailed write ups!

@headius
Copy link
Member Author

headius commented Jun 28, 2016

I have merged latest master and committed the numericToFloat fix. I'll let it cook in CI and merge if it looks ok.

@headius
Copy link
Member Author

headius commented Jul 9, 2016

Backed off the numeric optimization a bit. We'll get a better system in place for detecting core methods.

@headius headius merged commit fa5698e into jruby:master Jul 9, 2016
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@headius headius deleted the headius:misc_optz branch Jul 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.