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

Consider falsy value on `Concurrent::Map#compute_if_absent` fast non-blocking path #879

Merged

Conversation

@mtsmfm
Copy link
Contributor

@mtsmfm mtsmfm commented Jul 21, 2020

Currently, compute_if_absent with falsy value is much slower than truthy value
because non-blocking path only considers truthy one.

if stored_value = _get(key) # fast non-blocking path for the most likely case

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  gem 'benchmark-ips', require: 'benchmark/ips'
  gem 'concurrent-ruby', require: 'concurrent'
end

concurrent_map = Concurrent::Map.new
concurrent_map[:nil] = nil
concurrent_map[:non_nil] = 1

Benchmark.ips do |x|
  x.report("[Concurrent::Map] :nil") do
    concurrent_map.compute_if_absent(:nil) { raise }
  end

  x.report("[Concurrent::Map] :non_nil") do
    concurrent_map.compute_if_absent(:non_nil) { raise }
  end

  x.compare!
end
Warming up --------------------------------------
[Concurrent::Map] :nil
                       312.584k i/100ms
[Concurrent::Map] :non_nil
                       704.894k i/100ms
Calculating -------------------------------------
[Concurrent::Map] :nil
                          2.404M (± 6.6%) i/s -     12.191M in   5.095349s
[Concurrent::Map] :non_nil
                          6.757M (± 9.3%) i/s -     33.835M in   5.054357s

Comparison:
[Concurrent::Map] :non_nil:  6756566.7 i/s
[Concurrent::Map] :nil:  2404077.4 i/s - 2.81x  (± 0.00) slower

This patch makes it faster and keeps same-ish speed for truthy value.

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  gem 'benchmark-ips', require: 'benchmark/ips'
  gem 'concurrent-ruby', require: 'concurrent'
end

concurrent_map = Concurrent::Map.new
concurrent_map[:nil] = nil
concurrent_map[:non_nil] = 1

patched_concurrent_map = Concurrent::Map.new
patched_concurrent_map.extend(Module.new do
  def compute_if_absent(key)
    if Concurrent::NULL != (stored_value = @backend.fetch(key, Concurrent::NULL))
      stored_value
    else
      @write_lock.synchronize { super }
    end
  end
end)
patched_concurrent_map[:nil] = nil
patched_concurrent_map[:non_nil] = 1

Benchmark.ips do |x|
  x.report("[Concurrent::Map] :nil") do
    concurrent_map.compute_if_absent(:nil) { raise }
  end

  x.report("[Concurrent::Map] :non_nil") do
    concurrent_map.compute_if_absent(:non_nil) { raise }
  end

  x.report("[Concurrent::Map with patch] :nil") do
    patched_concurrent_map.compute_if_absent(:nil) { raise }
  end

  x.report("[Concurrent::Map with patch] :non_nil") do
    patched_concurrent_map.compute_if_absent(:non_nil) { raise }
  end

  x.compare!
end
Warming up --------------------------------------
[Concurrent::Map] :nil
                       310.135k i/100ms
[Concurrent::Map] :non_nil
                       923.645k i/100ms
[Concurrent::Map with patch] :nil
                       928.247k i/100ms
[Concurrent::Map with patch] :non_nil
                       840.197k i/100ms
Calculating -------------------------------------
[Concurrent::Map] :nil
                          2.049M (±11.4%) i/s -     10.234M in   5.060933s
[Concurrent::Map] :non_nil
                          7.181M (± 7.5%) i/s -     36.022M in   5.048215s
[Concurrent::Map with patch] :nil
                          7.479M (± 5.0%) i/s -     38.058M in   5.103572s
[Concurrent::Map with patch] :non_nil
                          7.413M (± 6.9%) i/s -     36.969M in   5.013929s

Comparison:
[Concurrent::Map with patch] :nil:  7478557.1 i/s
[Concurrent::Map with patch] :non_nil:  7413277.4 i/s - same-ish: difference falls within error
[Concurrent::Map] :non_nil:  7181043.9 i/s - same-ish: difference falls within error
[Concurrent::Map] :nil:  2049352.8 i/s - 3.65x  (± 0.00) slower
…blocking path

