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

threading branch #13410

Merged
merged 66 commits into from Oct 30, 2015

Conversation

Projects
None yet
@vtjnash
Member

vtjnash commented Oct 1, 2015

this closes #10772 and superceeds the other threading branches

the goal of this is to have the threading support off by default (controlled by a flag in julia.h), but to have the code building and passing tests either way; and to get this merged into master sooner rather than later.

kpamnany and others added some commits Jan 19, 2015

inlining improvement that helps #10527
this avoids inserting temporary variable copies of local variables
unless they might be assigned by an inner function.

Conflicts:
	base/inference.jl
Arch D. Robison
Merge branch 'adr/snapshot' of https://github.com/JuliaLang/julia int…
…o adr/threading,

which in turn is a snapshot of threading branch.

Conflicts:
	base/cartesian.jl
	base/inference.jl
	src/Makefile
	src/codegen.cpp
	src/gc.c
	src/init.c
	src/julia.h
	src/task.c
	test/runtests.jl
for TLS variables, need to call dlsym rather than & to get the addres…
…s of the metadata rather than the thread-local pointer
@JeffBezanson

This comment has been minimized.

Show comment
Hide comment
@JeffBezanson

JeffBezanson Sep 3, 2015

Member

I see...
This goes along with the strange smell of sweep_pool_region, where the outer loop is not over per-thread heaps. This is only per-page, so it's not horrible, but still leaves a bad taste.

Member

JeffBezanson commented on 2fc8d97 Sep 3, 2015

I see...
This goes along with the strange smell of sweep_pool_region, where the outer loop is not over per-thread heaps. This is only per-page, so it's not horrible, but still leaves a bad taste.

@carnaval

This comment has been minimized.

Show comment
Hide comment
@carnaval

carnaval Sep 3, 2015

Contributor

landmark

Contributor

carnaval commented on src/gc.c in 95f22cf Sep 3, 2015

landmark

@vtjnash

This comment has been minimized.

Show comment
Hide comment
@vtjnash

vtjnash Oct 24, 2015

Member

/usr/local is always in the compiler search path, which makes it hard to build applications on that machine that don't have unexpected conflicts or dependencies inherited from homebrew

Member

vtjnash commented Oct 24, 2015

/usr/local is always in the compiler search path, which makes it hard to build applications on that machine that don't have unexpected conflicts or dependencies inherited from homebrew

@iamed2

This comment has been minimized.

Show comment
Hide comment
@iamed2

iamed2 Oct 25, 2015

Contributor

llvm-config should handle this. The llvm-config built by Julia's build returns the correct locations.

I guess I should make a separate issue for this.

Contributor

iamed2 commented Oct 25, 2015

llvm-config should handle this. The llvm-config built by Julia's build returns the correct locations.

I guess I should make a separate issue for this.

@iamed2

This comment has been minimized.

Show comment
Hide comment
@iamed2

iamed2 Oct 26, 2015

Contributor

Well, I brew rm llvm33-juliad and yet the build (from a fresh clone) is still trying to access it! I'm getting this as part of the build process:

make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found
make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found
make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found
make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found
Contributor

iamed2 commented Oct 26, 2015

Well, I brew rm llvm33-juliad and yet the build (from a fresh clone) is still trying to access it! I'm getting this as part of the build process:

make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found
make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found
make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found
make[1]: /usr/local/opt/llvm33-julia/bin/llvm-config-3.3: Command not found

kpamnany added some commits Oct 26, 2015

Threads: make it easier to turn on threads
Set JULIA_THREADS = 1 in Make.user to build with threading. The
necessary flags are conditionally set in Make.inc.
@kpamnany

This comment has been minimized.

Show comment
Hide comment
@kpamnany

kpamnany Oct 26, 2015

Contributor

@tkelman: added a note in lbm3d.m stating that we have the author's permission to reproduce the code. Would you go ahead and fix whatever's wrong in src/support/dtypes.h?

@vtjnash and @JeffBezanson: if I did it right (tested on OS X and Linux), a Make.user containing JULIA_THREADS = 1 should be all that is needed to enable threads.

Let's merge this.

Contributor

kpamnany commented Oct 26, 2015

@tkelman: added a note in lbm3d.m stating that we have the author's permission to reproduce the code. Would you go ahead and fix whatever's wrong in src/support/dtypes.h?

@vtjnash and @JeffBezanson: if I did it right (tested on OS X and Linux), a Make.user containing JULIA_THREADS = 1 should be all that is needed to enable threads.

Let's merge this.

@hayd

This comment has been minimized.

Show comment
Hide comment
@hayd

hayd Oct 26, 2015

Member

@kpamnany Perhaps it's worth asking explicitly if the author would willing to allow us to relicense this code under MIT. Their terms appear to be the same as MIT and it would be easier for julia (one less exception).

Member

hayd commented Oct 26, 2015

@kpamnany Perhaps it's worth asking explicitly if the author would willing to allow us to relicense this code under MIT. Their terms appear to be the same as MIT and it would be easier for julia (one less exception).

@kpamnany

This comment has been minimized.

Show comment
Hide comment
@kpamnany

kpamnany Oct 26, 2015

Contributor

@hayd, @tkelman: Author's response is "Hi Kiran. MIT licence agreed. I'm sure this email is enough, but I'll update the page also when I get round to it. Cheers, Iain."

Contributor

kpamnany commented Oct 26, 2015

@hayd, @tkelman: Author's response is "Hi Kiran. MIT licence agreed. I'm sure this email is enough, but I'll update the page also when I get round to it. Cheers, Iain."

tkelman and others added some commits Oct 27, 2015

@vtjnash

This comment has been minimized.

Show comment
Hide comment
@vtjnash

vtjnash Oct 28, 2015

Member

missing a trailing _ on _OS_LINUX_

Member

vtjnash commented on src/codegen.cpp in 7723bd2 Oct 28, 2015

missing a trailing _ on _OS_LINUX_

@vtjnash

This comment has been minimized.

Show comment
Hide comment
@vtjnash

vtjnash Oct 28, 2015

Member

these should go in JCPPFLAGS

Member

vtjnash commented on Make.inc in 173cd22 Oct 28, 2015

these should go in JCPPFLAGS

@ViralBShah

This comment has been minimized.

Show comment
Hide comment
@ViralBShah

ViralBShah Oct 29, 2015

Member

Bump. Merge this soon?

Member

ViralBShah commented Oct 29, 2015

Bump. Merge this soon?

@tkelman

This comment has been minimized.

Show comment
Hide comment
@tkelman

tkelman Oct 29, 2015

Contributor

There's a conflict that needs resolving. I dislike merge commits since they can bury bad conflict resolutions along with thousands of lines of unreviewable diff of everything else that has happened on master, but this is too messy to rebase at this point. A comment about where conflicts were, or a manual commit that only touches the files where conflicts happened would help in tracking conflict resolutions.

Contributor

tkelman commented Oct 29, 2015

There's a conflict that needs resolving. I dislike merge commits since they can bury bad conflict resolutions along with thousands of lines of unreviewable diff of everything else that has happened on master, but this is too messy to rebase at this point. A comment about where conflicts were, or a manual commit that only touches the files where conflicts happened would help in tracking conflict resolutions.

@vtjnash vtjnash merged commit eaf6115 into master Oct 30, 2015

1 of 2 checks passed

continuous-integration/appveyor AppVeyor was unable to build non-mergeable pull request
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@vtjnash vtjnash deleted the jn/threading branch Oct 30, 2015

@vtjnash

This comment has been minimized.

Show comment
Hide comment
@vtjnash

vtjnash Oct 30, 2015

Member

I'm not sure why github doesn't present the merge commits better. git show gives me only the lines that were affected by the merge conflict. in the future, I may try to use git rebase --preserve-merges to reduce the number of merge commits.

Member

vtjnash commented Oct 30, 2015

I'm not sure why github doesn't present the merge commits better. git show gives me only the lines that were affected by the merge conflict. in the future, I may try to use git rebase --preserve-merges to reduce the number of merge commits.

@kpamnany

This comment has been minimized.

Show comment
Hide comment
@kpamnany

kpamnany Oct 30, 2015

Contributor

Yay!

Contributor

kpamnany commented Oct 30, 2015

Yay!

@ihnorton

This comment has been minimized.

Show comment
Hide comment
@ihnorton

