mirror of
https://github.com/enso-org/enso.git
synced 2024-11-30 05:35:09 +03:00
Fix inline compilation benchmarks and Improve searching Graph.Link performance (#9520)
As benchmarks show, a significant amount of time is spent traversing `Set` of `Graph.Link`s. That's unfortunate and unnecessary. We can equally keep helper maps that make search constant time. Fixed inline compilation benchmarks by properly cleaning up scopes after runs. Closes #9237. # Important Notes Things like ![Screenshot from 2024-03-22 11-23-01](https://github.com/enso-org/enso/assets/292128/7c1e220a-6e33-4396-a9b2-0e788f615323) ![Screenshot from 2024-03-22 11-13-19](https://github.com/enso-org/enso/assets/292128/0272b1cb-252e-4662-b539-174844941c8e) are all gone. There is plenty of it those are just samples. Benchmarks are back in order: ``` [info] # Warmup Iteration 1: 2.702 ms/op [info] # Warmup Iteration 2: 3.080 ms/op [info] # Warmup Iteration 3: 2.818 ms/op [info] # Warmup Iteration 4: 3.334 ms/op [info] # Warmup Iteration 5: 2.448 ms/op [info] # Warmup Iteration 6: 2.583 ms/op [info] Iteration 1: 2.908 ms/op [info] Iteration 2: 2.915 ms/op [info] Iteration 3: 2.774 ms/op [info] Iteration 4: 2.601 ms/op [info] Result "org.enso.compiler.benchmarks.inline.InlineCompilerBenchmark.longExpression": [info] 2.799 ±(99.9%) 0.953 ms/op [Average] [info] (min, avg, max) = (2.601, 2.799, 2.915), stdev = 0.148 [info] CI (99.9%): [1.846, 3.753] (assumes normal distribution) ```
This commit is contained in:
parent
207c7af136
commit
1c724a4c56
@ -3,10 +3,10 @@ package org.enso.compiler.benchmarks.inline;
|
||||
import java.io.ByteArrayOutputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.OutputStream;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import org.enso.compiler.Compiler;
|
||||
import org.enso.compiler.benchmarks.Utils;
|
||||
import org.enso.compiler.context.InlineContext;
|
||||
import org.graalvm.polyglot.Context;
|
||||
import org.openjdk.jmh.annotations.Benchmark;
|
||||
import org.openjdk.jmh.annotations.BenchmarkMode;
|
||||
@ -46,19 +46,18 @@ public class InlineCompilerBenchmark {
|
||||
private final OutputStream out = new ByteArrayOutputStream();
|
||||
private Compiler compiler;
|
||||
private Context ctx;
|
||||
private InlineContext mainInlineContext;
|
||||
private InlineSource inlineSource;
|
||||
private String longExpression;
|
||||
private Set<String> localVarNames;
|
||||
|
||||
@Setup
|
||||
public void setup() throws IOException {
|
||||
ctx = Utils.createDefaultContextBuilder().out(out).err(out).logHandler(out).build();
|
||||
var ensoCtx = Utils.leakEnsoContext(ctx);
|
||||
compiler = ensoCtx.getCompiler();
|
||||
|
||||
var inlineSource = InlineContextUtils.createMainMethodWithLocalVars(ctx, LOCAL_VARS_CNT);
|
||||
mainInlineContext = inlineSource.mainInlineContext();
|
||||
longExpression =
|
||||
InlineContextUtils.createLongExpression(inlineSource.localVarNames(), LONG_EXPR_SIZE);
|
||||
localVarNames = InlineContextUtils.localVarNames(LOCAL_VARS_CNT);
|
||||
longExpression = InlineContextUtils.createLongExpression(localVarNames, LONG_EXPR_SIZE);
|
||||
inlineSource = InlineContextUtils.createMainMethodWithLocalVars(ctx, localVarNames);
|
||||
}
|
||||
|
||||
@TearDown
|
||||
@ -75,11 +74,13 @@ public class InlineCompilerBenchmark {
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void longExpression(Blackhole blackhole) {
|
||||
var tuppleOpt = compiler.runInline(longExpression, mainInlineContext);
|
||||
if (tuppleOpt.isEmpty()) {
|
||||
throw new AssertionError("Unexpected: inline compilation should succeed");
|
||||
public void longExpression(Blackhole blackhole) throws IOException {
|
||||
try (InlineContextResource resource = inlineSource.inlineContextFactory().create()) {
|
||||
var tuppleOpt = compiler.runInline(longExpression, resource.inlineContext());
|
||||
if (tuppleOpt.isEmpty()) {
|
||||
throw new AssertionError("Unexpected: inline compilation should succeed");
|
||||
}
|
||||
blackhole.consume(tuppleOpt);
|
||||
}
|
||||
blackhole.consume(tuppleOpt);
|
||||
}
|
||||
}
|
||||
|
@ -6,7 +6,6 @@ import java.io.OutputStream;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import org.enso.compiler.Compiler;
|
||||
import org.enso.compiler.benchmarks.Utils;
|
||||
import org.enso.compiler.context.InlineContext;
|
||||
import org.enso.polyglot.RuntimeOptions;
|
||||
import org.graalvm.polyglot.Context;
|
||||
import org.openjdk.jmh.annotations.Benchmark;
|
||||
@ -48,7 +47,7 @@ public class InlineCompilerErrorBenchmark {
|
||||
private Compiler compiler;
|
||||
|
||||
private Context ctx;
|
||||
private InlineContext mainInlineContext;
|
||||
private InlineContextResourceFactory mainInlineContextResourceFactory;
|
||||
private String expressionWithErrors;
|
||||
|
||||
@Setup
|
||||
@ -63,11 +62,12 @@ public class InlineCompilerErrorBenchmark {
|
||||
var ensoCtx = Utils.leakEnsoContext(ctx);
|
||||
compiler = ensoCtx.getCompiler();
|
||||
|
||||
var inlineSource = InlineContextUtils.createMainMethodWithLocalVars(ctx, LOCAL_VARS_CNT);
|
||||
var localVarNames = InlineContextUtils.localVarNames(LOCAL_VARS_CNT);
|
||||
var inlineSource = InlineContextUtils.createMainMethodWithLocalVars(ctx, localVarNames);
|
||||
var longExpression =
|
||||
InlineContextUtils.createLongExpression(inlineSource.localVarNames(), LONG_EXPR_SIZE);
|
||||
expressionWithErrors = "UNDEFINED * " + longExpression + " * UNDEFINED";
|
||||
mainInlineContext = inlineSource.mainInlineContext();
|
||||
mainInlineContextResourceFactory = inlineSource.inlineContextFactory();
|
||||
}
|
||||
|
||||
@TearDown
|
||||
@ -79,8 +79,10 @@ public class InlineCompilerErrorBenchmark {
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void expressionWithErrors(Blackhole blackhole) {
|
||||
var tuppleOpt = compiler.runInline(expressionWithErrors, mainInlineContext);
|
||||
blackhole.consume(tuppleOpt);
|
||||
public void expressionWithErrors(Blackhole blackhole) throws IOException {
|
||||
try (InlineContextResource resource = mainInlineContextResourceFactory.create()) {
|
||||
var tuppleOpt = compiler.runInline(expressionWithErrors, resource.inlineContext());
|
||||
blackhole.consume(tuppleOpt);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -0,0 +1,23 @@
|
||||
package org.enso.compiler.benchmarks.inline;
|
||||
|
||||
import java.io.IOException;
|
||||
import org.enso.compiler.context.InlineContext;
|
||||
|
||||
/**
|
||||
* InlineContextResource ensures that the underlying InlineContext is properly cleaned up after
|
||||
* usage.
|
||||
*
|
||||
* @param inlineContext InlineContext for the main method
|
||||
*/
|
||||
public record InlineContextResource(InlineContext inlineContext) implements AutoCloseable {
|
||||
@Override
|
||||
public void close() throws IOException {
|
||||
inlineContext
|
||||
.localScope()
|
||||
.foreach(
|
||||
s -> {
|
||||
s.scope().removeScopeFromParent();
|
||||
return null;
|
||||
});
|
||||
}
|
||||
}
|
@ -0,0 +1,28 @@
|
||||
package org.enso.compiler.benchmarks.inline;
|
||||
|
||||
import org.enso.compiler.PackageRepository;
|
||||
import org.enso.compiler.context.InlineContext;
|
||||
import org.enso.interpreter.node.MethodRootNode;
|
||||
import org.enso.interpreter.runtime.EnsoContext;
|
||||
import org.enso.interpreter.runtime.data.Type;
|
||||
import org.enso.interpreter.runtime.scope.ModuleScope;
|
||||
|
||||
public record InlineContextResourceFactory(
|
||||
ModuleScope moduleScope,
|
||||
Type assocTypeReceiver,
|
||||
EnsoContext ensoCtx,
|
||||
PackageRepository pkgRepository) {
|
||||
|
||||
public InlineContextResource create() {
|
||||
var mainFunc = moduleScope.getMethodForType(assocTypeReceiver, "main");
|
||||
var mainFuncRootNode = (MethodRootNode) mainFunc.getCallTarget().getRootNode();
|
||||
var mainLocalScope = mainFuncRootNode.getLocalScope();
|
||||
return new InlineContextResource(
|
||||
InlineContext.fromJava(
|
||||
mainLocalScope.createChild(),
|
||||
moduleScope.getModule().asCompilerModule(),
|
||||
scala.Option.apply(false),
|
||||
ensoCtx.getCompilerConfig(),
|
||||
scala.Option.apply(pkgRepository)));
|
||||
}
|
||||
}
|
@ -5,8 +5,6 @@ import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
import org.enso.compiler.benchmarks.CodeGenerator;
|
||||
import org.enso.compiler.benchmarks.Utils;
|
||||
import org.enso.compiler.context.InlineContext;
|
||||
import org.enso.interpreter.node.MethodRootNode;
|
||||
import org.enso.interpreter.runtime.data.Type;
|
||||
import org.enso.polyglot.LanguageInfo;
|
||||
import org.enso.polyglot.MethodNames;
|
||||
@ -16,6 +14,16 @@ import org.graalvm.polyglot.Source;
|
||||
class InlineContextUtils {
|
||||
private InlineContextUtils() {}
|
||||
|
||||
static Set<String> localVarNames(int localVarsCnt) {
|
||||
Set<String> localVarNames = new HashSet<>();
|
||||
|
||||
for (int i = 0; i < localVarsCnt; i++) {
|
||||
var varName = "loc_var_" + i;
|
||||
localVarNames.add(varName);
|
||||
}
|
||||
return localVarNames;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a main method, generates some local variables, and fills their identifiers in the given
|
||||
* set.
|
||||
@ -23,18 +31,15 @@ class InlineContextUtils {
|
||||
* @param localVarsCnt How many local variables should be initialized in the main method
|
||||
* @return Body of the main method
|
||||
*/
|
||||
static InlineSource createMainMethodWithLocalVars(Context ctx, int localVarsCnt)
|
||||
static InlineSource createMainMethodWithLocalVars(Context ctx, Set<String> localVarNames)
|
||||
throws IOException {
|
||||
var sb = new StringBuilder();
|
||||
sb.append("main = ").append(System.lineSeparator());
|
||||
var codeGen = new CodeGenerator();
|
||||
Set<String> localVarNames = new HashSet<>();
|
||||
for (int i = 0; i < localVarsCnt; i++) {
|
||||
var varName = "loc_var_" + i;
|
||||
localVarNames.add(varName);
|
||||
for (String localVarName : localVarNames) {
|
||||
var literal = codeGen.nextLiteral();
|
||||
sb.append(" ")
|
||||
.append(varName)
|
||||
.append(localVarName)
|
||||
.append(" = ")
|
||||
.append(literal)
|
||||
.append(System.lineSeparator());
|
||||
@ -51,18 +56,11 @@ class InlineContextUtils {
|
||||
var moduleAssocType = module.invokeMember(MethodNames.Module.GET_ASSOCIATED_TYPE);
|
||||
var assocTypeReceiver = (Type) Utils.unwrapReceiver(ctx, moduleAssocType);
|
||||
var moduleScope = assocTypeReceiver.getDefinitionScope();
|
||||
var mainFunc = moduleScope.getMethodForType(assocTypeReceiver, "main");
|
||||
var mainFuncRootNode = (MethodRootNode) mainFunc.getCallTarget().getRootNode();
|
||||
var mainLocalScope = mainFuncRootNode.getLocalScope();
|
||||
var compiler = ensoCtx.getCompiler();
|
||||
var mainInlineContext =
|
||||
InlineContext.fromJava(
|
||||
mainLocalScope,
|
||||
moduleScope.getModule().asCompilerModule(),
|
||||
scala.Option.apply(false),
|
||||
ensoCtx.getCompilerConfig(),
|
||||
scala.Option.apply(compiler.packageRepository()));
|
||||
return new InlineSource(sb.toString(), mainInlineContext, localVarNames);
|
||||
var inlineCtxMeta =
|
||||
new InlineContextResourceFactory(
|
||||
moduleScope, assocTypeReceiver, ensoCtx, compiler.packageRepository());
|
||||
return new InlineSource(sb.toString(), inlineCtxMeta, localVarNames);
|
||||
}
|
||||
|
||||
static String createLongExpression(Set<String> localVars, int exprSize) {
|
||||
|
@ -1,11 +1,10 @@
|
||||
package org.enso.compiler.benchmarks.inline;
|
||||
|
||||
import java.util.Set;
|
||||
import org.enso.compiler.context.InlineContext;
|
||||
|
||||
record InlineSource(
|
||||
String source,
|
||||
// InlineContext for the main method
|
||||
InlineContext mainInlineContext,
|
||||
// InlineContextResource for the main method
|
||||
InlineContextResourceFactory inlineContextFactory,
|
||||
// Local variables in main method
|
||||
Set<String> localVarNames) {}
|
||||
|
@ -166,7 +166,7 @@ public final class PassPersistance {
|
||||
|
||||
var links =
|
||||
(scala.collection.immutable.Set) in.readInline(scala.collection.immutable.Set.class);
|
||||
g.links_$eq(links);
|
||||
g.initLinks(links);
|
||||
|
||||
var nextIdCounter = in.readInt();
|
||||
g.nextIdCounter_$eq(nextIdCounter);
|
||||
@ -178,7 +178,7 @@ public final class PassPersistance {
|
||||
@Override
|
||||
protected void writeObject(Graph obj, Output out) throws IOException {
|
||||
out.writeObject(obj.rootScope());
|
||||
out.writeInline(scala.collection.immutable.Set.class, obj.links());
|
||||
out.writeInline(scala.collection.immutable.Set.class, obj.getLinks());
|
||||
out.writeInt(obj.nextIdCounter());
|
||||
}
|
||||
|
||||
|
@ -5,14 +5,17 @@ import org.enso.syntax.text.Debug
|
||||
import org.enso.compiler.pass.analyse.alias.Graph.{Occurrence, Scope}
|
||||
|
||||
import java.util.UUID
|
||||
import scala.collection.immutable.HashMap
|
||||
import scala.collection.mutable
|
||||
import scala.reflect.ClassTag
|
||||
|
||||
/** A graph containing aliasing information for a given root scope in Enso. */
|
||||
sealed class Graph extends Serializable {
|
||||
var rootScope: Graph.Scope = new Graph.Scope()
|
||||
var links: Set[Graph.Link] = Set()
|
||||
var nextIdCounter = 0
|
||||
var rootScope: Graph.Scope = new Graph.Scope()
|
||||
private var links: Set[Graph.Link] = Set()
|
||||
private var sourceLinks: Map[Graph.Id, Set[Graph.Link]] = new HashMap()
|
||||
private var targetLinks: Map[Graph.Id, Set[Graph.Link]] = new HashMap()
|
||||
var nextIdCounter = 0
|
||||
|
||||
private var globalSymbols: Map[Graph.Symbol, Occurrence.Global] =
|
||||
Map()
|
||||
@ -24,11 +27,22 @@ sealed class Graph extends Serializable {
|
||||
val copy = new Graph
|
||||
copy.rootScope = this.rootScope.deepCopy(scope_mapping)
|
||||
copy.links = this.links
|
||||
copy.sourceLinks = this.sourceLinks
|
||||
copy.targetLinks = this.targetLinks
|
||||
copy.globalSymbols = this.globalSymbols
|
||||
copy.nextIdCounter = this.nextIdCounter
|
||||
copy
|
||||
}
|
||||
|
||||
def initLinks(links: Set[Graph.Link]): Unit = {
|
||||
sourceLinks = new HashMap()
|
||||
targetLinks = new HashMap()
|
||||
links.foreach(addSourceTargetLink)
|
||||
this.links = links
|
||||
}
|
||||
|
||||
def getLinks(): Set[Graph.Link] = links
|
||||
|
||||
/** Registers a requested global symbol in the aliasing scope.
|
||||
*
|
||||
* @param sym the symbol occurrence
|
||||
@ -46,6 +60,8 @@ sealed class Graph extends Serializable {
|
||||
def copy: Graph = {
|
||||
val graph = new Graph
|
||||
graph.links = links
|
||||
graph.sourceLinks = sourceLinks
|
||||
graph.targetLinks = targetLinks
|
||||
graph.rootScope = rootScope.deepCopy(mutable.Map())
|
||||
graph.nextIdCounter = nextIdCounter
|
||||
|
||||
@ -85,10 +101,20 @@ sealed class Graph extends Serializable {
|
||||
): Option[Graph.Link] = {
|
||||
scopeFor(occurrence.id).flatMap(_.resolveUsage(occurrence).map { link =>
|
||||
links += link
|
||||
addSourceTargetLink(link)
|
||||
link
|
||||
})
|
||||
}
|
||||
|
||||
private def addSourceTargetLink(link: Graph.Link): Unit = {
|
||||
sourceLinks = sourceLinks.updatedWith(link.source)(v =>
|
||||
v.map(s => s + link).orElse(Some(Set(link)))
|
||||
)
|
||||
targetLinks = targetLinks.updatedWith(link.target)(v =>
|
||||
v.map(s => s + link).orElse(Some(Set(link)))
|
||||
)
|
||||
}
|
||||
|
||||
/** Resolves any links for the given usage of a symbol, assuming the symbol
|
||||
* is global (i.e. method, constructor etc.)
|
||||
*
|
||||
@ -129,7 +155,10 @@ sealed class Graph extends Serializable {
|
||||
* @return a list of links in which `id` occurs
|
||||
*/
|
||||
def linksFor(id: Graph.Id): Set[Graph.Link] = {
|
||||
links.filter(l => l.source == id || l.target == id)
|
||||
sourceLinks.getOrElse(id, Set.empty[Graph.Link]) ++ targetLinks.getOrElse(
|
||||
id,
|
||||
Set()
|
||||
)
|
||||
}
|
||||
|
||||
/** Finds all links in the graph where `symbol` appears in the role
|
||||
@ -646,6 +675,17 @@ object Graph {
|
||||
|
||||
isDirectChildOf || isChildOfChildren
|
||||
}
|
||||
|
||||
private def removeScopeFromParent(scope: Scope): Unit = {
|
||||
childScopes = childScopes.filter(_ != scope)
|
||||
}
|
||||
|
||||
/** Disassociates this Scope from its parent.
|
||||
*/
|
||||
def removeScopeFromParent(): Unit = {
|
||||
assert(this.parent.nonEmpty)
|
||||
this.parent.foreach(_.removeScopeFromParent(this))
|
||||
}
|
||||
}
|
||||
|
||||
/** A link in the [[Graph]].
|
||||
|
@ -457,11 +457,11 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
.unsafeAs[Info.Occurrence]
|
||||
.id
|
||||
|
||||
goodGraph.links should contain(Link(aUseId, 0, aDefId))
|
||||
goodGraph.getLinks() should contain(Link(aUseId, 0, aDefId))
|
||||
}
|
||||
|
||||
"enforce the ordering scope constraint on function arguments" in {
|
||||
badGraph.links shouldBe empty
|
||||
badGraph.getLinks() shouldBe empty
|
||||
}
|
||||
}
|
||||
|
||||
@ -484,7 +484,7 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
.unsafeAs[Info.Scope.Root]
|
||||
.graph
|
||||
|
||||
val graphLinks = methodWithLambdaGraph.links
|
||||
val graphLinks = methodWithLambdaGraph.getLinks()
|
||||
|
||||
val topLambda = methodWithLambda.body.asInstanceOf[Function.Lambda]
|
||||
val topLambdaBody = topLambda.body
|
||||
@ -863,7 +863,7 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
.unsafeGetMetadata(AliasAnalysis, "Missing aliasing info")
|
||||
.unsafeAs[Info.Scope.Root]
|
||||
.graph
|
||||
val graphLinks = graph.links
|
||||
val graphLinks = graph.getLinks()
|
||||
|
||||
val lambda = addMethod.body.asInstanceOf[Function.Lambda]
|
||||
|
||||
@ -933,7 +933,7 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
.unsafeGetMetadata(AliasAnalysis, "Missing aliasing info")
|
||||
.unsafeAs[Info.Scope.Root]
|
||||
.graph
|
||||
val graphLinks = graph.links
|
||||
val graphLinks = graph.getLinks()
|
||||
|
||||
val lambda = conversionMethod.body.asInstanceOf[Function.Lambda]
|
||||
val lambdaBody = lambda.body.asInstanceOf[Expression.Block]
|
||||
@ -1077,7 +1077,7 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
.id
|
||||
graph.rootScope.getOccurrence(scrutineeId) shouldBe defined
|
||||
|
||||
graph.links should contain(Link(scrutineeId, 0, scrutBindingId))
|
||||
graph.getLinks() should contain(Link(scrutineeId, 0, scrutBindingId))
|
||||
|
||||
val scrutBindingExprId = scrutBindingExpr
|
||||
.unsafeGetMetadata(AliasAnalysis, "")
|
||||
@ -1094,7 +1094,7 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
.id
|
||||
graph.rootScope.getOccurrence(aDefId) shouldBe defined
|
||||
|
||||
graph.links should contain(Link(scrutBindingExprId, 0, aDefId))
|
||||
graph.getLinks() should contain(Link(scrutBindingExprId, 0, aDefId))
|
||||
}
|
||||
|
||||
"create child scopes for the branch function" in {
|
||||
@ -1183,8 +1183,8 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
val bUseId =
|
||||
bUse.getMetadata(AliasAnalysis).get.unsafeAs[Info.Occurrence].id
|
||||
|
||||
graph.links should contain(Link(aUseId, 1, consBranchADefId))
|
||||
graph.links should contain(Link(bUseId, 1, consBranchBDefId))
|
||||
graph.getLinks() should contain(Link(aUseId, 1, consBranchADefId))
|
||||
graph.getLinks() should contain(Link(bUseId, 1, consBranchBDefId))
|
||||
}
|
||||
|
||||
"correctly link to pattern variables in type patterns" in {
|
||||
@ -1223,8 +1223,12 @@ class AliasAnalysisTest extends CompilerTest {
|
||||
val integerUseId =
|
||||
integerUse.getMetadata(AliasAnalysis).get.unsafeAs[Info.Occurrence].id
|
||||
|
||||
graph.links should contain(Link(numUseId, 1, tpeBranchNameDefId))
|
||||
graph.links should not contain (Link(integerUseId, 1, tpeBranchTpeDefId))
|
||||
graph.getLinks() should contain(Link(numUseId, 1, tpeBranchNameDefId))
|
||||
graph.getLinks() should not contain (Link(
|
||||
integerUseId,
|
||||
1,
|
||||
tpeBranchTpeDefId
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user