Skip to content

Commit

Permalink
fix #8207, @parallel evaluates its range twice
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Aug 21, 2015
1 parent 128797f commit 30ed04c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 12 deletions.
23 changes: 11 additions & 12 deletions base/multi.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1491,34 +1491,29 @@ function pfor(f, N::Int)
[@spawn f(first(c), last(c)) for c in splitrange(N, nworkers())]
end

function make_preduce_body(reducer, var, body, ran)
localize_vars(
function make_preduce_body(reducer, var, body, R)
quote
function (lo::Int, hi::Int)
R = $(esc(ran))
$(esc(var)) = R[lo]
$(esc(var)) = ($R)[lo]
ac = $(esc(body))
if lo != hi
for $(esc(var)) in R[(lo+1):hi]
for $(esc(var)) in ($R)[(lo+1):hi]
ac = ($(esc(reducer)))(ac, $(esc(body)))
end
end
ac
end
end
)
end

function make_pfor_body(var, body, ran)
localize_vars(
function make_pfor_body(var, body, R)
quote
function (lo::Int, hi::Int)
for $(esc(var)) in ($(esc(ran)))[lo:hi]
for $(esc(var)) in ($R)[lo:hi]
$(esc(body))
end
end
end
)
end

macro parallel(args...)
Expand All @@ -1537,16 +1532,20 @@ macro parallel(args...)
var = loop.args[1].args[1]
r = loop.args[1].args[2]
body = loop.args[2]
localize_vars(
if na==1
quote
pfor($(make_pfor_body(var, body, r)), length($(esc(r))))
therange = $(esc(r))
pfor($(make_pfor_body(var, body, :therange)), length(therange))
end
else
quote
therange = $(esc(r))

This comment has been minimized.

Copy link
@amitmurthy

amitmurthy Aug 24, 2015

Contributor

I think this commit leads to a regression in performance when

@time @parallel (+) for i=1:100_000_000; randn(); end; is typed directly into a REPL. Wrapping the test code in a function makes it go away. Is it due to therange being treated as a global?

cc : @JeffBezanson

preduce($(esc(reducer)),
$(make_preduce_body(reducer, var, body, r)), length($(esc(r))))
$(make_preduce_body(reducer, var, body, :therange)), length(therange))
end
end
)
end


Expand Down
8 changes: 8 additions & 0 deletions test/parallel.jl
Original file line number Diff line number Diff line change
Expand Up @@ -506,3 +506,11 @@ let t = @task 42
schedule(t, ErrorException(""), error=true)
@test_throws ErrorException wait(t)
end

# issue #8207
let A = Any[]
@parallel (+) for i in (push!(A,1); 1:2)
i
end
@test length(A) == 1
end

0 comments on commit 30ed04c

Please sign in to comment.