Skip to content
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

Use thread local variables instead of class variables #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

astro
Copy link
Owner

@astro astro commented May 28, 2015

Got this by mail, leaving it here for the new maintainer, @musybite

@musybite
Copy link
Collaborator

@jakemack thanks, your contribution is greatly appreciated!

I wonder what was specific reason to prefer thread_variable_get/thread_variable_set over plain []/[]=? IMO all those class variables making more sense in fiber-local context than in "shared-between-all-fibers-thread-local-context".

@jakemack
Copy link

jakemack commented Aug 5, 2015

That's a good point, I believe you are right.

@jakemack
Copy link

@musybite Is this a change you'd want me to make and submit or is it quick and easy enough for you to implement?

@ojab
Copy link

ojab commented Dec 8, 2015

Actually there is a use case for thread local variables: Celluloid runs actors in separate threads, but different calls to actors are run in different Fibers. Testcase shows that fibers are different and variables are lost:

I, [2015-12-08T10:34:21.428386 #19221]  INFO -- : : thread = '#<Celluloid::Thread:0x007f827b1b8500>' 'second', fiber = '#<Fiber:0x007f8279a3d9e8>' ''
I, [2015-12-08T10:34:21.428514 #19221]  INFO -- : : thread = '#<Celluloid::Thread:0x007f8279a26180>' 'first', fiber = '#<Fiber:0x007f8279a3d1f0>' ''
I, [2015-12-08T10:34:21.429111 #19221]  INFO -- : : thread = '#<Celluloid::Thread:0x007f827b1b8500>' 'second', fiber = '#<Fiber:0x007f827b863b20>' ''
I, [2015-12-08T10:34:21.429280 #19221]  INFO -- : : thread = '#<Celluloid::Thread:0x007f8279a26180>' 'first', fiber = '#<Fiber:0x007f827b8631e8>' ''

So at least for this/my usecase fiber-local variables are no-go.

@musybite
Copy link
Collaborator

Sorry for long response time :)
So, we have at least couple of valid use-cases for thread-local variables and i see no real reason for fiber-local variables. I guess i'll adapt @jakemack's MR.

@rajsahae
Copy link

I'm curious if this will ever be merged? @musybite are you still actively maintaining this gem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants