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

Towards enabling multi-threading on master #15743

Closed
ViralBShah opened this issue Apr 2, 2016 · 10 comments · Fixed by #15997
Closed

Towards enabling multi-threading on master #15743

ViralBShah opened this issue Apr 2, 2016 · 10 comments · Fixed by #15997
Labels
multithreading Base.Threads and related functionality

Comments

@ViralBShah
Copy link
Member

I am wondering what it takes to enable multi-threading in the default build on master. Clearly we have loads to do, but while it is still not turned on by default, we won't get the mass testing.

I believe the steps are to enable Travis support first, where everything is tested with multi-threading enabled. Then, turn it on in master, and finally fix all the issues that show up.

Is this possible to pull off in the 0.5 release timeframe? As always, it would be great to try and have a release at JuliaCon.

@ViralBShah ViralBShah added the multithreading Base.Threads and related functionality label Apr 2, 2016
@eschnett
Copy link
Contributor

eschnett commented Apr 2, 2016

See #15647...

@kpamnany
Copy link
Contributor

kpamnany commented Apr 15, 2016

Fixes needed:

Additions needed:

  • Introduce spawn/sync in addition to the parfor we have now
  • Integrate tasks with threads
  • Get I/O working in threads
  • Handle exceptions in threads properly
  • Add more performance tests

I'm actively working on the additions now.

@ViralBShah
Copy link
Member Author

ViralBShah commented Apr 16, 2016

I think at the very least, multi-threading can be turned on in travis with 1 thread right away (except on mac).

@yuyichao
Copy link
Contributor

It can be tested on travis/appveyor with more than one thread (the travis version of osx also doesn't have the clang bug that breaks the build). The only issue is if we want to test non-threading build at the same time and the longer queue as a result of it.

@eschnett
Copy link
Contributor

I don't think there have been cases where threading led to spurious bugs. I don't think it's necessary to have non-threading tests at the same time.

@yuyichao
Copy link
Contributor

Threading and non-threading uses different codegen path and that's the part that is broken the most often if they are not both tested.

@tkelman
Copy link
Contributor

tkelman commented Apr 16, 2016

Is the non threading codegen path going to continue to be used indefinitely for some reason?

@yuyichao
Copy link
Contributor

The difference in code path is mainly how we general tls reference which is used for gcstack and current exception. Of course for non-threading build it is not actually tls so we replace it with the global symbol/address directly. This can't be done for the threading build.

If we want to use the threading code path for non-threading build we'll need to pay the cost for a function call (a few cycles) for tls lookup in non-threading build.

@tkelman
Copy link
Contributor

tkelman commented Apr 16, 2016

I guess the question is whether it's going to be noticeable. So I think the immediate path forward is to propose enabling the thread flag by default in a PR and run the benchmarks on it.

If it is a noticeable problem, could we maybe trigger the old path via an opt in annotation macro?

@vtjnash
Copy link
Member

vtjnash commented Apr 17, 2016

If it is a noticeable problem, could we maybe trigger the old path via an opt in annotation macro?

to some extent, yes: we could hardcode the TLS value for thread 0 (although, what's the point then of turning on multi-threading). and we would still pay the performance penalty in sysimg code & C code, etc. I don't think I would be in favor of allowing this added complexity into the language semantics.

tkelman added a commit that referenced this issue Apr 22, 2016
unless you opt in to setting JULIA_NUM_THREADS
closes #15743
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants