Skip to content

improve rational performance#5366

Merged
kares merged 1 commit intojruby:masterfrom
ahorek:rational_perf
Oct 16, 2018
Merged

improve rational performance#5366
kares merged 1 commit intojruby:masterfrom
ahorek:rational_perf

Conversation

@ahorek
Copy link
Contributor

@ahorek ahorek commented Oct 13, 2018 •
edited
Loading

this patch reduces allocations and performance by specializing #canonicalizeInternal for long arguments

i took #5364 as a baseline

         Time#subsec      6.295M (±17.4%) i/s -     28.872M in   4.986291s
            nil#to_r     16.621M (±10.2%) i/s -     81.430M in   4.989624s
           Time#to_r      6.258M (±10.3%) i/s -     30.531M in   4.993177s
          Date#parse    218.346k (± 4.2%) i/s -      1.098M in   5.040803s

patch

         Time#subsec      6.922M (±18.0%) i/s -     31.771M in   4.983271s
            nil#to_r     17.436M (±12.6%) i/s -     84.362M in   4.987866s
           Time#to_r      7.347M (± 6.8%) i/s -     36.455M in   4.992077s
          Date.parse    230.798k (± 3.7%) i/s -      1.154M in   5.005673s

benchmark

require 'benchmark/ips'
require 'date'

Benchmark.ips do |x|
  x.report "Time#subsec" do |t|
    time = Time.now
    t.times { time.subsec }
  end

  x.report "nil#to_r" do |t|
    t.times { nil.to_r }
  end

  x.report "Time#to_r" do |t|
    time = Time.now
    t.times { time.to_r }
  end

  x.report "Date#parse" do |t|
    date = '2018-07-17'
    t.times { Date.parse(date, false) }
  end
end

@ahorek ahorek force-pushed the rational_perf branch 3 times, most recently from 9ce945f to ba449eb Compare October 13, 2018 22:53
kares
kares reviewed Oct 15, 2018
View reviewed changes
core/src/main/java/org/jruby/RubyNil.java Outdated
@JRubyMethod
public static IRubyObject to_r(ThreadContext context, IRubyObject recv) {
return RubyRational.newRationalCanonicalize(context, RubyFixnum.zero(context.runtime));
return RubyRational.newRationalRaw(context.runtime, RubyFixnum.zero(context.runtime));
Copy link
Member

@kares kares Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will break compat with (deprecated) mathn as with require 'mathn/rational' nil.to_r returns 0

Copy link
Contributor Author

@ahorek ahorek Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch, thanks!

ahorek
ahorek commented Oct 15, 2018
View reviewed changes
@kares kares merged commit b8a3aec into jruby:master Oct 16, 2018
@kares kares added this to the JRuby 9.2.1.0 milestone Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kares kares kares left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

JRuby 9.2.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@ahorek @kares