Currently, `compute_if_absent` with falsy value is much slower than truthy value
because non-blocking path only considers truthy one.
https://github.com/ruby-concurrency/concurrent-ruby/blob/f749b81cb6c6291640c0004b57e60dbc2b59a72b/lib/concurrent-ruby/concurrent/collection/map/mri_map_backend.rb#L22

```ruby
require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  gem 'benchmark-ips', require: 'benchmark/ips'
  gem 'concurrent-ruby', require: 'concurrent'
end

concurrent_map = Concurrent::Map.new
concurrent_map[:nil] = nil
concurrent_map[:non_nil] = 1

Benchmark.ips do |x|
  x.report("[Concurrent::Map] :nil") do
    concurrent_map.compute_if_absent(:nil) { raise }
  end

  x.report("[Concurrent::Map] :non_nil") do
    concurrent_map.compute_if_absent(:non_nil) { raise }
  end

  x.compare!
end
```

```
Warming up --------------------------------------
[Concurrent::Map] :nil
                       312.584k i/100ms
[Concurrent::Map] :non_nil
                       704.894k i/100ms
Calculating -------------------------------------
[Concurrent::Map] :nil
                          2.404M (± 6.6%) i/s -     12.191M in   5.095349s
[Concurrent::Map] :non_nil
                          6.757M (± 9.3%) i/s -     33.835M in   5.054357s

Comparison:
[Concurrent::Map] :non_nil:  6756566.7 i/s
[Concurrent::Map] :nil:  2404077.4 i/s - 2.81x  (± 0.00) slower
```

This patch makes it faster and keeps same-ish speed for truthy value.

```ruby
require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  gem 'benchmark-ips', require: 'benchmark/ips'
  gem 'concurrent-ruby', require: 'concurrent'
end

concurrent_map = Concurrent::Map.new
concurrent_map[:nil] = nil
concurrent_map[:non_nil] = 1

patched_concurrent_map = Concurrent::Map.new
patched_concurrent_map.extend(Module.new do
  def compute_if_absent(key)
    if Concurrent::NULL != (stored_value = @backend.fetch(key, Concurrent::NULL))
      stored_value
    else
      @write_lock.synchronize { super }
    end
  end
end)
patched_concurrent_map[:nil] = nil
patched_concurrent_map[:non_nil] = 1

Benchmark.ips do |x|
  x.report("[Concurrent::Map] :nil") do
    concurrent_map.compute_if_absent(:nil) { raise }
  end

  x.report("[Concurrent::Map] :non_nil") do
    concurrent_map.compute_if_absent(:non_nil) { raise }
  end

  x.report("[Concurrent::Map with patch] :nil") do
    patched_concurrent_map.compute_if_absent(:nil) { raise }
  end

  x.report("[Concurrent::Map with patch] :non_nil") do
    patched_concurrent_map.compute_if_absent(:non_nil) { raise }
  end

  x.compare!
end
```

```
Warming up --------------------------------------
[Concurrent::Map] :nil
                       310.135k i/100ms
[Concurrent::Map] :non_nil
                       923.645k i/100ms
[Concurrent::Map with patch] :nil
                       928.247k i/100ms
[Concurrent::Map with patch] :non_nil
                       840.197k i/100ms
Calculating -------------------------------------
[Concurrent::Map] :nil
                          2.049M (±11.4%) i/s -     10.234M in   5.060933s
[Concurrent::Map] :non_nil
                          7.181M (± 7.5%) i/s -     36.022M in   5.048215s
[Concurrent::Map with patch] :nil
                          7.479M (± 5.0%) i/s -     38.058M in   5.103572s
[Concurrent::Map with patch] :non_nil
                          7.413M (± 6.9%) i/s -     36.969M in   5.013929s

Comparison:
[Concurrent::Map with patch] :nil:  7478557.1 i/s
[Concurrent::Map with patch] :non_nil:  7413277.4 i/s - same-ish: difference falls within error
[Concurrent::Map] :non_nil:  7181043.9 i/s - same-ish: difference falls within error
[Concurrent::Map] :nil:  2049352.8 i/s - 3.65x  (± 0.00) slower
```
Copy link
Member

@pitr-ch pitr-ch left a comment

Great find! Thanks 👍

@pitr-ch pitr-ch merged commit 6fd8da7 into ruby-concurrency:master Jul 22, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mtsmfm mtsmfm deleted the mtsmfm:consider-falsy-in-compute-if-absent branch Jul 23, 2020
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.