Improve perf of Graph.Scope.scopeFor hotspot (#9620)

`scopeFor` appears to be a hotspot of the compiler. By choosing a more suitable data structure that indexes on the occurrence's id we seem to gain about 25% on some benchmarks. Quick win.

Related to #9235.

# Important Notes
Local benchmark runs of `org.enso.compiler.benchmarks.module.ManyLocalVarsBenchmark.longMethodWithLotOfLocalVars
`
Before
```
[info] # Warmup Iteration   1: 61.638 ms/op
[info] # Warmup Iteration   2: 49.224 ms/op
[info] # Warmup Iteration   3: 47.341 ms/op
[info] # Warmup Iteration   4: 46.946 ms/op
[info] # Warmup Iteration   5: 46.901 ms/op
[info] # Warmup Iteration   6: 49.536 ms/op
[info] Iteration   1: 50.438 ms/op
[info] Iteration   2: 47.326 ms/op
[info] Iteration   3: 46.917 ms/op
[info] Iteration   4: 45.824 ms/op
```

After
```
[info] # Warmup Iteration   1: 86.493 ms/op
[info] # Warmup Iteration   2: 36.084 ms/op
[info] # Warmup Iteration   3: 32.588 ms/op
[info] # Warmup Iteration   4: 33.895 ms/op
[info] # Warmup Iteration   5: 31.986 ms/op
[info] # Warmup Iteration   6: 31.236 ms/op
[info] Iteration   1: 31.258 ms/op
[info] Iteration   2: 31.673 ms/op
[info] Iteration   3: 30.931 ms/op
[info] Iteration   4: 30.902 ms/op
```
This commit is contained in:
Hubert Plociniczak 2024-04-04 17:26:06 +02:00 committed by GitHub
parent d9c7bf4138
commit a82a429127
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 30 additions and 24 deletions

View File

@ -24,6 +24,7 @@ import org.enso.persist.Persistable;
import org.enso.persist.Persistance; import org.enso.persist.Persistance;
import org.openide.util.lookup.ServiceProvider; import org.openide.util.lookup.ServiceProvider;
import scala.Option; import scala.Option;
import scala.Tuple2$;
@Persistable(clazz = CachePreferenceAnalysis.WeightInfo.class, id = 1111) @Persistable(clazz = CachePreferenceAnalysis.WeightInfo.class, id = 1111)
@Persistable(clazz = DataflowAnalysis.DependencyInfo.class, id = 1112) @Persistable(clazz = DataflowAnalysis.DependencyInfo.class, id = 1112)
@ -128,7 +129,8 @@ public final class PassPersistance {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
protected Graph.Scope readObject(Input in) throws IOException { protected Graph.Scope readObject(Input in) throws IOException {
var childScopes = in.readInline(scala.collection.immutable.List.class); var childScopes = in.readInline(scala.collection.immutable.List.class);
var occurrences = (scala.collection.immutable.Set) in.readObject(); var occurrencesValues = (scala.collection.immutable.Set<Graph.Occurrence>) in.readObject();
var occurrences = occurrencesValues.map(v -> Tuple2$.MODULE$.apply(v.id(), v)).toMap(null);
var allDefinitions = in.readInline(scala.collection.immutable.List.class); var allDefinitions = in.readInline(scala.collection.immutable.List.class);
var parent = new Graph.Scope(childScopes, occurrences, allDefinitions); var parent = new Graph.Scope(childScopes, occurrences, allDefinitions);
var optionParent = Option.apply(parent); var optionParent = Option.apply(parent);
@ -145,7 +147,7 @@ public final class PassPersistance {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
protected void writeObject(Graph.Scope obj, Output out) throws IOException { protected void writeObject(Graph.Scope obj, Output out) throws IOException {
out.writeInline(scala.collection.immutable.List.class, obj.childScopes()); out.writeInline(scala.collection.immutable.List.class, obj.childScopes());
out.writeObject(obj.occurrences()); out.writeObject(obj.occurrences().values().toSet());
out.writeInline(scala.collection.immutable.List.class, obj.allDefinitions()); out.writeInline(scala.collection.immutable.List.class, obj.allDefinitions());
} }
} }

View File

@ -140,10 +140,10 @@ class LocalScope(
.getOrElse(Map()) .getOrElse(Map())
scope.occurrences.foreach { scope.occurrences.foreach {
case x: AliasGraph.Occurrence.Def => case (id, x: AliasGraph.Occurrence.Def) =>
parentResult += x.symbol -> new FramePointer( parentResult += x.symbol -> new FramePointer(
level, level,
allFrameSlotIdxs(x.id) allFrameSlotIdxs(id)
) )
case _ => case _ =>
} }

View File

@ -602,12 +602,12 @@ case object AliasAnalysis extends IRPass {
) )
) )
} else { } else {
val f = scope.occurrences.collectFirst { val f = scope.occurrences.values.collectFirst {
case x if x.symbol == name.name => x case x if x.symbol == name.name => x
} }
arg arg
.copy( .copy(
ascribedType = Some(new Redefined.Arg(name, arg.location)) ascribedType = Some(Redefined.Arg(name, arg.location))
) )
.updateMetadata( .updateMetadata(
new MetadataPair( new MetadataPair(

View File

@ -272,11 +272,11 @@ sealed class Graph extends Serializable {
def getShadowedIds( def getShadowedIds(
scope: Graph.Scope scope: Graph.Scope
): Set[Graph.Occurrence] = { ): Set[Graph.Occurrence] = {
scope.occurrences.collect { scope.occurrences.values.collect {
case d: Occurrence.Def if d.symbol == definition.symbol => d case d: Occurrence.Def if d.symbol == definition.symbol => d
case g: Occurrence.Global if g.symbol == definition.symbol => g case g: Occurrence.Global if g.symbol == definition.symbol => g
} ++ scope.parent.map(getShadowedIds).getOrElse(Set()) } ++ scope.parent.map(getShadowedIds).getOrElse(Set())
} }.toSet
definition match { definition match {
case d: Occurrence.Def => case d: Occurrence.Def =>
@ -343,13 +343,13 @@ object Graph {
/** A representation of a local scope in Enso. /** A representation of a local scope in Enso.
* *
* @param childScopes all scopes that are _direct_ children of `this` * @param childScopes all scopes that are _direct_ children of `this`
* @param occurrences all symbol occurrences in `this` scope * @param occurrences all symbol occurrences in `this` scope indexed by the identifier of the name
* @param allDefinitions all definitions in this scope, including synthetic ones. * @param allDefinitions all definitions in this scope, including synthetic ones.
* Note that there may not be a link for all these definitions. * Note that there may not be a link for all these definitions.
*/ */
sealed class Scope( sealed class Scope(
var childScopes: List[Scope] = List(), var childScopes: List[Scope] = List(),
var occurrences: Set[Occurrence] = Set(), var occurrences: Map[Id, Occurrence] = HashMap(),
var allDefinitions: List[Occurrence.Def] = List() var allDefinitions: List[Occurrence.Def] = List()
) extends Serializable { ) extends Serializable {
@ -437,7 +437,13 @@ object Graph {
* @param occurrence the occurrence to add * @param occurrence the occurrence to add
*/ */
def add(occurrence: Occurrence): Unit = { def add(occurrence: Occurrence): Unit = {
occurrences += occurrence if (occurrences.contains(occurrence.id)) {
throw new CompilerError(
s"Multiple occurrences found for ID ${occurrence.id}."
)
} else {
occurrences += ((occurrence.id, occurrence))
}
} }
/** Adds a definition, including a definition with synthetic name, without /** Adds a definition, including a definition with synthetic name, without
@ -456,7 +462,7 @@ object Graph {
* @return the occurrence for `id`, if it exists * @return the occurrence for `id`, if it exists
*/ */
def getOccurrence(id: Graph.Id): Option[Occurrence] = { def getOccurrence(id: Graph.Id): Option[Occurrence] = {
occurrences.find(o => o.id == id) occurrences.get(id)
} }
/** Finds any occurrences for the provided symbol in the current scope, if /** Finds any occurrences for the provided symbol in the current scope, if
@ -469,9 +475,9 @@ object Graph {
def getOccurrences[T <: Occurrence: ClassTag]( def getOccurrences[T <: Occurrence: ClassTag](
symbol: Graph.Symbol symbol: Graph.Symbol
): Set[Occurrence] = { ): Set[Occurrence] = {
occurrences.collect { occurrences.values.collect {
case o: T if o.symbol == symbol => o case o: T if o.symbol == symbol => o
} }.toSet
} }
/** Unsafely gets the occurrence for the provided ID in the current scope. /** Unsafely gets the occurrence for the provided ID in the current scope.
@ -496,7 +502,9 @@ object Graph {
def hasSymbolOccurrenceAs[T <: Occurrence: ClassTag]( def hasSymbolOccurrenceAs[T <: Occurrence: ClassTag](
symbol: Graph.Symbol symbol: Graph.Symbol
): Boolean = { ): Boolean = {
occurrences.collect { case x: T if x.symbol == symbol => x }.nonEmpty occurrences.values.collectFirst {
case x: T if x.symbol == symbol => x
}.nonEmpty
} }
/** Resolves usages of symbols into links where possible, creating an edge /** Resolves usages of symbols into links where possible, creating an edge
@ -511,7 +519,7 @@ object Graph {
occurrence: Graph.Occurrence.Use, occurrence: Graph.Occurrence.Use,
parentCounter: Int = 0 parentCounter: Int = 0
): Option[Graph.Link] = { ): Option[Graph.Link] = {
val definition = occurrences.find { val definition = occurrences.values.find {
case Graph.Occurrence.Def(_, name, _, _, _) => case Graph.Occurrence.Def(_, name, _, _, _) =>
name == occurrence.symbol name == occurrence.symbol
case _ => false case _ => false
@ -530,7 +538,7 @@ object Graph {
* @return a string representation of `this` * @return a string representation of `this`
*/ */
override def toString: String = override def toString: String =
s"Scope(occurrences = $occurrences, childScopes = $childScopes)" s"Scope(occurrences = ${occurrences.values}, childScopes = $childScopes)"
/** Counts the number of scopes in this scope. /** Counts the number of scopes in this scope.
* *
@ -555,9 +563,7 @@ object Graph {
* @return the scope where `id` occurs * @return the scope where `id` occurs
*/ */
def scopeFor(id: Graph.Id): Option[Scope] = { def scopeFor(id: Graph.Id): Option[Scope] = {
val possibleCandidates = occurrences.filter(o => o.id == id) if (!occurrences.contains(id)) {
if (possibleCandidates.isEmpty) {
if (childScopes.isEmpty) { if (childScopes.isEmpty) {
None None
} else { } else {
@ -584,10 +590,8 @@ object Graph {
Some(childCandidate) Some(childCandidate)
} }
} }
} else if (possibleCandidates.size == 1) {
Some(this)
} else { } else {
throw new CompilerError(s"Multiple occurrences found for ID $id.") Some(this)
} }
} }
@ -629,7 +633,7 @@ object Graph {
* @return the set of symbols * @return the set of symbols
*/ */
def symbols: Set[Graph.Symbol] = { def symbols: Set[Graph.Symbol] = {
val symbolsInThis = occurrences.map(_.symbol) val symbolsInThis = occurrences.values.map(_.symbol).toSet
val symbolsInChildScopes = childScopes.flatMap(_.symbols) val symbolsInChildScopes = childScopes.flatMap(_.symbols)
symbolsInThis ++ symbolsInChildScopes symbolsInThis ++ symbolsInChildScopes