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

Optimize speed and memory for cached values returns #10

Merged
merged 1 commit into from
Aug 5, 2019

Conversation

AlexWayfer
Copy link
Contributor

require 'bundler/setup'
Bundler.setup

require 'benchmark'
require 'benchmark/ips'
require 'benchmark/memory'

puts '```ruby'
puts File.read(__FILE__).gsub("\t", '  ')
puts '```'
puts
puts '### Output'
puts
puts '```'

require_relative 'lib/memery'
require_relative 'lib/memery_new'

class Foo
  class << self
    include Memery
    include MemeryNew

    def base_find(char)
      ('a'..'k').find { |letter| letter == char }
    end

    memoize def find_old(char)
      base_find(char)
    end

    memoize_new def find_new(char)
      base_find(char)
    end
  end
end

def memery_old
  Foo.find_old('d')
end

def memery_new
  Foo.find_new('d')
end

def test
  exit unless p (((p memery_old) == (p memery_new)))
end

test

Benchmark.ips do |x|
  x.report('memery_old') { memery_old }
  x.report('memery_new') { memery_new }

  x.compare!
end

Benchmark.memory do |x|
  x.report('memery_old') { 100.times { memery_old } }
  x.report('memery_new') { 100.times { memery_new } }

  x.compare!
end

puts '```'
"d"
"d"
true
Warming up --------------------------------------
          memery_old    24.664k i/100ms
          memery_new    31.363k i/100ms
Calculating -------------------------------------
          memery_old    285.113k (± 5.7%) i/s -      1.431M in   5.034622s
          memery_new    360.540k (± 6.2%) i/s -      1.819M in   5.067173s

Comparison:
          memery_new:   360540.0 i/s
          memery_old:   285112.6 i/s - 1.26x  slower

Calculating -------------------------------------
          memery_old    32.800k memsize (     0.000  retained)
                       500.000  objects (     0.000  retained)
                         3.000  strings (     0.000  retained)
          memery_new    16.000k memsize (     0.000  retained)
                       400.000  objects (     0.000  retained)
                         3.000  strings (     0.000  retained)

Comparison:
          memery_new:      16000 allocated
          memery_old:      32800 allocated - 2.05x more

@coveralls
Copy link

coveralls commented May 14, 2019

Pull Request Test Coverage Report for Build 33

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 31: 0.0%
Covered Lines: 152
Relevant Lines: 152

💛 - Coveralls

Copy link
Owner

@tycooon tycooon left a comment

Choose a reason for hiding this comment

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

Could you please provide the whole benchmark script?

lib/memery.rb Outdated Show resolved Hide resolved
@AlexWayfer
Copy link
Contributor Author

Could you please provide the whole benchmark script?

I have provided. Memery — the file from master, MemeryNew — renamed version from this PR.

```ruby

require 'bundler/setup'
Bundler.setup

require 'benchmark'
require 'benchmark/ips'
require 'benchmark/memory'

puts '```ruby'
puts File.read(__FILE__).gsub("\t", '  ')
puts '```'
puts
puts '### Output'
puts
puts '```'

require_relative 'lib/memery'
require_relative 'lib/memery_new'

class Foo
  class << self
    include Memery
    include MemeryNew

    def base_find(char)
      ('a'..'k').find { |letter| letter == char }
    end

    memoize def find_old(char)
      base_find(char)
    end

    memoize_new def find_new(char)
      base_find(char)
    end
  end
end

def memery_old
  Foo.find_old('d')
end

def memery_new
  Foo.find_new('d')
end

def test
  exit unless p (((p memery_old) == (p memery_new)))
end

test

Benchmark.ips do |x|
  x.report('memery_old') { memery_old }
  x.report('memery_new') { memery_new }

  x.compare!
end

Benchmark.memory do |x|
  x.report('memery_old') { 100.times { memery_old } }
  x.report('memery_new') { 100.times { memery_new } }

  x.compare!
end

puts '```'
```

```
"d"
"d"
true
Warming up --------------------------------------
          memery_old    24.664k i/100ms
          memery_new    31.363k i/100ms
Calculating -------------------------------------
          memery_old    285.113k (± 5.7%) i/s -      1.431M in   5.034622s
          memery_new    360.540k (± 6.2%) i/s -      1.819M in   5.067173s

Comparison:
          memery_new:   360540.0 i/s
          memery_old:   285112.6 i/s - 1.26x  slower

Calculating -------------------------------------
          memery_old    32.800k memsize (     0.000  retained)
                       500.000  objects (     0.000  retained)
                         3.000  strings (     0.000  retained)
          memery_new    16.000k memsize (     0.000  retained)
                       400.000  objects (     0.000  retained)
                         3.000  strings (     0.000  retained)

Comparison:
          memery_new:      16000 allocated
          memery_old:      32800 allocated - 2.05x more
```
@swrobel
Copy link

swrobel commented Aug 3, 2019

@tycooon is this in a review-able state now?

@swrobel
Copy link

swrobel commented Aug 3, 2019

FYI, I benchmarked memery vs Memoist. With the published gem version, memery is 1.32x slower & allocates 1.92x more. With this fix, it's roughly identical:

Comparison:
             memoist:   417894.3 i/s
              memery:   405734.6 i/s - same-ish: difference falls within error
             memoist:   192 allocated
              memery:   200 allocated - 1.04x more

@tycooon tycooon merged commit 40fa12b into tycooon:master Aug 5, 2019
@AlexWayfer AlexWayfer deleted the optimize branch August 14, 2019 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants