-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add @threads :dynamic
#40908
Add @threads :dynamic
#40908
Conversation
lenr = length(r) | ||
while (i = atomic_add!(idx, UInt(1))) <= lenr | ||
local $(esc(lidx)) = @inbounds r[i] | ||
$(esc(lbody)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to come up with a different approach here. We need to avoid generating two copies of the code: the code size is 2^n for nesting depth n. Even though nesting @threads
loops is not common, avoiding this is the best practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the compiler be able to constprop the schedule to delete the branch not taken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, I'd just check if schedule === :dynamic
or schedule === :static
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for late reply.
I think the :dynamic
version is just like the default version which only use multi-threads in thread 1. So I don't think the 2^n thing will happen.
This is why I said ":dynamic
creates one task per thread if possible" in the doc string.
|
||
The default schedule (used when no `schedule` argument is present) is subject to change. | ||
|
||
!!! compat "Julia 1.5" | ||
The `schedule` argument is available as of Julia 1.5. | ||
|
||
!!! compat "Julia 1.7" | ||
The `schedule` argument supports `:dynamic` as of Julia 1.7. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 space indent
Welcome, and thanks for working on this! Very good first PR. I think we'll want to discuss exactly how the scheduling should work. You version works fine for small iteration counts, but I wonder if we still want to do some chunking to prevent excessive overhead. |
I was thinking that it would be nice to be able to specify how many threads I want to split the loop into e.g. @threads 4 for i=1:100 # <--- split loop into 4 tasks
do_work()
end |
A similar approach has been implemented in #43919 and the current idea is to develop more advance dynamic scheduling into that namespace, or new schedule names |
Better balance if tasks in iteration have different time costs.