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

Auto Parallelism #2000

Merged
merged 8 commits into from
Sep 27, 2021
Merged

Auto Parallelism #2000

merged 8 commits into from
Sep 27, 2021

Conversation

kustosz
Copy link
Contributor

@kustosz kustosz commented Sep 23, 2021

Pull Request Description

Introduces a rudimentary version of auto parallelism

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Scala, Java, and Rust style guides.
  • All documentation and configuration conforms to the markdown and YAML style guides.
  • All code has been tested where possible.

static method resolution

type analysis applied

build the graph

assign blocks

cleanup

IRGen

codegen
@kustosz kustosz marked this pull request as ready for review September 24, 2021 10:58
Copy link
Contributor

@iamrecursion iamrecursion left a comment

Choose a reason for hiding this comment

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

Code is good. Some minor readability comments.

I've verified that this doesn't seem to impact any of the compilation and caching work.

RELEASES.md Outdated
@@ -1,5 +1,11 @@
# Enso Next

## Interpreter/Runtime

- Added support for automatic parallelization of computations
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Added support for automatic parallelization of computations
- Added experimental support for automatic parallelization of computations

@@ -56,6 +56,7 @@ object Main {
private val UPLOAD_OPTION = "upload"
private val HIDE_PROGRESS = "hide-progress"
private val AUTH_TOKEN = "auth-token"
private val AUTO_PARALLELISM_OPTION = "with-auto-parallelism"
Copy link
Contributor

Choose a reason for hiding this comment

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

Alignment.

Comment on lines 816 to 822
moduleIr.bindings.find {
case method: IR.Module.Scope.Definition.Method.Explicit =>
method.methodReference.methodName.name == this.method.name && method.methodReference.typePointer
.getMetadata(MethodDefinitions)
.contains(Resolution(ResolvedModule(module)))
case _ => false
}.get
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be unsafeGetIr and you probably want a safe version returning Option[Definition].

Comment on lines 81 to 82
* followed by a series of pinnedd lines.
* @param parallelizable the parallelizable lines
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* followed by a series of pinnedd lines.
* @param parallelizable the parallelizable lines
* followed by a series of pinned lines.
*
* @param parallelizable the parallelizable lines

Comment on lines 91 to 92
* Computes the transitive closure of the dependency info.
* @param segment the segment containing only immediate dependency info.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Computes the transitive closure of the dependency info.
* @param segment the segment containing only immediate dependency info.
* Computes the transitive closure of the dependency info.
*
* @param segment the segment containing only immediate dependency info.

Comment on lines 115 to 116
* parallelizable line to a block.
* @param segment the segment for which assignment should be done.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* parallelizable line to a block.
* @param segment the segment for which assignment should be done.
* parallelizable line to a block.
*
* @param segment the segment for which assignment should be done.

// Going backwards through the lines (i.e. post-order of the dependency
// graph), assign each not-yet-assigned parallelized line to a new thread.
// Also move all its dependencies to the new thread (or promote to current
// thread, see #moveTo.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// thread, see #moveTo.
// thread), see #moveTo.

Comment on lines 176 to 177
* Splits a block into sub-blocks based on the parallelism status.
* @param exprs the exprs to split, together with their status.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Splits a block into sub-blocks based on the parallelism status.
* @param exprs the exprs to split, together with their status.
* Splits a block into sub-blocks based on the parallelism status.
*
* @param exprs the exprs to split, together with their status.

@kustosz kustosz merged commit f4823f0 into main Sep 27, 2021
@kustosz kustosz deleted the wip/mk/auto-parallel branch September 27, 2021 22:48
@kustosz kustosz restored the wip/mk/auto-parallel branch September 28, 2021 08:59
@iamrecursion iamrecursion deleted the wip/mk/auto-parallel branch September 28, 2021 12:00
4e6 pushed a commit that referenced this pull request Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants