From 7dac5491d8c4d3a95fed081e2a2e43864bb7896a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 19 Aug 2020 17:49:23 +0200 Subject: [PATCH 1/8] Also track toCharArray --- compiler/src/dotty/tools/dotc/transform/Instrumentation.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Instrumentation.scala b/compiler/src/dotty/tools/dotc/transform/Instrumentation.scala index f4dafd131b62..848c6866b22a 100644 --- a/compiler/src/dotty/tools/dotc/transform/Instrumentation.scala +++ b/compiler/src/dotty/tools/dotc/transform/Instrumentation.scala @@ -29,7 +29,7 @@ class Instrumentation extends MiniPhase { thisPhase => ctx.settings.YinstrumentAllocations.value private val namesOfInterest = List( - "::", "+=", "toString", "newArray", "box", + "::", "+=", "toString", "newArray", "box", "toCharArray", "map", "flatMap", "filter", "withFilter", "collect", "foldLeft", "foldRight", "take", "reverse", "mapConserve", "mapconserve", "filterConserve", "zip") private var namesToRecord: Set[Name] = _ From cf8f605499af4e2afe0ff2c88c61bf9f2b15c864 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 20 Aug 2020 10:41:31 +0200 Subject: [PATCH 2/8] Avoid new name creation in isVarArityClass --- .../src/dotty/tools/dotc/core/Definitions.scala | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 55dd7464e789..01d59c963cc4 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -1050,11 +1050,15 @@ class Definitions { def scalaClassName(ref: Type)(using Context): TypeName = scalaClassName(ref.classSymbol) private def isVarArityClass(cls: Symbol, prefix: String) = - cls.isClass && cls.owner.eq(ScalaPackageClass) && - cls.name.testSimple(name => - name.startsWith(prefix) && - name.length > prefix.length && - name.drop(prefix.length).forall(_.isDigit)) + cls.isClass + && cls.owner.eq(ScalaPackageClass) + && cls.name.testSimple(name => + name.startsWith(prefix) + && name.length > prefix.length + && digitsOnlyAfter(name, prefix.length)) + + private def digitsOnlyAfter(name: SimpleName, idx: Int): Boolean = + idx == name.length || name(idx).isDigit && digitsOnlyAfter(name, idx + 1) def isBottomClass(cls: Symbol): Boolean = if (ctx.explicitNulls && !ctx.phase.erasedTypes) cls == NothingClass From b81089147eec1319e9454c4f7912119c373bd1d2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Jul 2020 19:41:11 +0200 Subject: [PATCH 3/8] Avoid creating a mutable hashmap in typedStats --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 405e450a4691..e39f382e26bb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2596,7 +2596,7 @@ class Typer extends Namer def typedStats(stats: List[untpd.Tree], exprOwner: Symbol)(using Context): (List[Tree], Context) = { val buf = new mutable.ListBuffer[Tree] - val enumContexts = new mutable.HashMap[Symbol, Context] + var enumContexts: SimpleIdentityMap[Symbol, Context] = SimpleIdentityMap.Empty val initialNotNullInfos = ctx.notNullInfos // A map from `enum` symbols to the contexts enclosing their definitions @tailrec def traverse(stats: List[untpd.Tree])(using Context): (List[Tree], Context) = stats match { @@ -2618,7 +2618,7 @@ class Typer extends Namer // replace body with expansion, because it will be used as inlined body // from separately compiled files - the original BodyAnnotation is not kept. case mdef1: TypeDef if mdef1.symbol.is(Enum, butNot = Case) => - enumContexts(mdef1.symbol) = ctx + enumContexts = enumContexts.updated(mdef1.symbol, ctx) buf += mdef1 case EmptyTree => // clashing synthetic case methods are converted to empty trees, drop them here @@ -2650,7 +2650,8 @@ class Typer extends Namer } def finalize(stat: Tree)(using Context): Tree = stat match { case stat: TypeDef if stat.symbol.is(Module) => - for (enumContext <- enumContexts.get(stat.symbol.linkedClass)) + val enumContext = enumContexts(stat.symbol.linkedClass) + if enumContext != null then checkEnumCaseRefsLegal(stat, enumContext) stat.removeAttachment(Deriver) match { case Some(deriver) => deriver.finalize(stat) From 13f9c9c35c2398bc15d8102dd775ec3390d19611 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 24 Jul 2020 20:17:44 +0200 Subject: [PATCH 4/8] Move type assigner to store Thsi gets Context size down to 72 bytes from 80, assuming a 12 byte header and 8 byte rounding. In the type/* benchmark this gives a saving of ~700K contexts * 8 bytes = 5.4M vs a store allocation increase of 11K * ~90bytes -= 1M (approx). --- .../src/dotty/tools/dotc/core/Contexts.scala | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 8e2e8c463c01..bd71a09b1c8b 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -49,8 +49,9 @@ object Contexts { private val (profilerLoc, store7) = store6.newLocation[Profiler]() private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]() private val (importInfoLoc, store9) = store8.newLocation[ImportInfo]() + private val (typeAssignerLoc, store10) = store9.newLocation[TypeAssigner](TypeAssigner) - private val initialStore = store9 + private val initialStore = store10 /** The current context */ inline def ctx(using ctx: Context): Context = ctx @@ -157,11 +158,6 @@ object Contexts { protected def typerState_=(typerState: TyperState): Unit = _typerState = typerState final def typerState: TyperState = _typerState - /** The current type assigner or typer */ - private var _typeAssigner: TypeAssigner = _ - protected def typeAssigner_=(typeAssigner: TypeAssigner): Unit = _typeAssigner = typeAssigner - final def typeAssigner: TypeAssigner = _typeAssigner - /** The current bounds in force for type parameters appearing in a GADT */ private var _gadt: GadtConstraint = _ protected def gadt_=(gadt: GadtConstraint): Unit = _gadt = gadt @@ -228,6 +224,9 @@ object Contexts { /** The currently active import info */ def importInfo = store(importInfoLoc) + /** The current type assigner or typer */ + def typeAssigner: TypeAssigner = store(typeAssignerLoc) + /** The new implicit references that are introduced by this scope */ protected var implicitsCache: ContextualImplicits = null def implicits: ContextualImplicits = { @@ -483,7 +482,6 @@ object Contexts { _owner = origin.owner _tree = origin.tree _scope = origin.scope - _typeAssigner = origin.typeAssigner _gadt = origin.gadt _searchHistory = origin.searchHistory _source = origin.source @@ -574,10 +572,6 @@ object Contexts { def setNewTyperState(): this.type = setTyperState(typerState.fresh().setCommittable(true)) def setExploreTyperState(): this.type = setTyperState(typerState.fresh().setCommittable(false)) def setReporter(reporter: Reporter): this.type = setTyperState(typerState.fresh().setReporter(reporter)) - def setTypeAssigner(typeAssigner: TypeAssigner): this.type = - util.Stats.record("Context.setTypeAssigner") - this.typeAssigner = typeAssigner - this def setTyper(typer: Typer): this.type = { this.scope = typer.scope; setTypeAssigner(typer) } def setGadt(gadt: GadtConstraint): this.type = util.Stats.record("Context.setGadt") @@ -615,6 +609,7 @@ object Contexts { def setProfiler(profiler: Profiler): this.type = updateStore(profilerLoc, profiler) def setNotNullInfos(notNullInfos: List[NotNullInfo]): this.type = updateStore(notNullInfosLoc, notNullInfos) def setImportInfo(importInfo: ImportInfo): this.type = updateStore(importInfoLoc, importInfo) + def setTypeAssigner(typeAssigner: TypeAssigner): this.type = updateStore(typeAssignerLoc, typeAssigner) def setProperty[T](key: Key[T], value: T): this.type = setMoreProperties(moreProperties.updated(key, value)) @@ -742,7 +737,6 @@ object Contexts { typerState = TyperState.initialState() owner = NoSymbol tree = untpd.EmptyTree - typeAssigner = TypeAssigner moreProperties = Map(MessageLimiter -> DefaultMessageLimiter()) source = NoSource store = initialStore From 6f5dba18f99d9077aa495b63fd103e38dabcd2eb Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 25 Jul 2020 11:10:23 +0200 Subject: [PATCH 5/8] Optimize stillValidInOwner --- compiler/src/dotty/tools/dotc/core/Scopes.scala | 6 ++++++ .../src/dotty/tools/dotc/core/SymDenotations.scala | 12 +++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Scopes.scala b/compiler/src/dotty/tools/dotc/core/Scopes.scala index 1726b2963e4b..90b27683cdf7 100644 --- a/compiler/src/dotty/tools/dotc/core/Scopes.scala +++ b/compiler/src/dotty/tools/dotc/core/Scopes.scala @@ -138,6 +138,12 @@ object Scopes { def next(): Symbol = { val r = e.sym; e = lookupNextEntry(e); r } } + /** Does this scope contain a reference to `sym` when looking up `name`? */ + final def contains(name: Name, sym: Symbol)(using Context): Boolean = + var e = lookupEntry(name) + while e != null && e.sym != sym do e = lookupNextEntry(e) + e != null + /** The denotation set of all the symbols with given name in this scope * Symbols occur in the result in reverse order relative to their occurrence * in `this.toList`. diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 37c337de08c0..7652d3ce4927 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2361,19 +2361,17 @@ object SymDenotations { stillValidInOwner(denot) } - private[SymDenotations] def stillValidInOwner(denot: SymDenotation)(using Context): Boolean = try { + private[SymDenotations] def stillValidInOwner(denot: SymDenotation)(using Context): Boolean = try val owner = denot.owner.denot - stillValid(owner) && ( + stillValid(owner) + && ( !owner.isClass || owner.isRefinementClass || owner.is(Scala2x) - || (owner.unforcedDecls.lookupAll(denot.name) contains denot.symbol) + || owner.unforcedDecls.contains(denot.name, denot.symbol) || denot.isSelfSym || denot.isLocalDummy) - } - catch { - case ex: StaleSymbol => false - } + catch case ex: StaleSymbol => false /** Explain why symbol is invalid; used for debugging only */ def traceInvalid(denot: Denotation)(using Context): Boolean = { From 26e49de8592c6d0f62ce25ac895b11a8e4dce36e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 20 Aug 2020 13:20:08 +0200 Subject: [PATCH 6/8] Eliminate closure in foreachTypeVar It's fairly hot code, and eliminating the closure also avoids Int boxing. --- .../src/dotty/tools/dotc/core/OrderingConstraint.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 98797b01eadd..089bc44f77ac 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -567,11 +567,13 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def foreachTypeVar(op: TypeVar => Unit): Unit = boundsMap.foreachBinding { (poly, entries) => - for (i <- 0 until paramCount(entries)) - typeVar(entries, i) match { + var i = 0 + val limit = paramCount(entries) + while i < limit do + typeVar(entries, i) match case tv: TypeVar if !tv.inst.exists => op(tv) case _ => - } + i += 1 } private var myUninstVars: mutable.ArrayBuffer[TypeVar] = _ From 66967f93ba74ca73dff8c764008984e4dc09b5b1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 25 Jul 2020 12:32:40 +0200 Subject: [PATCH 7/8] Optimize calculateLineIndices --- .../src/dotty/tools/dotc/util/SourceFile.scala | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/SourceFile.scala b/compiler/src/dotty/tools/dotc/util/SourceFile.scala index 36ff21c297ab..82f6957a28a0 100644 --- a/compiler/src/dotty/tools/dotc/util/SourceFile.scala +++ b/compiler/src/dotty/tools/dotc/util/SourceFile.scala @@ -103,18 +103,18 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends def positionInUltimateSource(position: SourcePosition): SourcePosition = SourcePosition(underlying, position.span shift start) - private def isLineBreak(idx: Int) = - if (idx >= length) false else { - val ch = content()(idx) - // don't identify the CR in CR LF as a line break, since LF will do. - if (ch == CR) (idx + 1 == length) || (content()(idx + 1) != LF) - else isLineBreakChar(ch) - } - private def calculateLineIndices(cs: Array[Char]) = { val buf = new ArrayBuffer[Int] buf += 0 - for (i <- 0 until cs.length) if (isLineBreak(i)) buf += i + 1 + var i = 0 + while i < cs.length do + val isLineBreak = + val ch = cs(i) + // don't identify the CR in CR LF as a line break, since LF will do. + if ch == CR then i + 1 == cs.length || cs(i + 1) != LF + else isLineBreakChar(ch) + if isLineBreak then buf += i + 1 + i += 1 buf += cs.length // sentinel, so that findLine below works smoother buf.toArray } From 41aba4b6d71731a4a174900fc79f5310b10d6014 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 25 Jul 2020 18:29:34 +0200 Subject: [PATCH 8/8] Avoid stack overflows in TreeAccumulator The previously optimized apply function was not tail recursive since it was not final. --- compiler/src/dotty/tools/dotc/ast/Trees.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index c422642879af..2cbccdd32e61 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -1379,11 +1379,11 @@ object Trees { // Ties the knot of the traversal: call `foldOver(x, tree))` to dive in the `tree` node. def apply(x: X, tree: Tree)(using Context): X - def apply(x: X, trees: List[Tree])(using Context): X = trees match - case tree :: rest => - apply(apply(x, tree), rest) - case Nil => - x + def apply(x: X, trees: List[Tree])(using Context): X = + def fold(x: X, trees: List[Tree]): X = trees match + case tree :: rest => fold(apply(x, tree), rest) + case Nil => x + fold(x, trees) def foldOver(x: X, tree: Tree)(using Context): X = if (tree.source != ctx.source && tree.source.exists)