Skip to content
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

RFC: Split multi.jl into multiple files. Wrap parallel into its own module. #19056

Closed
wants to merge 1 commit into from

Conversation

amitmurthy
Copy link
Contributor

A first step in refactoring and making multi.jl more modular with cleaner separation of functionality. This PR splits multi.jl into smaller files and moves all multiprocessing code into a new module Base.Parallel

@kshyatt kshyatt added parallelism Parallel or distributed computation multithreading Base.Threads and related functionality labels Oct 22, 2016
@amitmurthy amitmurthy removed the multithreading Base.Threads and related functionality label Oct 23, 2016
@amitmurthy
Copy link
Contributor Author

Any thoughts on this? Any reasons to NOT have a Parallel submodule.

@vchuravy
Copy link
Member

👍 from me.

@amitmurthy
Copy link
Contributor Author

Will merge this tomorrow.

@ViralBShah
Copy link
Member

ViralBShah commented Oct 24, 2016

Now that you ask, there could be a separate module called RPC (for remotecall, fetch, Futures, etc.) that can be used to build any arbitrary distributed computing system and user-facing parallel computing models in a parallel module.

Not relevant here, but do we need to put Channels in their own module too?

@amitmurthy
Copy link
Contributor Author

@ViralBShah , I agree with you on differentiating between a higher level API (for users) and a lower level RPC API (for library writers). However, at this time I want to merge this as is and take some more time to debate the correct way of modularization Parallel and the interfaces between each of them.

Broadly, functionally I can think of

  • ClusterManagement - partially modularized via ClusterManagers.jl . Needs a rework to handle elastic computation scenarios better.
  • Serialization, Transport, Topology - Option to plugin other serialization schemes? Other IO infrastructure, routing via intermediate nodes, etc.
  • Communication primitives - point-to-point remote calls (existing), collective MPI style communication (not yet defined)
  • User facing API - pmap, distributed arrays, etc

Not relevant here, but do we need to put Channels in their own module too?

IMO, Channels and all task related code should be grouped together under module Tasks

@vtjnash
Copy link
Member

vtjnash commented Oct 24, 2016

Any reasons to NOT have a Parallel submodule

In order to make this a toplevel module? 😛

But seriously, would it be difficult to make this a toplevel module? I think using Parallel would be nicer than using Base.Parallel, and would better align with making this a stdlib module.

# * integrate event loop with other kinds of i/o (non-messages)
# * recover from i/o errors
# * all-to-all communication
# * dynamically adding nodes (then always start with 1 and grow)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can delete this list (* means done)

@vchuravy
Copy link
Member

@vtjnash That is the long term-goal, but should we start moving bits and pieces out before we have the infrastructure for stdlib?

@vtjnash
Copy link
Member

vtjnash commented Oct 25, 2016

We should be able to make them toplevel modules without yet changing anything about the stdlib infrastructures. I expect this may be one of the harder ones to detangle, however. The PR here will be a useful start to that process.

@amitmurthy
Copy link
Contributor Author

As a first step, I'll just rebase and merge as is. Will take a shot at making it a top-level module in a different PR.

The main issues in distentangling that I see are:

@amitmurthy
Copy link
Contributor Author

I am going to rework this again in a few days to make it mergeable. Would like to get it in in the 0.6 timeframe. No user facing functionality change. Just code reorg.

@amitmurthy amitmurthy added this to the 0.6.0 milestone Jan 5, 2017
@tkelman
Copy link
Contributor

tkelman commented Jan 19, 2017

still planning on doing this?

@amitmurthy
Copy link
Contributor Author

Yes. I am waiting for other PRs to get in / not get in before redoing this PR.

@StefanKarpinski
Copy link
Member

You may want to refresh this change so it can be merged since it's not externally visible and then rebase the parallel for change on top of this.

@StefanKarpinski StefanKarpinski removed this from the 0.6.0 milestone Feb 2, 2017
@StefanKarpinski
Copy link
Member

Not externally visible – can be done whenever.

@amitmurthy
Copy link
Contributor Author

Closing in favor of #20428

As mentioned there, we should get this in soon. Will help in backporting bugfixes onto 0.6.

@amitmurthy amitmurthy closed this Feb 3, 2017
@amitmurthy amitmurthy deleted the amitm/split_multi branch February 3, 2017 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parallelism Parallel or distributed computation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants