Skip to content

move attr_volatile into Concurrent::Synchronization::Volatile module #424

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

Merged
merged 5 commits into from
Nov 2, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 25 additions & 18 deletions ext/com/concurrent_ruby/ext/SynchronizationLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.jruby.RubyClass;
import org.jruby.RubyModule;
import org.jruby.RubyObject;
import org.jruby.RubyBasicObject;
import org.jruby.anno.JRubyClass;
import org.jruby.anno.JRubyMethod;
import org.jruby.runtime.ObjectAllocator;
Expand Down Expand Up @@ -47,6 +48,9 @@ public void load(Ruby runtime, boolean wrap) throws IOException {
defineModule("Concurrent").
defineModuleUnder("Synchronization");

RubyModule jrubyAttrVolatileModule = synchronizationModule.defineModuleUnder("JRubyAttrVolatile");
jrubyAttrVolatileModule.defineAnnotatedMethods(JRubyAttrVolatile.class);

defineClass(runtime, synchronizationModule, "AbstractObject", "JRubyObject",
JRubyObject.class, JRUBY_OBJECT_ALLOCATOR);

Expand Down Expand Up @@ -81,21 +85,16 @@ private RubyClass defineClass(Ruby runtime, RubyModule namespace, String parentN
// SynchronizedVariableAccessor wraps with synchronized block, StampedVariableAccessor uses fullFence or
// volatilePut

@JRubyClass(name = "JRubyObject", parent = "AbstractObject")
public static class JRubyObject extends RubyObject {
private static volatile ThreadContext threadContext = null;

public JRubyObject(Ruby runtime, RubyClass metaClass) {
super(runtime, metaClass);
}
// module JRubyAttrVolatile
public static class JRubyAttrVolatile {

@JRubyMethod
public IRubyObject initialize(ThreadContext context) {
return this;
}
// volatile threadContext is used as a memory barrier per the JVM memory model happens-before semantic
// on volatile fields. any volatile field could have been used but using the thread context is an
// attempt to avoid code elimination.
private static volatile ThreadContext threadContext = null;

@JRubyMethod(name = "full_memory_barrier", visibility = Visibility.PRIVATE)
public IRubyObject fullMemoryBarrier(ThreadContext context) {
public static IRubyObject fullMemoryBarrier(ThreadContext context, IRubyObject self) {
// Prevent reordering of ivar writes with publication of this instance
if (UnsafeHolder.U == null || !UnsafeHolder.SUPPORTS_FENCES) {
// Assuming that following volatile read and write is not eliminated it simulates fullFence.
Expand All @@ -109,35 +108,43 @@ public IRubyObject fullMemoryBarrier(ThreadContext context) {
}

@JRubyMethod(name = "instance_variable_get_volatile", visibility = Visibility.PROTECTED)
public IRubyObject instanceVariableGetVolatile(ThreadContext context, IRubyObject name) {
public static IRubyObject instanceVariableGetVolatile(ThreadContext context, IRubyObject self, IRubyObject name) {
// Ensure we ses latest value with loadFence
if (UnsafeHolder.U == null || !UnsafeHolder.SUPPORTS_FENCES) {
// piggybacking on volatile read, simulating loadFence
final ThreadContext oldContext = threadContext;
return instance_variable_get(context, name);
return ((RubyBasicObject)self).instance_variable_get(context, name);
} else {
UnsafeHolder.loadFence();
return instance_variable_get(context, name);
return ((RubyBasicObject)self).instance_variable_get(context, name);
}
}

@JRubyMethod(name = "instance_variable_set_volatile", visibility = Visibility.PROTECTED)
public IRubyObject InstanceVariableSetVolatile(ThreadContext context, IRubyObject name, IRubyObject value) {
public static IRubyObject InstanceVariableSetVolatile(ThreadContext context, IRubyObject self, IRubyObject name, IRubyObject value) {
// Ensure we make last update visible
if (UnsafeHolder.U == null || !UnsafeHolder.SUPPORTS_FENCES) {
// piggybacking on volatile write, simulating storeFence
final IRubyObject result = instance_variable_set(name, value);
final IRubyObject result = ((RubyBasicObject)self).instance_variable_set(name, value);
threadContext = context;
return result;
} else {
// JRuby uses StampedVariableAccessor which calls fullFence
// so no additional steps needed.
// See https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/runtime/ivars/StampedVariableAccessor.java#L151-L159
return instance_variable_set(name, value);
return ((RubyBasicObject)self).instance_variable_set(name, value);
}
}
}

@JRubyClass(name = "JRubyObject", parent = "AbstractObject")
public static class JRubyObject extends RubyObject {

public JRubyObject(Ruby runtime, RubyClass metaClass) {
super(runtime, metaClass);
}
}

@JRubyClass(name = "Object", parent = "JRubyObject")
public static class Object extends JRubyObject {

Expand Down
1 change: 1 addition & 0 deletions lib/concurrent/synchronization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'concurrent/synchronization/jruby_object'
require 'concurrent/synchronization/rbx_object'
require 'concurrent/synchronization/object'
require 'concurrent/synchronization/volatile'

require 'concurrent/synchronization/abstract_lockable_object'
require 'concurrent/synchronization/mri_lockable_object'
Expand Down
44 changes: 26 additions & 18 deletions lib/concurrent/synchronization/jruby_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,41 @@ module Synchronization

if Concurrent.on_jruby?

# @!visibility private
# @!macro internal_implementation_note
class JRubyObject < AbstractObject

def initialize
# nothing to do
module JRubyAttrVolatile
def self.included(base)
base.extend(ClassMethods)
end

def self.attr_volatile(*names)
names.each do |name|
module ClassMethods
def attr_volatile(*names)
names.each do |name|

ivar = :"@volatile_#{name}"
ivar = :"@volatile_#{name}"

class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{name}
instance_variable_get_volatile(:#{ivar})
end
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{name}
instance_variable_get_volatile(:#{ivar})
end

def #{name}=(value)
instance_variable_set_volatile(:#{ivar}, value)
end
RUBY
def #{name}=(value)
instance_variable_set_volatile(:#{ivar}, value)
end
RUBY

end
names.map { |n| [n, :"#{n}="] }.flatten
end
names.map { |n| [n, :"#{n}="] }.flatten
end
end

# @!visibility private
# @!macro internal_implementation_note
class JRubyObject < AbstractObject
include JRubyAttrVolatile

def initialize
# nothing to do
end
end
end
end
Expand Down
46 changes: 27 additions & 19 deletions lib/concurrent/synchronization/mri_object.rb
Original file line number Diff line number Diff line change
@@ -1,35 +1,43 @@
module Concurrent
module Synchronization

# @!visibility private
# @!macro internal_implementation_note
class MriObject < AbstractObject
module MriAttrVolatile
def self.included(base)
base.extend(ClassMethods)
end

def initialize
# nothing to do
module ClassMethods
def attr_volatile(*names)
names.each do |name|
ivar = :"@volatile_#{name}"
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{name}
#{ivar}
end

def #{name}=(value)
#{ivar} = value
end
RUBY
end
names.map { |n| [n, :"#{n}="] }.flatten
end
end

def full_memory_barrier
# relying on undocumented behavior of CRuby, GVL acquire has lock which ensures visibility of ivars
# https://github.com/ruby/ruby/blob/ruby_2_2/thread_pthread.c#L204-L211
end
end

def self.attr_volatile(*names)
names.each do |name|
ivar = :"@volatile_#{name}"
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{name}
#{ivar}
end
# @!visibility private
# @!macro internal_implementation_note
class MriObject < AbstractObject
include MriAttrVolatile

def #{name}=(value)
#{ivar} = value
end
RUBY
end
names.map { |n| [n, :"#{n}="] }.flatten
def initialize
# nothing to do
end
end

end
end
50 changes: 29 additions & 21 deletions lib/concurrent/synchronization/rbx_object.rb
Original file line number Diff line number Diff line change
@@ -1,37 +1,45 @@
module Concurrent
module Synchronization

module RbxAttrVolatile
def self.included(base)
base.extend(ClassMethods)
end

# @!visibility private
# @!macro internal_implementation_note
class RbxObject < AbstractObject
def initialize
# nothing to do
module ClassMethods
def attr_volatile(*names)
names.each do |name|
ivar = :"@volatile_#{name}"
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{name}
Rubinius.memory_barrier
#{ivar}
end

def #{name}=(value)
#{ivar} = value
Rubinius.memory_barrier
end
RUBY
end
names.map { |n| [n, :"#{n}="] }.flatten
end
end

def full_memory_barrier
# Rubinius instance variables are not volatile so we need to insert barrier
Rubinius.memory_barrier
end
end

def self.attr_volatile *names
names.each do |name|
ivar = :"@volatile_#{name}"
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def #{name}
Rubinius.memory_barrier
#{ivar}
end
# @!visibility private
# @!macro internal_implementation_note
class RbxObject < AbstractObject
include RbxAttrVolatile

def #{name}=(value)
#{ivar} = value
Rubinius.memory_barrier
end
RUBY
end
names.map { |n| [n, :"#{n}="] }.flatten
def initialize
# nothing to do
end
end

end
end
35 changes: 35 additions & 0 deletions lib/concurrent/synchronization/volatile.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
module Concurrent
module Synchronization

# Volatile adds the attr_volatile class method when included.
#
# @example
# class Foo
# include Concurrent::Synchronization::Volatile
#
# attr_volatile :bar
#
# def initialize
# self.bar = 1
# end
# end
#
# foo = Foo.new
# foo.bar
# => 1
# foo.bar = 2
# => 2

Volatile = case
when Concurrent.on_cruby?
MriAttrVolatile
when Concurrent.on_jruby?
JRubyAttrVolatile
when Concurrent.on_rbx?
RbxAttrVolatile
else
warn 'Possibly unsupported Ruby implementation'
MriAttrVolatile
end
Copy link
Member

Choose a reason for hiding this comment

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

Could you use same pattern as Object is using, with the Implementation intermediate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure exactly what you are suggesting: I see that in Object there is private_constant :ObjectImplementation, but in volatile.rb the Volatile constant must stay public since the it the module you include, which points to the correct module implementation.

or do you mean just adding

    # @!visibility private
    # @!macro internal_implementation_note

to the actual module implementation, like module JRubyAttrVolatile ?

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry, I did not think it through, it's a module not a class so it is not applicable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one thing we could do is to maybe DRY up this switch over the ruby implementations.

end
end
Loading