Skip to content

Synchronization updates: final and volatile fields, immutable struct #284

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.

Sign up for GitHub

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 17 commits into from
May 2, 2015
Merged

Synchronization updates: final and volatile fields, immutable struct #284

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6dbfa92
Add ns_initialize which is always called inside synchronize block
pitr-ch Apr 28, 2015
2a182b4
Remove wait, signal, broadcast in favor of ns_ counterparts
pitr-ch Apr 28, 2015
505d6dd
Make synchronize private by default
pitr-ch Apr 28, 2015
08c2423
Add ensure_ivar_visibility! and volatile attributes
pitr-ch Apr 28, 2015
41b7adf
Add ImmutableStruct implementation
pitr-ch Apr 28, 2015
5c4425f
Apply new Synchronization::Object capabilities to Edge::Future
pitr-ch Apr 28, 2015
1ba4f88
ImmutableStruct Improvements
pitr-ch Apr 29, 2015
47cde7b
Test stabilization
pitr-ch Apr 29, 2015
310b7df
Synchronization module documentation
pitr-ch Apr 29, 2015
2907026
Move ensure_ivar_visibility! ruby method the the Java extension on JRuby
pitr-ch Apr 29, 2015
b94357b
Move Object definition to its own file
pitr-ch Apr 30, 2015
8c92b51
Remove unnecessary definition of members methods by eval
pitr-ch Apr 30, 2015
32162e0
Extract ruby_version comparison
pitr-ch Apr 30, 2015
3dc0e87
Making Event and Future fields volatile to remove few more synchroniz…
pitr-ch Apr 30, 2015
df217b2
Merge branch 'master' into synchronization
pitr-ch May 2, 2015
cab6395
Shorten the lock and condition ivar names
pitr-ch May 2, 2015
67a82f7
Ensure task arguments are supplied by block arguments not by captured…
pitr-ch May 2, 2015
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
Prev Previous commit
Next Next commit
Remove unnecessary definition of members methods by eval
  • Loading branch information
@pitr-ch
pitr-ch committed Apr 30, 2015
commit 8c92b51ba7930da7b9263380094e78b4ab2bfabf
9 changes: 4 additions & 5 deletions lib/concurrent/synchronization/immutable_struct.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,12 @@ class ImmutableStruct < Synchronization::Object
def self.with_fields(*names, &block)
Copy link
Member

Choose a reason for hiding this comment

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

I think construction and behavior of these should mirror Ruby's Struct class. Again, I'm a big fan of the concept but we should be aware of the user's expectations. This is similar to the conversation regarding Agent. Struct is part of the Ruby standard library so it's reasonable for a user to expect an ImmutableStruct to be "an immutable Struct."

Copy link
Member

Choose a reason for hiding this comment

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

PS: I'd be happy to update this class after we merge. Having built similar classes in other projects I think it would be fun to work on this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah you are right I should have made it to match in the first place. I'll appreciate your help, I am not sure how to deal with the new method, where it in one case constructs classes and in the second instances.

Copy link
Member

Choose a reason for hiding this comment

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

I'll work on it tonight. I'll be in a hotel room looking for something productive to do (I'm attending Stir Trek tomorrow).

Class.new(self) do
attr_reader(*names)

class_eval <<-RUBY, __FILE__, __LINE__ + 1
Copy link
Member

Choose a reason for hiding this comment

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

What is the advantage of class_eval over define_method?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only performance improvements in some cases, otherwise define_method is cleaner. I wanted to avoid storing instance variables using reflection (which is optimized only on Truffle AFAIK).

Copy link
Member Author

Choose a reason for hiding this comment

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

One eval removed in 3dbc08f.

def initialize(#{names.join(', ')})
#{names.map { |n| '@' + n.to_s }.join(', ')} = #{names.join(', ')}
ensure_ivar_visibility!
end

def members
#{names.inspect}
end

def self.members
#{names.inspect}
end
Expand All @@ -46,6 +41,10 @@ def self.[](*args)
new *args
end

def members
self.class.members
end

include Enumerable

def each(&block)
Expand Down