make at-everywhere import modules before remote calls #21718

stevengj · 2017-05-05T19:44:58Z

This PR is intended to make @everywhere using Foo a bit more reliable and performant, by turning it into essentially import Foo; @everywhere using Foo. Similarly for @everwhere begin ... end and other blocks that might contain import or using statements.

e.g. right now @everywhere using Foo gives WARNING: replacing module Foo. because it ends up importing Foo twice, and that in turn can cause problems with certain modules (e.g. PyCall: JuliaPy/PyCall.jl#388) that don't like to be initialized twice. Also there are potential race conditions with cache recompilation.

Not completely sure how to implement a test for this.

amitmurthy · 2017-05-08T03:22:06Z

Wouldn't it be better to treat @everywhere using Foo as a import Foo; @everywhere importall Foo? Will also address JuliaLang/Distributed.jl#20

timholy · 2017-05-08T07:48:55Z

Having using mean the same thing as importall would mean that I couldn't "override" (vs. "extend") a method in Foo.

timholy · 2017-05-08T08:01:14Z

base/distributed/macros.jl

@@ -48,6 +48,24 @@ macro fetchfrom(p, expr)
    :(remotecall_fetch($thunk, $(esc(p))))
 end

+# extract a list of modules to import from an expression
+extract_imports(x) = Symbol[]
+function extract_imports(ex::Expr)


Not sure whether it's a big deal, but this might be more efficient as

function extract_imports!(imports, ex::Expr)

and having the everywhere caller execute it as extract_imports!(Symbol[], ex). That way you don't have to allocate an array for every single token.

Good point, will do.

StefanKarpinski · 2017-05-08T14:37:37Z

+100 to this PR.

-1000 on having using silently translated to importall. Why would that be better?

amitmurthy · 2017-05-08T16:38:32Z

-1000 on having using silently translated to importall. Why would that be better?

That was a stupid suggestion. I was under the mistaken assumption that using on the workers would trigger a load of the module on all other workers.

… calls

…ts functions

stevengj · 2017-05-08T21:11:06Z

I eliminated the unnecessary array allocations mentioned by @timholy, as well as unnecessary allocations introduced by isexpr(ex, [...heads...]).

For the latter, since we already supported isexpr(ex, heads) for heads::Union{Set,Vector}, I simply added Tuple and changed calls to use it. (The isexpr function is undocumented, albeit widely used in almost 100 packages.) In a microbenchmark, isexpr(ex, [...]) is about 2-3x slower than isexpr(ex, (...)).

JeffBezanson · 2017-05-09T20:27:43Z

test/meta.jl

@@ -122,13 +122,17 @@ using Base.Meta

 @test isexpr(:(1+1),Set([:call]))
 @test isexpr(:(1+1),Vector([:call]))
+@test isexpr(:(1+1),(:call))


Did you mean to pass a tuple here?

whoops, right

stevengj · 2017-05-10T12:10:40Z

Travis failure was a network timeout:

INFO: Cloning METADATA from https://github.com/JuliaLang/METADATA.jl
  0     0    0     0    0     0      0      0 --:--:--  0:01:15 --:--:--     0INFO: No packages to install, update or remove
  0     0    0     0    0     0      0      0 --:--:--  0:01:18 --:--:--     0curl: (7) Failed to connect to 192.0.2.1 port 80: Operation timed out
download (3)       |   80.40  |  0.00  |  0.0 |  7.94      | 179.59

JeffBezanson · 2017-05-11T15:08:20Z

Actually, I bet this is working around a bug. No combination of local and remote usings should try to load the same thing twice on one machine.

amitmurthy · 2017-05-11T15:13:47Z

Yes, there is an open issue for the bug - #19187 .

amitmurthy · 2017-05-11T15:15:03Z

This issue has been known and open for quite sometime, hence I am for merging this workaround for now - can be removed when the bug is fixed.

JeffBezanson · 2017-05-11T15:52:36Z

Hang on, I might have a fix.

JeffBezanson · 2017-05-12T04:00:36Z

Ok, I've been trying various things. The immediate cause of the issue is that _require_from_serialized tells worker processes to load a package without respecting package_locks. That can be fixed pretty easily, but then we hit the more fundamental issue that there's no synchronization mechanism for precompilation itself --- we don't have a way to know whether another node is precompiling a package, and if so, wait for it to finish. So I think a full fix for this requires fixing #17320. We should probably merge this PR now, and aim to fix #17320 in 0.6.1.

tkelman · 2017-05-14T16:18:19Z

this should have been squashed, the intermediate commits fail tests

… calls (cherry picked from commit 66b8036) ref #21718 don't insert toplevel expression unless there are imports (cherry picked from commit 18291b0) eliminate unnecessary array allocations from isexpr and extract_imports functions (cherry picked from commit 305037a) test fixes (cherry picked from commit 24a74a5)

pwl · 2017-07-13T10:30:09Z

After upgrading to 0.6 I started having

julia> f()=@everywhere using Base.Test
f (generic function with 1 method)

julia> f()
ERROR: error compiling f: unsupported or misplaced expression "toplevel" in function f
Stacktrace:
 [1] macro expansion at ./REPL.jl:97 [inlined]
 [2] (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:73

Is the above error intended or is it a bug?

@iblis17 suggested to use f()=@eval @everywhere using Base.Test instead, which fixed the issue.

stevengj · 2017-07-13T11:34:04Z

@pwl, you can't do a using statement inside a function. It really shouldn't have worked before.

pwl · 2017-07-13T12:31:11Z

@stevengj you are totally right, my mistake. In any case, the error is rather cryptic, without the @everywhere I'm getting ERROR: error compiling f: unsupported or misplaced expression "using" in function f, which is more explicit.

kshyatt added modules parallelism Parallel or distributed computation labels May 5, 2017

kshyatt requested a review from amitmurthy May 7, 2017 22:28

timholy reviewed May 8, 2017

View reviewed changes

amitmurthy mentioned this pull request May 8, 2017

Optimize remotecall pattern used in distributed loading #21745

Merged

stevengj added 3 commits May 8, 2017 17:05

make at-everywhere import modules first on node 1 before doing remote…

66b8036

… calls

don't insert toplevel expression unless there are imports

18291b0

eliminate unnecessary array allocations from isexpr and extract_impor…

305037a

…ts functions

stevengj force-pushed the everywhereusing branch from 3452589 to 305037a Compare May 8, 2017 21:07

JeffBezanson reviewed May 9, 2017

View reviewed changes

test fixes

24a74a5

timholy approved these changes May 9, 2017

View reviewed changes

JeffBezanson added the backport pending 0.6 label May 12, 2017

JeffBezanson merged commit f92387b into JuliaLang:master May 12, 2017

stevengj deleted the everywhereusing branch May 12, 2017 14:20

tkelman removed the backport pending 0.6 label May 18, 2017

amitmurthy mentioned this pull request May 22, 2017

@everywhere using loads module twice on the workers #19187

Closed

stevengj mentioned this pull request Jun 29, 2017

Crashing with @everywhere using #12381

Closed

simonbyrne mentioned this pull request Oct 6, 2017

Running parallel MPI RCall JuliaInterop/RCall.jl#204

Closed

vchuravy mentioned this pull request Nov 13, 2019

Surprising and undocumented import handling in Distributed.@everywhere JuliaLang/Distributed.jl#65

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make at-everywhere import modules before remote calls #21718

make at-everywhere import modules before remote calls #21718

stevengj commented May 5, 2017 •

edited

Loading

amitmurthy commented May 8, 2017

timholy commented May 8, 2017

timholy May 8, 2017

stevengj May 8, 2017

StefanKarpinski commented May 8, 2017 •

edited

Loading

amitmurthy commented May 8, 2017

stevengj commented May 8, 2017

JeffBezanson May 9, 2017

stevengj May 9, 2017

stevengj commented May 10, 2017

JeffBezanson commented May 11, 2017

amitmurthy commented May 11, 2017

amitmurthy commented May 11, 2017

JeffBezanson commented May 11, 2017

JeffBezanson commented May 12, 2017

tkelman commented May 14, 2017

pwl commented Jul 13, 2017

stevengj commented Jul 13, 2017

pwl commented Jul 13, 2017

make at-everywhere import modules before remote calls #21718

make at-everywhere import modules before remote calls #21718

Conversation

stevengj commented May 5, 2017 • edited Loading

amitmurthy commented May 8, 2017

timholy commented May 8, 2017

timholy May 8, 2017

Choose a reason for hiding this comment

stevengj May 8, 2017

Choose a reason for hiding this comment

StefanKarpinski commented May 8, 2017 • edited Loading

amitmurthy commented May 8, 2017

stevengj commented May 8, 2017

JeffBezanson May 9, 2017

Choose a reason for hiding this comment

stevengj May 9, 2017

Choose a reason for hiding this comment

stevengj commented May 10, 2017

JeffBezanson commented May 11, 2017

amitmurthy commented May 11, 2017

amitmurthy commented May 11, 2017

JeffBezanson commented May 11, 2017

JeffBezanson commented May 12, 2017

tkelman commented May 14, 2017

pwl commented Jul 13, 2017

stevengj commented Jul 13, 2017

pwl commented Jul 13, 2017

stevengj commented May 5, 2017 •

edited

Loading

StefanKarpinski commented May 8, 2017 •

edited

Loading