ihnorton Oct 30, 2015

Member

😹

Member

ihnorton commented Oct 30, 2015

😹

@johnmyleswhite

This comment has been minimized.

Show comment
Hide comment
@johnmyleswhite
Member

johnmyleswhite commented Oct 30, 2015

🎉

@tkelman

This comment has been minimized.

Show comment
Hide comment
@tkelman

tkelman Oct 30, 2015

Contributor

@vtjnash that is a great tip, thanks for that - https://gist.github.com/7e083009a110666331ba

Contributor

tkelman commented Oct 30, 2015

@vtjnash that is a great tip, thanks for that - https://gist.github.com/7e083009a110666331ba

@vtjnash

This comment has been minimized.

Show comment
Hide comment
@vtjnash

vtjnash Oct 30, 2015

Member

and even that is a massive over-approximation of the merge conflict (which was just resolving the order of the the assert vs JL_LOCK changes at the top of jl_trampoline_compile_linfo)

Member

vtjnash commented Oct 30, 2015

and even that is a massive over-approximation of the merge conflict (which was just resolving the order of the the assert vs JL_LOCK changes at the top of jl_trampoline_compile_linfo)

@KristofferC

This comment has been minimized.

Show comment
Hide comment
@KristofferC

KristofferC Oct 30, 2015

Contributor

💯

Contributor

KristofferC commented Oct 30, 2015

💯

@JeffBezanson

This comment has been minimized.

Show comment
Hide comment
@JeffBezanson
Member

JeffBezanson commented Oct 30, 2015

🎆

@ViralBShah

This comment has been minimized.

Show comment
Hide comment
@ViralBShah

ViralBShah Oct 30, 2015

Member

👍

Member

ViralBShah commented Oct 30, 2015

👍

@tkelman

This comment has been minimized.

Show comment
Hide comment
@tkelman

tkelman Oct 30, 2015

Contributor

Here's a source build with all the deps in place for this: https://hub.docker.com/r/tkelman/julia64-part2/

sudo docker run -t -i tkelman/julia64-part2:threads /bin/bash
git pull
make -j4 # just julia src and sysimg, all deps are already built
Contributor

tkelman commented Oct 30, 2015

Here's a source build with all the deps in place for this: https://hub.docker.com/r/tkelman/julia64-part2/

sudo docker run -t -i tkelman/julia64-part2:threads /bin/bash
git pull
make -j4 # just julia src and sysimg, all deps are already built
@KristofferC

This comment has been minimized.

Show comment
Hide comment
@KristofferC

KristofferC Oct 30, 2015

Contributor

@tkelman Hmm, I seem to get stuck on second round of inference.jl..

Contributor

KristofferC commented Oct 30, 2015

@tkelman Hmm, I seem to get stuck on second round of inference.jl..

@tkelman

This comment has been minimized.

Show comment
Hide comment
@tkelman

tkelman Oct 30, 2015

Contributor

odd, are you in a VM? maybe adding MARCH=x86-64 might help?

Contributor

tkelman commented Oct 30, 2015

odd, are you in a VM? maybe adding MARCH=x86-64 might help?

@KristofferC

This comment has been minimized.

Show comment
Hide comment
@KristofferC

KristofferC Oct 30, 2015

Contributor

Yes, I used the "Kitematic"-thing for Docker on windows which seems to use VirtualBox. Seems to work now with MARCH

Contributor

KristofferC commented Oct 30, 2015

Yes, I used the "Kitematic"-thing for Docker on windows which seems to use VirtualBox. Seems to work now with MARCH

@tkelman

This comment has been minimized.

Show comment
Hide comment
@tkelman

tkelman Oct 30, 2015

Contributor

Probably a virtualbox bug with avx.

Contributor

tkelman commented Oct 30, 2015

Probably a virtualbox bug with avx.

@StefanKarpinski

This comment has been minimized.

Show comment
Hide comment
@StefanKarpinski

StefanKarpinski Oct 31, 2015

Member

Getting this merge is so awesome.

Member

StefanKarpinski commented Oct 31, 2015

Getting this merge is so awesome.

@KristofferC

This comment has been minimized.

Show comment
Hide comment
@KristofferC

KristofferC Oct 31, 2015

