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

record_accessor: Make code simple and bit faster #2660

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

repeatedly
Copy link
Member

Signed-off-by: Masahiro Nakagawa repeatedly@gmail.com

Which issue(s) this PR fixes:
None

What this PR does / why we need it:
Make record_accessor code simple by moving call_index to default implementation.
New implementation is bit faster.

Warming up --------------------------------------
                base   317.029k i/100ms
                test   327.406k i/100ms
Calculating -------------------------------------
                base     10.600M (± 6.1%) i/s -     52.944M in   5.019043s
                test     10.797M (± 4.2%) i/s -     54.022M in   5.013882s

Docs Changes:
No need

Release Note:
Same as title.

Signed-off-by: Masahiro Nakagawa <repeatedly@gmail.com>
@repeatedly repeatedly added the enhancement Feature request or improve operations label Oct 21, 2019
@repeatedly repeatedly requested a review from ganmacs October 21, 2019 07:34
@repeatedly repeatedly self-assigned this Oct 21, 2019
Copy link
Member

@ganmacs ganmacs left a comment

Choose a reason for hiding this comment

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

Code is good.
But I have a question. what made code a bit faster? what kind of benchmark script did you use?

@repeatedly
Copy link
Member Author

Ah, I forgot to paste the code.

require 'benchmark/ips'

class Base
  def initialize
    @keys = 'k'
    mcall = method(:call_index)
    singleton_class.module_eval do
      define_method(:call, mcall)
    end
  end

  def call(r)
  end

  def call_dig(r)
    r.dig(*@keys)
  end

  def call_index(r)
    r[@keys]
  end
end

class Test
  def initialize
    @keys = 'k'
  end

  def call(r)
    r[@keys]
  end

  def call_dig(r)
    r.dig(*@keys)
  end
end

record = {'k' => 'foo'}
base = Base.new
test = Test.new

Benchmark.ips do |x|
  x.report "base" do
    base.call(record)
  end

  x.report "test" do
    test.call(record)
  end
end

@ganmacs
Copy link
Member

ganmacs commented Oct 21, 2019

I see. made it faster when using a default implementation 👍

@repeatedly repeatedly merged commit 0e8b05f into master Oct 21, 2019
@repeatedly repeatedly deleted the simple-record_accessor branch October 21, 2019 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants