Remove localize_vars. Serialize global values under Main.

[amitmurthy]

Closes:
#11228
#8591

Fixes #2669 (comment)

The following can now be fixed by automatic serialization of globals (as mentioned in issue #19000) :
#5930
#12367
#14399
#6760

This helps in removing some of the scoping issues w.r.t. parallel macros. A strategy to track and automatic serialization of globals should help in removing the rest.

Till automatic serialization of globals is in place, this will break existing code that would work with globals due to the transparent localize_vars call.

We can discuss and merge this now or wait till we have a strategy to deal with globals and merge them together. A reason for delaying a merge would be to preempt issues being reported of globals not being serialized.

[andreasnoack]

#11228 already seemed to be fixed on 0.5. At least I couldn't see a timing difference between the two examples.

[JeffBezanson]

Looks good so far. On this branch, let's also (1) remove the localize_vars machinery completely, (2) fix globals, so we have a complete fix.

[bjarthur]

will ge great to have this. see also #19468 #19558 #18227

[amitmurthy]

remove the localize_vars machinery completely

Have removed find_vars. There are also references to localize in macroexpand.scm and in the generated julia_flisp.boot - should I remove the localize section macroexpand.scm?

fix globals

Need some help here. Tried to follow the serialization process via debug statements. They are represented as GlobalRefs which are effectively symbols. How do I dereference and check on the object pointed to?

[amitmurthy]

getfield(Module, Symbol) should work right? I'll take a shot at it.

[tkelman]

If any packages are calling these, we should probably deprecate them instead of deleting immediately.

[amitmurthy]

If any packages are calling these, we should probably deprecate them instead of deleting immediately.

It is safe to remove. localize_vars is an undocumented, internal function and AFAIK not used outside Base.

On the serialization of globals topic, the scenario where we are only sending an identifier for previously sent anonymous functions has yet to be handled. In this case serialize(s, ::GlobalRef) is not invoked. We will need to track global refs within an anon function and send them over if their hash/object_id has changed even in the case where only the cluster unique object number is sent.

[JeffBezanson]

Yes, also remove the code in macroexpand.scm.

[amitmurthy]

Serialization of globals has been implemented as follows:

• Applicable only when running under a Julia cluster and interacting with workers.
  Does not happen with the default implementation, for example directly serializing to an IOBuffer.

• Only globals under Main are serialized.

• There is no caching/attempt to keep the same global symbols in sync across nodes.
  For example, If 1 sends Main.foo to workers 2 and 3, and then 3 updates its Main.foo and sends it to 2, 1 will continue to have the old value.

• Only globals referenced in anonymous functions are sent. As the parallel macros and remotecall* functions all create a thunk for serialization, this is not an issue. However, as complete type data for TypeName objects which are not anonymous functions, are currently not serialized, the following does not work:

  foo() = 1
  remotecall_fetch(foo, 2)
  

  while this works:

  bar = ()->1
  remotecall_fetch(bar, 2)
  

  We need to support the former too. Will do away with the need to perform an @everywhere for new function definitions under Main. It should also handle the case if foo is redefined on the caller node subject to the caveat that no attempt is made to keep definitions in sync across the cluster.

  I am not familiar with the internal structure of function definitions and TypeName, as also how to detect if named functions are changed (edit: both hash and object_id are unchanged for redefined functions). Would need help or we can keep this part open as an issue and fix in a later PR.

• Bitstypes are always sent to the remote node.

• Non-bitstypes are sent the first time and consequently only if the hash is different (edit: or the object_id has changed).

• The current impl tracks globals referenced in TypeName. A previous impl did so by handling
  ser/deser of GlobalRefs directly. I feel the current impl is cleaner and simpler. The older impl can be seen at commit ba65424.

[amitmurthy]

Ready for review. Will update NEWS and add a section to the manual after review.

cc: @JeffBezanson , @vtjnash

[tkelman]

did you intend to commit this? better to keep local and add to gitignore?

[amitmurthy]

Oops! Will remove it. Thanks for catching it.

[tkelman]

maybe a good addition to contrib in a separate PR, if vscode can be made to look at settings files in different locations?

[amitmurthy]

Sure.

[tkelman]

this still needs to be deleted

[ViralBShah]

Bump @JeffBezanson @vtjnash for a review.

[amitmurthy]

Bumping again. We should really get this in in 0.6 . Will address a wide range of parallel issues.

[StefanKarpinski]

I took a crack at resolving the conflict here by just keeping everything from the PR and master.

[tkelman]

this still needs to be deleted

[amitmurthy]

Squashed and rebased. @JeffBezanson can you give it one more look-over and merge?

[amitmurthy]

OSX error in spawn test appears to be unrelated to this PR.

[StefanKarpinski]

MacOS failure might be a timeout? It's a bit hard to tell.

[amitmurthy]

Restarted the job. Old log at https://gist.github.com/amitmurthy/1f4597802dd08720e962fb92b0d9ca0e

[StefanKarpinski]

@JeffBezanson, this just needs you to take one last look and merge.

[bdeonovic]

Is it possible to test julia with this commit in v0.5?

[amitmurthy]

It should merge cleanly onto 0.5 - at least the files in base/. The PR however will not be officially backported onto v0.5 as it involves a change in behavior.