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.

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

Jump to bottom

Remove single thread limitation on mac. #163

Merged
merged 1 commit into from
May 10, 2015
Merged

Remove single thread limitation on mac. #163

merged 1 commit into from
May 10, 2015

Conversation

albertnetymk
Copy link
Contributor

No description provided.

kikofernandez
kikofernandez reviewed May 9, 2015
View reviewed changes
src/runtime/encore/encore.c
@@ -197,7 +207,8 @@ void actor_save_context(encore_actor_t *actor, ucontext_t *ctx)
context *old = this_context;
this_context = pop_context(actor);
assert_swap(ctx, &this_context->ctx);
this_context = old;
tail(old);
Copy link
Contributor

Choose a reason for hiding this comment

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

hi albert, why is this line better than the old this_context = old? Does it matter or do you prefer to have it in a function?

@TobiasWrigstad
Copy link
Contributor

Kiko, I believe this is a trick to force the C compiler to omit more resilient code for reading the thread-local variable.

@kikofernandez
Copy link
Contributor

I would strongly suggest to have a comment that states so.

@kikofernandez kikofernandez self-assigned this May 10, 2015
@kikofernandez
Copy link
Contributor

@albertnetymk tested on mac 10000 times running get. No errors in DEBUG and RELEASE mode.

However, async_foreach has deadlocked when running a simple make test. I have tested async_foreach in the current master branch (EAGER and removing the SINGLE_THREAD on Mac) and it deadlocks as well. Difficult to say if this commit breaks tasks or not... could you test on your Linux all of the asyncs at least 1000 times and report if it deadlocks?

My tests for this PR:

Program name Running Deadlocks?
async 1000 NO
async_block 1000 NO
async_chain 1000 NO
async_finish 1000 NO
async_finish_stmts 1000 NO
async_force_gc 1000 YES
async_foreach 1000 YES
global_async 1000 NO

@albertnetymk
Copy link
Contributor Author

It's consistent with the runs on Linux. async_force_gc and async_foreach have concurrency bug on master branch. This PR only deals with lazy strategy when actors call get. I think they are orthogonal of each other.

@albertnetymk
Copy link
Contributor Author

Updated the tail function name to make it more explicit on osx for thread local variables.

@kikofernandez
Copy link
Contributor

@albertnetymk you are right, I'll merge this PR and create an issue for the tasks

kikofernandez pushed a commit that referenced this pull request May 10, 2015
Merge pull request #163 from albertnetymk/mac-lazy
99b8f89 
Remove single thread limitation on mac.
@kikofernandez kikofernandez merged commit 99b8f89 into parapluu:master May 10, 2015
@kikofernandez kikofernandez deleted the mac-lazy branch May 10, 2015 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kikofernandez kikofernandez

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@albertnetymk @TobiasWrigstad @kikofernandez