-
Notifications
You must be signed in to change notification settings - Fork 326
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
Convert Array_Like_Helpers.map
to a builtin to reduce stack size
#11363
Merged
Merged
Changes from 17 commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
1a0f946
[WIP] Add Stack_Size_Spec
Akirathan 695eeb3
Problem_Behavior is builtin type
Akirathan 8753d07
No_Wrap is builtin type
Akirathan 400f0fb
vector_from_function is builtin method
Akirathan a0fb709
vector_from_function accepts No_Wrap as onProblems argument
Akirathan 0487d00
Stack_Size_Spec will be a regression test
Akirathan 6e0c7f8
Report_Error unwraps payload
Akirathan 935fbdc
Report_Error returns DataflowError
Akirathan 3e32eb4
No_Wrap immediately returns error
Akirathan d1eb427
Report_Warning correctly attaches warning
Akirathan f026654
Attach additional warnings when limit is reached
Akirathan 1eb8edc
Remove unused code
Akirathan 847c496
vector_from_function is builtin
Akirathan 990ee6a
Convert Stack_Size_Spec to (regression) test
Akirathan 6de3d1b
Add clue to the test
Akirathan e0063aa
Vector.map and Array_Like_Helpers.map are tail calls
Akirathan 9c3d18f
Revert "Set stack size to 16M (#11327)"
Akirathan 544dfc7
Merge branch 'develop' into wip/akirathan/11329-map-reduce-stack
Akirathan 6e23ed6
Fix after merge
Akirathan ece75fb
Builtins.getBuiltinType method call is behind truffle boundary
Akirathan e76a6bd
Remove NeverDefault warning
Akirathan e16961f
MetaObjectTest.compareQualifiedAndSimpleTypeName skips singleton types
Akirathan bf28a12
singleton builtin types are skipped in other tests as well
Akirathan ea6b09f
Add instances of Problem_Behavior to meta tests
Akirathan 204d5fb
No_Wrap and Problem_Behavior are builtins in EnsoContext
Akirathan fcbb60a
type error panic is hidden behind TruffleBoundary
Akirathan 3949ed8
Stack_Size_Spec runs only on LInux
Akirathan eb38c35
Fix native image run tests - add java.lang.RuntimeException to NI
Akirathan a445a31
Skip singleton builtin types in MetaIsATest
Akirathan 89b5e59
Cache onProblems parameter
Akirathan 6ac51d7
Add filtering for benchmarks
Akirathan e13a77a
Add docs for tracing compilation for runtime-benchmarks
Akirathan ab27a4d
Close context in TypePatternBenchmarks
Akirathan 5b96444
No_Wrap is an uniquely constructible builtin
Akirathan d2dcc04
VectorFromFunctionNode caches onProblems atom ctor and uses loop profile
Akirathan dee9abe
Merge branch 'develop' into wip/akirathan/11329-map-reduce-stack
Akirathan cd3e04b
Add value for No_Wrap to meta tests
Akirathan 2cbc1e9
onProblems is No_Wrap.Value
Akirathan 7f22096
Merge branch 'develop' into wip/akirathan/11329-map-reduce-stack
Akirathan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
COMP_PATH=$(dirname "$0")/../component | ||
|
||
JAVA_OPTS="--add-opens=java.base/java.nio=ALL-UNNAMED -Xss16M $JAVA_OPTS" | ||
JAVA_OPTS="--add-opens=java.base/java.nio=ALL-UNNAMED $JAVA_OPTS" | ||
exec java --module-path $COMP_PATH $JAVA_OPTS -m org.enso.runner/org.enso.runner.Main "$@" | ||
exit |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
@echo off | ||
set comp-dir=%~dp0\..\component | ||
set JAVA_OPTS=%JAVA_OPTS% --add-opens=java.base/java.nio=ALL-UNNAMED -Xss16M | ||
set JAVA_OPTS=%JAVA_OPTS% --add-opens=java.base/java.nio=ALL-UNNAMED | ||
java --module-path %comp-dir% -Dpolyglot.compiler.IterativePartialEscape=true %JAVA_OPTS% -m org.enso.runner/org.enso.runner.Main %* | ||
exit /B %errorlevel% |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 19 additions & 0 deletions
19
.../src/main/java/org/enso/interpreter/node/expression/builtin/error/AdditionalWarnings.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package org.enso.interpreter.node.expression.builtin.error; | ||
|
||
import java.util.List; | ||
import org.enso.interpreter.dsl.BuiltinType; | ||
import org.enso.interpreter.node.expression.builtin.UniquelyConstructibleBuiltin; | ||
|
||
@BuiltinType | ||
public class AdditionalWarnings extends UniquelyConstructibleBuiltin { | ||
|
||
@Override | ||
protected String getConstructorName() { | ||
return "Error"; | ||
} | ||
|
||
@Override | ||
protected List<String> getConstructorParamNames() { | ||
return List.of("count"); | ||
} | ||
} |
9 changes: 9 additions & 0 deletions
9
engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/NoWrap.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package org.enso.interpreter.node.expression.builtin.error; | ||
|
||
import org.enso.interpreter.dsl.BuiltinType; | ||
import org.enso.interpreter.node.expression.builtin.Builtin; | ||
|
||
@BuiltinType(name = "Standard.Base.Data.Vector.No_Wrap") | ||
public class NoWrap extends Builtin { | ||
// Empty on purpose - does not have any constructors | ||
} |
27 changes: 27 additions & 0 deletions
27
...ime/src/main/java/org/enso/interpreter/node/expression/builtin/error/ProblemBehavior.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package org.enso.interpreter.node.expression.builtin.error; | ||
|
||
import java.util.List; | ||
import org.enso.interpreter.dsl.BuiltinType; | ||
import org.enso.interpreter.node.expression.builtin.Builtin; | ||
import org.enso.interpreter.runtime.data.atom.AtomConstructor; | ||
|
||
@BuiltinType(name = "Standard.Base.Errors.Problem_Behavior.Problem_Behavior") | ||
public class ProblemBehavior extends Builtin { | ||
|
||
@Override | ||
protected List<Cons> getDeclaredConstructors() { | ||
return List.of(new Cons("Ignore"), new Cons("Report_Warning"), new Cons("Report_Error")); | ||
} | ||
|
||
public AtomConstructor getIgnore() { | ||
return getConstructors()[0]; | ||
} | ||
|
||
public AtomConstructor getReportWarning() { | ||
return getConstructors()[1]; | ||
} | ||
|
||
public AtomConstructor getReportError() { | ||
return getConstructors()[2]; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
150 changes: 150 additions & 0 deletions
150
...untime/src/main/java/org/enso/interpreter/runtime/data/vector/VectorFromFunctionNode.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
package org.enso.interpreter.runtime.data.vector; | ||
|
||
import com.oracle.truffle.api.dsl.Cached; | ||
import com.oracle.truffle.api.dsl.Specialization; | ||
import com.oracle.truffle.api.frame.VirtualFrame; | ||
import com.oracle.truffle.api.library.CachedLibrary; | ||
import com.oracle.truffle.api.nodes.Node; | ||
import com.oracle.truffle.api.profiles.BranchProfile; | ||
import org.enso.interpreter.dsl.BuiltinMethod; | ||
import org.enso.interpreter.node.callable.dispatch.InvokeFunctionNode; | ||
import org.enso.interpreter.node.expression.builtin.error.AdditionalWarnings; | ||
import org.enso.interpreter.node.expression.builtin.error.NoWrap; | ||
import org.enso.interpreter.node.expression.builtin.error.ProblemBehavior; | ||
import org.enso.interpreter.runtime.EnsoContext; | ||
import org.enso.interpreter.runtime.callable.function.Function; | ||
import org.enso.interpreter.runtime.data.atom.Atom; | ||
import org.enso.interpreter.runtime.error.DataflowError; | ||
import org.enso.interpreter.runtime.error.PanicException; | ||
import org.enso.interpreter.runtime.library.dispatch.TypesLibrary; | ||
import org.enso.interpreter.runtime.state.State; | ||
import org.enso.interpreter.runtime.warning.AppendWarningNode; | ||
import org.enso.interpreter.runtime.warning.Warning; | ||
import org.enso.interpreter.runtime.warning.WarningsLibrary; | ||
|
||
@BuiltinMethod( | ||
type = "Array_Like_Helpers", | ||
name = "vector_from_function", | ||
description = "Creates a vector from a function.") | ||
public abstract class VectorFromFunctionNode extends Node { | ||
public static VectorFromFunctionNode build() { | ||
return VectorFromFunctionNodeGen.create(); | ||
} | ||
|
||
private static final int MAX_MAP_WARNINGS = 10; | ||
|
||
/** | ||
* @param length Length of the vector to create. | ||
* @param func Callback function called with index as argument. | ||
* @param onProblems Can be either an atom of type {@code Problem_Behavior} or {@code No_Wrap} | ||
* type. | ||
* @return Vector constructed from the given function. | ||
*/ | ||
abstract Object execute( | ||
VirtualFrame frame, State state, long length, Function func, Object onProblems); | ||
|
||
@Specialization | ||
Object doIt( | ||
VirtualFrame frame, | ||
State state, | ||
long length, | ||
Function func, | ||
Object onProblemsAtom, | ||
@Cached("buildWithArity(1)") InvokeFunctionNode invokeFunctionNode, | ||
@Cached("build()") AppendWarningNode appendWarningNode, | ||
@CachedLibrary(limit = "3") WarningsLibrary warnsLib, | ||
@CachedLibrary(limit = "3") TypesLibrary typesLib, | ||
@Cached BranchProfile errorEncounteredProfile) { | ||
var ctx = EnsoContext.get(this); | ||
var onProblems = processOnProblemsArg(onProblemsAtom, typesLib); | ||
var len = Math.toIntExact(length); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What will happen on |
||
var nothing = ctx.getNothing(); | ||
var target = ArrayBuilder.newBuilder(len); | ||
var errorsEncountered = 0; | ||
for (int i = 0; i < len; i++) { | ||
var value = invokeFunctionNode.execute(func, frame, state, new Long[] {(long) i}); | ||
Object valueToAdd = value; | ||
if (value instanceof DataflowError err) { | ||
errorEncounteredProfile.enter(); | ||
switch (onProblems) { | ||
Akirathan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
case IGNORE -> valueToAdd = nothing; | ||
case REPORT_ERROR -> { | ||
var mapErr = ctx.getBuiltins().error().makeMapError(i, err.getPayload()); | ||
return DataflowError.withDefaultTrace(state, mapErr, this); | ||
} | ||
case REPORT_WARNING -> { | ||
errorsEncountered++; | ||
if (errorsEncountered > MAX_MAP_WARNINGS) { | ||
valueToAdd = nothing; | ||
} else { | ||
var wrappedInWarn = | ||
Warning.attach(ctx, nothing, err.getPayload(), null, appendWarningNode); | ||
valueToAdd = wrappedInWarn; | ||
} | ||
} | ||
case NO_WRAP -> { | ||
return err; | ||
} | ||
} | ||
} | ||
target.add(valueToAdd, warnsLib); | ||
} | ||
var vector = target.asVector(true); | ||
if (errorsEncountered >= MAX_MAP_WARNINGS) { | ||
var additionalWarnsBuiltin = ctx.getBuiltins().getBuiltinType(AdditionalWarnings.class); | ||
long additionalWarnsCnt = errorsEncountered - MAX_MAP_WARNINGS; | ||
var additionalWarns = additionalWarnsBuiltin.newInstance(additionalWarnsCnt); | ||
var vecWithAdditionalWarns = | ||
Warning.attach(ctx, vector, additionalWarns, null, appendWarningNode); | ||
return vecWithAdditionalWarns; | ||
} else { | ||
return vector; | ||
} | ||
} | ||
|
||
private OnProblems processOnProblemsArg(Object onProblems, TypesLibrary typesLib) { | ||
var ctx = EnsoContext.get(this); | ||
var problemBehaviorBuiltin = ctx.getBuiltins().getBuiltinType(ProblemBehavior.class); | ||
var noWrapBuiltin = ctx.getBuiltins().getBuiltinType(NoWrap.class); | ||
var typeError = | ||
Akirathan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ctx.getBuiltins() | ||
.error() | ||
.makeTypeError(problemBehaviorBuiltin.getType(), onProblems, "onProblems"); | ||
if (onProblems instanceof Atom onProblemsAtom) { | ||
if (isIgnore(onProblemsAtom, problemBehaviorBuiltin)) { | ||
return OnProblems.IGNORE; | ||
} else if (isReportError(onProblemsAtom, problemBehaviorBuiltin)) { | ||
return OnProblems.REPORT_ERROR; | ||
} else if (isReportWarning(onProblemsAtom, problemBehaviorBuiltin)) { | ||
return OnProblems.REPORT_WARNING; | ||
} | ||
} | ||
if (!typesLib.hasType(onProblems)) { | ||
throw new PanicException(typeError, this); | ||
} | ||
var onProblemsType = typesLib.getType(onProblems); | ||
if (onProblemsType == noWrapBuiltin.getType()) { | ||
return OnProblems.NO_WRAP; | ||
} | ||
throw new PanicException(typeError, this); | ||
} | ||
|
||
private static boolean isReportWarning(Atom onProblems, ProblemBehavior problemBehaviorBuiltin) { | ||
return onProblems.getConstructor() == problemBehaviorBuiltin.getReportWarning(); | ||
} | ||
|
||
private static boolean isReportError(Atom onProblems, ProblemBehavior problemBehaviorBuiltin) { | ||
return onProblems.getConstructor() == problemBehaviorBuiltin.getReportError(); | ||
} | ||
|
||
private static boolean isIgnore(Atom onProblems, ProblemBehavior problemBehaviorBuiltin) { | ||
return onProblems.getConstructor() == problemBehaviorBuiltin.getIgnore(); | ||
} | ||
|
||
private enum OnProblems { | ||
IGNORE, | ||
REPORT_ERROR, | ||
REPORT_WARNING, | ||
NO_WRAP | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off-topic, but I'd like to know the answer: What is
name
attribute optional? It is specified on some of the builtins, but not on all. Shouldn't it be specified all the time? I am asking because ofStandard.Prelude
needed asmain = 6 * 7
no longer works #8852the proposed
Standard.Prelude
should include all the builtins and their methods, but we want to have their names exactly the same as inStandard.Base
- can we generate such aPrelude.enso
from these annotations?