Contributor

Since I am eager to play with this, is there any documentation yet for the new thread-macros or is it best to just look at the examples and go from there?

Contributor

KristofferC commented Oct 31, 2015

Since I am eager to play with this, is there any documentation yet for the new thread-macros or is it best to just look at the examples and go from there?

@ViralBShah

This comment has been minimized.

Show comment
Hide comment
@ViralBShah

ViralBShah Nov 1, 2015

Member

Best to look at the threads perf tests. The interface is very rudimentary. I don't think we should document until APIs stabilise.

Member

ViralBShah commented Nov 1, 2015

Best to look at the threads perf tests. The interface is very rudimentary. I don't think we should document until APIs stabilise.

@kpamnany

This comment has been minimized.

Show comment
Hide comment
@kpamnany

kpamnany Nov 1, 2015

Contributor

There are a few small example programs in test/perf/threads/. The simplest use cases are in test/threads.jl which is kind of a tutorial.

The interface is, in fact, rudimentary and the syntax preliminary. However, I think the basic interface (jl_threading_run(threadfun, args)) is extremely powerful. Not only does it allow high performance parallel for loops (which might be the single most widely used (shared-memory) parallelism construct in technical computing applications designed for scale), but together with atomics, which are also here, it can easily be used to build other shared memory parallelism models, from transactional memory to work-stealing. So, while there are certainly many many things remaining to do, I'm hoping many people will use this and offer feedback to drive priorities, or even better, write some code and raise some PRs.

@ViralBShah, @StefanKarpinski: what's a good venue to discuss the threading interface and improvements to it? Here? In #1790?

Contributor

kpamnany commented Nov 1, 2015

There are a few small example programs in test/perf/threads/. The simplest use cases are in test/threads.jl which is kind of a tutorial.

The interface is, in fact, rudimentary and the syntax preliminary. However, I think the basic interface (jl_threading_run(threadfun, args)) is extremely powerful. Not only does it allow high performance parallel for loops (which might be the single most widely used (shared-memory) parallelism construct in technical computing applications designed for scale), but together with atomics, which are also here, it can easily be used to build other shared memory parallelism models, from transactional memory to work-stealing. So, while there are certainly many many things remaining to do, I'm hoping many people will use this and offer feedback to drive priorities, or even better, write some code and raise some PRs.

@ViralBShah, @StefanKarpinski: what's a good venue to discuss the threading interface and improvements to it? Here? In #1790?

@ViralBShah

This comment has been minimized.

Show comment
Hide comment
@ViralBShah

ViralBShah Nov 1, 2015

Member

I think we should close #1790 and open specific issues to discuss various threading related issues and improvements.

Member

ViralBShah commented Nov 1, 2015

I think we should close #1790 and open specific issues to discuss various threading related issues and improvements.

@KristofferC

This comment has been minimized.

Show comment
Hide comment
@KristofferC

KristofferC Nov 9, 2015

Contributor

Using this branch I could speed up my kd tree creation with a factor of about 2.25x (using 4 threads) encountering very little problem. Really nice job here! I look forward to when this is enabled by default.

Contributor

KristofferC commented Nov 9, 2015

Using this branch I could speed up my kd tree creation with a factor of about 2.25x (using 4 threads) encountering very little problem. Really nice job here! I look forward to when this is enabled by default.

@kpamnany

This comment has been minimized.

Show comment
Hide comment
@kpamnany

kpamnany Dec 7, 2015

Contributor

@KristofferC: that kd-tree code would be a nice performance benchmark for test/perf/threads/ if you can share it. PRs for performance tests would be very welcome.

Contributor

kpamnany commented Dec 7, 2015

@KristofferC: that kd-tree code would be a nice performance benchmark for test/perf/threads/ if you can share it. PRs for performance tests would be very welcome.

@vchuravy vchuravy referenced this pull request Apr 8, 2016

Closed

Base structure for Native Operators #16

0 of 2 tasks complete
@tkelman

This comment has been minimized.

Show comment
Hide comment
@tkelman

tkelman Dec 2, 2016

Contributor

why did you move this? ref #19443

Contributor

tkelman commented on Make.inc in 4282ba2 Dec 2, 2016

why did you move this? ref #19443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment