-
Notifications
You must be signed in to change notification settings - Fork 420
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
ThreadLocalVar #22
Closed
Closed
ThreadLocalVar #22
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
module Concurrent | ||
|
||
module ThreadLocalSymbolAllocator | ||
|
||
protected | ||
|
||
def allocate_symbol | ||
# Warning: this will space leak if you create ThreadLocalVars in a loop - not sure what to do about it | ||
@symbol = "thread_local_symbol_#{self.object_id}_#{Time.now.hash}".to_sym | ||
end | ||
|
||
end | ||
|
||
module ThreadLocalOldStorage | ||
|
||
include ThreadLocalSymbolAllocator | ||
|
||
protected | ||
|
||
def allocate_storage | ||
allocate_symbol | ||
end | ||
|
||
def get | ||
Thread.current[@symbol] | ||
end | ||
|
||
def set(value) | ||
Thread.current[@symbol] = value | ||
end | ||
|
||
end | ||
|
||
module ThreadLocalNewStorage | ||
|
||
include ThreadLocalSymbolAllocator | ||
|
||
protected | ||
|
||
def allocate_storage | ||
allocate_symbol | ||
end | ||
|
||
def get | ||
Thread.current.thread_variable_get(@symbol) | ||
end | ||
|
||
def set(value) | ||
Thread.current.thread_variable_set(@symbol, value) | ||
end | ||
|
||
end | ||
|
||
module ThreadLocalJavaStorage | ||
|
||
protected | ||
|
||
def allocate_storage | ||
@var = java.lang.ThreadLocal.new | ||
end | ||
|
||
def get | ||
@var.get | ||
end | ||
|
||
def set(value) | ||
@var.set(value) | ||
end | ||
|
||
end | ||
|
||
class ThreadLocalVar | ||
|
||
NIL_SENTINEL = Object.new | ||
|
||
if defined? java.lang.ThreadLocal.new | ||
include ThreadLocalJavaStorage | ||
elsif Thread.current.respond_to?(:thread_variable_set) | ||
include ThreadLocalNewStorage | ||
else | ||
include ThreadLocalOldStorage | ||
end | ||
|
||
def initialize(default = nil) | ||
@default = default | ||
allocate_storage | ||
end | ||
|
||
def value | ||
value = get | ||
|
||
if value.nil? | ||
@default | ||
elsif value == NIL_SENTINEL | ||
nil | ||
else | ||
value | ||
end | ||
end | ||
|
||
def value=(value) | ||
if value.nil? | ||
stored_value = NIL_SENTINEL | ||
else | ||
stored_value = value | ||
end | ||
|
||
set stored_value | ||
|
||
value | ||
end | ||
|
||
end | ||
|
||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
# Thread local variables | ||
|
||
A `ThreadLocalVar` is a variable where the value is different for each thread. | ||
Each variable may have a default value, but when you modify the variable only | ||
the current thread will ever see that change. | ||
|
||
```ruby | ||
v = ThreadLocalVar.new(14) | ||
v.value #=> 14 | ||
v.value = 2 | ||
v.value #=> 2 | ||
``` | ||
|
||
```ruby | ||
v = ThreadLocalVar.new(14) | ||
|
||
t1 = Thread.new do | ||
v.value #=> 14 | ||
v.value = 1 | ||
v.value #=> 1 | ||
end | ||
|
||
t2 = Thread.new do | ||
v.value #=> 14 | ||
v.value = 2 | ||
v.value #=> 2 | ||
end | ||
|
||
v.value #=> 14 | ||
``` | ||
|
||
Note that except on JRuby, `ThreadLocalVar` needs to allocate a unique symbol | ||
for each instance. This may lead to a space leak. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
require 'spec_helper' | ||
|
||
module Concurrent | ||
|
||
describe Future do | ||
|
||
context '#initialize' do | ||
|
||
it 'can set an initial value' do | ||
v = ThreadLocalVar.new(14) | ||
v.value.should eq 14 | ||
end | ||
|
||
it 'sets nil as a default initial value' do | ||
v = ThreadLocalVar.new | ||
v.value.should be_nil | ||
end | ||
|
||
it 'sets the same initial value for all threads' do | ||
v = ThreadLocalVar.new(14) | ||
t1 = Thread.new { v.value } | ||
t2 = Thread.new { v.value } | ||
t1.value.should eq 14 | ||
t2.value.should eq 14 | ||
end | ||
|
||
end | ||
|
||
context '#value' do | ||
|
||
it 'returns the current value' do | ||
v = ThreadLocalVar.new(14) | ||
v.value.should eq 14 | ||
end | ||
|
||
it 'returns the value after modification' do | ||
v = ThreadLocalVar.new(14) | ||
v.value = 2 | ||
v.value.should eq 2 | ||
end | ||
|
||
end | ||
|
||
context '#value=' do | ||
|
||
it 'sets a new value' do | ||
v = ThreadLocalVar.new(14) | ||
v.value = 2 | ||
v.value.should eq 2 | ||
end | ||
|
||
it 'returns the new value' do | ||
v = ThreadLocalVar.new(14) | ||
(v.value = 2).should eq 2 | ||
end | ||
|
||
it 'does not modify the initial value for other threads' do | ||
v = ThreadLocalVar.new(14) | ||
v.value = 2 | ||
t = Thread.new { v.value } | ||
t.value.should eq 14 | ||
end | ||
|
||
it 'does not modify the value for other threads' do | ||
v = ThreadLocalVar.new(14) | ||
v.value = 2 | ||
|
||
b1 = CountDownLatch.new(2) | ||
b2 = CountDownLatch.new(2) | ||
|
||
t1 = Thread.new do | ||
b1.count_down | ||
b1.wait | ||
v.value = 1 | ||
b2.count_down | ||
b2.wait | ||
v.value | ||
end | ||
|
||
t2 = Thread.new do | ||
b1.count_down | ||
b1.wait | ||
v.value = 2 | ||
b2.count_down | ||
b2.wait | ||
v.value | ||
end | ||
|
||
t1.value.should eq 1 | ||
t2.value.should eq 2 | ||
end | ||
|
||
end | ||
|
||
end | ||
|
||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should allocate this from a global counter instead? Really make sure it is unique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use
SecureRandom.uuid
? Would that provide sufficient uniqueness? http://www.ruby-doc.org/stdlib-2.1.1/libdoc/securerandom/rdoc/SecureRandom.html#method-c-uuidThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... that's still just probabilistic. So all we could say to users is this code will probably work. I'll do it properly and use a counter. People probably aren't creating these in an inner loop anyway.
I'm more worried about the space leak.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it could be a good idea, but what about an "upside down" implementation?
At the moment threadlocals are implemented in MRI using thread storage, but we can reverse it keeping an internal map with threads as key.
This we'll avoid every memory leak or uniqueness; I've just launched a very basic benchmark (Thread.current.thread_variable vs a map guarded by a lock) and this option seems only slightly slower with respect to the current one.
If you think this is a good idea to explore, we could try to build better benchmark
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should take a look at how MRI implements Thread.current and thread locals internally. It's likely that's not using any shared hash at all, so if we have our own global hash with a local around, surely that won't scale very well.
I'll commit using this approach since with a global counter it will work, and the counter shouldn't really be a problem unless we're allocating
ThreadLocalVar
in an inner loop, and then we can go back and look at it empirically. It's not much code to change if we find a better approach later.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be honest, I don't know enough about the MRI internals to have any idea how that's implemented. I'm curious so I'm going to take a look and see if I can figure it out. I trust your judgement on this.