Adds a JRuby optimized version of Concurrent::Atom #441

lucasallan · 2015-10-09T05:44:17Z

Non-optimized version

~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   4.720000   0.030000   4.750000 (  4.103692)
After JVM warm-up...
   3.920000   0.020000   3.940000 (  3.925786)

Optimized version

~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   2.980000   0.020000   3.000000 (  2.742403)
After JVM warm-up...
   2.440000   0.010000   2.450000 (  2.479192)

Benchmark script:

#!/usr/bin/env ruby

require 'concurrent'
require 'benchmark'

NUM      = 50_000_000
WARM_UPS = NUM

def jruby?
  defined? JRUBY_VERSION
end

def version
  gem_dir = Gem::Specification.find_by_name('concurrent-ruby').gem_dir
  File.open(File.join(gem_dir, 'VERSION'), 'r').read
rescue
  'unknown'
end

puts "~~~ Ruby version: #{RUBY_VERSION}"
if jruby?
  puts "~~~ JRuby version: #{JRUBY_VERSION}"
else
  puts "~~~ Ruby engine: #{RUBY_ENGINE}"
end
puts "~~~ Gem version: #{version}"

Benchmark.bm do |stats|

  puts "Benchmarking #{Concurrent::Atom}..."

  ref   = Concurrent::Atom.new('foo')
  value = nil

  puts 'Before JVM warm-up...' if jruby?
  stats.report do
    NUM.times { value = ref.value }
  end

  if jruby?
    WARM_UPS.times { value = ref.value }

    puts 'After JVM warm-up...'
    stats.report do
      NUM.times { value = ref.value }
    end
  end
end

ianks · 2015-10-09T06:05:32Z

lib/concurrent/jruby_atom.rb

+  require 'concurrent/concern/observable'
+  require 'java'
+
+  java_import 'java.util.concurrent.atomic.AtomicReference'


Out of curiosity, how big is the perf hit of using Concurrent::Reference versus the Java one?

jdantonio · 2015-10-16T11:08:58Z

@lucasallan Thank you very much for working on this. I'm having difficulty following the differences between the implementations. AFAICT the main difference is that the newer version simply uses java.util.concurrent.atomic.AtomicReference directly rather than Concurrent::AtomicReference. Is that correct?

lucasallan · 2015-10-16T16:11:37Z

@jdantonio This version is directly based on Clojure code[1], but I guess we should be using Concurrent::AtomicReference rather than java.util.concurrent.atomic.AtomicReference.
But before it wasn't using AtomicReference anyway, right?

[1] - https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Atom.java

jdantonio · 2015-10-16T18:57:36Z

I didn't look at the Clojure code before implementing this or Agent. I worked entirely from the documented behavior. I probably should have looked at the Clojure source. :-) If the implementation can be improved following that example I'm all for it, but can we improve it for the base implementation? Do we need a JRuby-specific version?

lucasallan · 2015-10-19T03:32:55Z

If we build an implementation around AtomicReference then we don't need a JRuby optimized version, since AtomicReference is already jruby-optimized.

I'll work on it during this week and let you know the results.

jdantonio · 2015-10-19T14:16:40Z

@lucasallan Thank you!

lucasallan · 2015-10-20T21:23:49Z

@jdantonio Results:

JRuby Using `Concurrent::Atomic`

➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   4.190000   0.040000   4.230000 (  3.116446)
After JVM warm-up...
   2.840000   0.010000   2.850000 (  2.848073)


➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   2.860000   0.020000   2.880000 (  2.546445)
After JVM warm-up...
   2.400000   0.010000   2.410000 (  2.395978)



➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   2.620000   0.020000   2.640000 (  2.438905)
After JVM warm-up...
   2.500000   0.010000   2.510000 (  2.504156)



➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   2.740000   0.010000   2.750000 (  2.386496)
After JVM warm-up...
   2.390000   0.010000   2.400000 (  2.376449)

