ThreadLocalVar #22
ThreadLocalVar #22
Conversation
|
|
||
| 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 |
There was a problem hiding this comment.
Perhaps we should allocate this from a global counter instead? Really make sure it is unique.
There was a problem hiding this comment.
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-uuid
There was a problem hiding this comment.
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.
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.
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.
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.
Thoughts?