JRuby Using concurrent-ruby-0.9.1(Current implementation):

➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   4.690000   0.040000   4.730000 (  4.067498)
After JVM warm-up...
   3.690000   0.010000   3.700000 (  3.705187)


➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   5.000000   0.030000   5.030000 (  4.547322)
After JVM warm-up...
   4.200000   0.010000   4.210000 (  4.217589)


➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   4.710000   0.020000   4.730000 (  4.314722)
After JVM warm-up...
   4.290000   0.010000   4.300000 (  4.258959)


➜  concurrent-ruby git:(master) ✗ ruby ../atom_bench.rb
~~~ Ruby version: 2.2.2
~~~ JRuby version: 9.0.1.0
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
Before JVM warm-up...
   4.320000   0.020000   4.340000 (  3.944181)
After JVM warm-up...
   3.820000   0.020000   3.840000 (  3.842717)

MRI

MRI Using `Concurrent::Atomic`

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  18.470000   0.040000  18.510000 ( 18.550063)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  18.510000   0.030000  18.540000 ( 18.586338)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  18.460000   0.050000  18.510000 ( 18.567479)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  18.500000   0.050000  18.550000 ( 18.611711)

MRI Using concurrent-ruby-0.9.1(Current implementation):

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  16.340000   0.050000  16.390000 ( 16.473306)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  16.430000   0.070000  16.500000 ( 16.622789)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  16.440000   0.070000  16.510000 ( 16.620927)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
  16.880000   0.100000  16.980000 ( 17.167879)

MRI with C extensions

MRI EXT Using `Concurrent::Atomic`

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb                                     
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.090000   0.010000   6.100000 (  6.120272)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.130000   0.020000   6.150000 (  6.176619)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.110000   0.020000   6.130000 (  6.143121)

➜  concurrent-ruby git:(master) ruby ../atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.150000   0.020000   6.170000 (  6.199732)

MRI EXT Using concurrent-ruby-0.9.1(Current implementation):

➜  open-source  ruby atom_bench.rb 
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.490000   0.020000   6.510000 (  6.530063)

➜  open-source  ruby atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.370000   0.060000   6.430000 (  6.579451)

➜  open-source  ruby atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.240000   0.020000   6.260000 (  6.292967)

➜  open-source  ruby atom_bench.rb
~~~ Ruby version: 2.2.3
~~~ Ruby engine: ruby
~~~ Gem version: unknown
       user     system      total        real
Benchmarking Concurrent::Atom...
   6.290000   0.020000   6.310000 (  6.340481)

jdantonio · 2015-10-21T13:52:59Z

Thank you!

Adds a JRuby optimized version of Concurrent::Atom

pitr-ch · 2015-10-22T21:09:53Z

@lucasallan I very appreciate your contributions but this particular change is unsafe. The @State instance variable is not properly initialized. Generally speaking I think all of our abstractions should use synchronisation layer to avoid this type of problems. It also helps greatly to solve issues across the gem if one is found. I know the benchmark shows improvement but when the number of iterations is taken into-account it's not big gain. It's also best to avoid optimisations of a method inlining kind, that's a job for the Ruby interpreter. We should prefer code clarity and algorithmic optimisations. I would suggest to revert this change.

lucasallan · 2015-10-22T21:49:29Z

@pitr-ch Could you clarify what you meant by is not properly initialized? Also, we're using Concurrent:: AtomicReference which already give us some benefits + performance, I don't understand why it would be a bad idea.

pitr-ch · 2015-10-23T13:26:51Z

Compiler or processor can reorder writes, in particular it may choose to reorder write to @state variable with write of the newly created Atom object to a variable accessible by other threads. If it does than another thread may observe Atom object with @State variable still uninitialized which would lead to #value method failing on nil error.

There is nothing bad about Concurrent::AtomicReference, in fact attr_volatile_with_cas uses it to implement CAS operations over the state field. The problematic part is that this is already abstracted pattern and it's expected to be different in future based on Ruby implementation. Some Ruby implementations may implement this directly (CAS operations over instance variables) which will allow to get rid of the indirection caused by AtomicReference. When this happens this implementation would not get the benefit of the abstraction provided by the layer, but it would have to be modified again to use the layer.

lucasallan · 2015-10-23T16:05:51Z

Fair enough. Thanks for the awesome explanation @pitr-ch - I'll revert it later today.

pitr-ch · 2015-10-23T17:26:05Z

Thanks @lucasallan.

lucasallan assigned jdantonio Oct 9, 2015

jdantonio added the in progress label Oct 9, 2015

ianks reviewed Oct 9, 2015
View reviewed changes

Improves Concurrent::Atom performance

87d3a62

lucasallan force-pushed the master branch from fa6df50 to 87d3a62 Compare October 20, 2015 21:24

jdantonio added a commit that referenced this pull request Oct 21, 2015

Merge pull request #441 from lucasallan/master

cd158c2

Adds a JRuby optimized version of Concurrent::Atom

jdantonio merged commit cd158c2 into ruby-concurrency:master Oct 21, 2015

jdantonio removed the in progress label Oct 21, 2015

lucasallan mentioned this pull request Oct 26, 2015

Revert "Adds a JRuby optimized version of Concurrent::Atom" #455

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adds a JRuby optimized version of Concurrent::Atom #441

Adds a JRuby optimized version of Concurrent::Atom #441

Uh oh!

lucasallan commented Oct 9, 2015

Uh oh!

ianks Oct 9, 2015

Uh oh!

jdantonio commented Oct 16, 2015

Uh oh!

lucasallan commented Oct 16, 2015

Uh oh!

jdantonio commented Oct 16, 2015

Uh oh!

lucasallan commented Oct 19, 2015

Uh oh!

jdantonio commented Oct 19, 2015

Uh oh!

lucasallan commented Oct 20, 2015

Uh oh!

jdantonio commented Oct 21, 2015

Uh oh!

pitr-ch commented Oct 22, 2015

Uh oh!

lucasallan commented Oct 22, 2015

Uh oh!

pitr-ch commented Oct 23, 2015

Uh oh!

lucasallan commented Oct 23, 2015

Uh oh!

pitr-ch commented Oct 23, 2015

Uh oh!

Uh oh!

Adds a JRuby optimized version of Concurrent::Atom #441

Adds a JRuby optimized version of Concurrent::Atom #441

Uh oh!

Conversation

lucasallan commented Oct 9, 2015

Uh oh!

ianks Oct 9, 2015

Choose a reason for hiding this comment

Uh oh!

jdantonio commented Oct 16, 2015

Uh oh!

lucasallan commented Oct 16, 2015

Uh oh!

jdantonio commented Oct 16, 2015

Uh oh!

lucasallan commented Oct 19, 2015

Uh oh!

jdantonio commented Oct 19, 2015

Uh oh!

lucasallan commented Oct 20, 2015

JRuby Using Concurrent::Atomic

JRuby Using concurrent-ruby-0.9.1(Current implementation):

MRI

MRI Using Concurrent::Atomic

MRI Using concurrent-ruby-0.9.1(Current implementation):

MRI with C extensions

MRI EXT Using Concurrent::Atomic

MRI EXT Using concurrent-ruby-0.9.1(Current implementation):

Uh oh!

jdantonio commented Oct 21, 2015

Uh oh!

pitr-ch commented Oct 22, 2015

Uh oh!

lucasallan commented Oct 22, 2015

Uh oh!

pitr-ch commented Oct 23, 2015

Uh oh!

lucasallan commented Oct 23, 2015

Uh oh!

pitr-ch commented Oct 23, 2015

Uh oh!

Uh oh!

JRuby Using `Concurrent::Atomic`

MRI Using `Concurrent::Atomic`

MRI EXT Using `Concurrent::Atomic`