From de04a7d5f566357df56019c25d2953d7b35b5bc1 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 14:16:14 +0100 Subject: [PATCH 01/28] Better benchmark and Stats infrastructure - Allow to specify the number of `#compilers` to run and the number of #runs per compiler. - Allow to specify with #wait-after a run after which we ask for a prompt. Useful for precise profiling. - Fix bug in Stats that prevented printing of detailed statistics - Make Profiler work with Console output. Show total allocations. --- compiler/src/dotty/tools/dotc/Bench.scala | 32 +++++++++++-------- compiler/src/dotty/tools/dotc/Driver.scala | 2 +- .../dotty/tools/dotc/profile/Profiler.scala | 13 +++++--- .../src/dotty/tools/dotc/util/Stats.scala | 15 ++++----- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Bench.scala b/compiler/src/dotty/tools/dotc/Bench.scala index c9c032b0ae7d..5f5e9fc799b5 100644 --- a/compiler/src/dotty/tools/dotc/Bench.scala +++ b/compiler/src/dotty/tools/dotc/Bench.scala @@ -14,24 +14,22 @@ import scala.annotation.internal.sharable object Bench extends Driver: @sharable private var numRuns = 1 - - private def ntimes(n: Int)(op: => Reporter): Reporter = - (0 until n).foldLeft(emptyReporter)((_, _) => op) - + @sharable private var numCompilers = 1 + @sharable private var waitAfter = -1 + @sharable private var curCompiler = 0 @sharable private var times: Array[Int] = _ override def doCompile(compiler: Compiler, files: List[AbstractFile])(using Context): Reporter = - times = new Array[Int](numRuns) var reporter: Reporter = emptyReporter for i <- 0 until numRuns do + val curRun = curCompiler * numRuns + i val start = System.nanoTime() reporter = super.doCompile(compiler, files) - times(i) = ((System.nanoTime - start) / 1000000).toInt - println(s"time elapsed: ${times(i)}ms") - if ctx.settings.Xprompt.value then + times(curRun) = ((System.nanoTime - start) / 1000000).toInt + println(s"time elapsed: ${times(curRun)}ms") + if ctx.settings.Xprompt.value || waitAfter == curRun + 1 then print("hit to continue >") System.in.nn.read() - println() reporter def extractNumArg(args: Array[String], name: String, default: Int = 1): (Int, Array[String]) = { @@ -42,20 +40,26 @@ object Bench extends Driver: def reportTimes() = val best = times.sorted - val measured = numRuns / 3 + val measured = numCompilers * numRuns / 3 val avgBest = best.take(measured).sum / measured val avgLast = times.reverse.take(measured).sum / measured - println(s"best out of $numRuns runs: ${best(0)}") + println(s"best out of ${numCompilers * numRuns} runs: ${best(0)}") println(s"average out of best $measured: $avgBest") println(s"average out of last $measured: $avgLast") - override def process(args: Array[String], rootCtx: Context): Reporter = + override def process(args: Array[String]): Reporter = val (numCompilers, args1) = extractNumArg(args, "#compilers") val (numRuns, args2) = extractNumArg(args1, "#runs") + val (waitAfter, args3) = extractNumArg(args2, "#wait-after", -1) + this.numCompilers = numCompilers this.numRuns = numRuns + this.waitAfter = waitAfter + this.times = new Array[Int](numCompilers * numRuns) var reporter: Reporter = emptyReporter - for i <- 0 until numCompilers do - reporter = super.process(args2, rootCtx) + curCompiler = 0 + while curCompiler < numCompilers do + reporter = super.process(args3) + curCompiler += 1 reportTimes() reporter diff --git a/compiler/src/dotty/tools/dotc/Driver.scala b/compiler/src/dotty/tools/dotc/Driver.scala index 0af35dd4068a..5a2c8b7be56e 100644 --- a/compiler/src/dotty/tools/dotc/Driver.scala +++ b/compiler/src/dotty/tools/dotc/Driver.scala @@ -171,7 +171,7 @@ class Driver { * the other overloads without worrying about breaking compatibility * with sbt. */ - final def process(args: Array[String]): Reporter = + def process(args: Array[String]): Reporter = process(args, null: Reporter | Null, null: interfaces.CompilerCallback | Null) /** Entry point to the compiler using a custom `Context`. diff --git a/compiler/src/dotty/tools/dotc/profile/Profiler.scala b/compiler/src/dotty/tools/dotc/profile/Profiler.scala index 25c53903c10b..64cc08160701 100644 --- a/compiler/src/dotty/tools/dotc/profile/Profiler.scala +++ b/compiler/src/dotty/tools/dotc/profile/Profiler.scala @@ -13,6 +13,7 @@ import javax.management.{Notification, NotificationEmitter, NotificationListener import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.Contexts._ import dotty.tools.io.AbstractFile +import annotation.internal.sharable object Profiler { def apply()(using Context): Profiler = @@ -217,14 +218,16 @@ sealed trait ProfileReporter { } object ConsoleProfileReporter extends ProfileReporter { - + @sharable var totalAlloc = 0L override def reportBackground(profiler: RealProfiler, threadRange: ProfileRange): Unit = - // TODO - ??? + reportCommon(EventType.BACKGROUND, profiler, threadRange) override def reportForeground(profiler: RealProfiler, threadRange: ProfileRange): Unit = - // TODO - ??? + reportCommon(EventType.MAIN, profiler, threadRange) + @nowarn("cat=deprecation") + private def reportCommon(tpe:EventType, profiler: RealProfiler, threadRange: ProfileRange): Unit = + totalAlloc += threadRange.allocatedBytes + println(s"${threadRange.phase.phaseName.replace(',', ' ')},run ns = ${threadRange.runNs},idle ns = ${threadRange.idleNs},cpu ns = ${threadRange.cpuNs},user ns = ${threadRange.userNs},allocated = ${threadRange.allocatedBytes},heap at end = ${threadRange.end.heapBytes}, total allocated = $totalAlloc ") override def close(profiler: RealProfiler): Unit = () diff --git a/compiler/src/dotty/tools/dotc/util/Stats.scala b/compiler/src/dotty/tools/dotc/util/Stats.scala index f04957f26400..e9b72015b202 100644 --- a/compiler/src/dotty/tools/dotc/util/Stats.scala +++ b/compiler/src/dotty/tools/dotc/util/Stats.scala @@ -55,15 +55,14 @@ import collection.mutable } def maybeMonitored[T](op: => T)(using Context): T = - if (ctx.settings.YdetailedStats.value && hits.nonEmpty) { + if ctx.settings.YdetailedStats.value then monitored = true try op - finally { - aggregate() - println() - println(hits.toList.sortBy(_._2).map{ case (x, y) => s"$x -> $y" } mkString "\n") - hits.clear() - } - } + finally + if hits.nonEmpty then + aggregate() + println() + println(hits.toList.sortBy(_._2).map{ case (x, y) => s"$x -> $y" } mkString "\n") + hits.clear() else op } From 7e0c8151cb70489090318506c956381e83f068f4 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 15:21:48 +0100 Subject: [PATCH 02/28] Reduce allocations for pickling - Avoid boxing overheads in NameBuffer and TreeBuffer - Avoid repeated re-allocations in updateMapWithDeltas --- .../tools/dotc/core/tasty/NameBuffer.scala | 71 ++++++++++--------- .../tools/dotc/core/tasty/TreeBuffer.scala | 8 +-- .../tools/dotc/core/tasty/TreePickler.scala | 13 +++- 3 files changed, 52 insertions(+), 40 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala b/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala index 623508780325..33d4d0ea258e 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala @@ -6,7 +6,7 @@ package tasty import dotty.tools.tasty.TastyBuffer import TastyBuffer._ -import collection.mutable +import collection.mutable.ArrayBuffer import Names.{Name, chrs, SimpleName, DerivedName, TypeName} import NameKinds._ import NameOps._ @@ -16,42 +16,43 @@ import NameTags.{SIGNED, TARGETSIGNED} class NameBuffer extends TastyBuffer(10000) { import NameBuffer._ - private val nameRefs = new mutable.LinkedHashMap[Name, NameRef] + private val nameRefs = new util.EqHashMap[Name, NameRef] + private val nameBuf = new ArrayBuffer[Name] - def nameIndex(name: Name): NameRef = { + def nameIndex(name: Name): NameRef = val name1 = name.toTermName - nameRefs.get(name1) match { - case Some(ref) => - ref - case None => - name1 match { - case SignedName(original, Signature(params, result), target) => - nameIndex(original) - if !original.matchesTargetName(target) then nameIndex(target) - nameIndex(result) - params.foreach { - case param: TypeName => - nameIndex(param) - case _ => - } - case AnyQualifiedName(prefix, name) => - nameIndex(prefix); nameIndex(name) - case AnyUniqueName(original, separator, num) => - nameIndex(separator) - if (!original.isEmpty) nameIndex(original) - case DerivedName(original, _) => - nameIndex(original) - case _ => - } - val ref = NameRef(nameRefs.size) - nameRefs(name1) = ref - ref - } - } + val ref: NameRef | Null = nameRefs.lookup(name1) + if ref != null then ref.uncheckedNN + else + name1 match + case SignedName(original, Signature(params, result), target) => + nameIndex(original) + if !original.matchesTargetName(target) then nameIndex(target) + nameIndex(result) + params.foreach { + case param: TypeName => + nameIndex(param) + case _ => + } + case AnyQualifiedName(prefix, name) => + nameIndex(prefix); nameIndex(name) + case AnyUniqueName(original, separator, num) => + nameIndex(separator) + if (!original.isEmpty) nameIndex(original) + case DerivedName(original, _) => + nameIndex(original) + case _ => + val ref1 = NameRef(nameRefs.size) + nameRefs(name1) = ref1 + nameBuf += name1 + ref1 - private def withLength(op: => Unit, lengthWidth: Int = 1): Unit = { + private inline def withLength(inline op: Unit, lengthWidth: Int = 1): Unit = { val lengthAddr = currentAddr - for (i <- 0 until lengthWidth) writeByte(0) + var i = 0 + while i < lengthWidth do + writeByte(0) + i += 1 op val length = currentAddr.index - lengthAddr.index - lengthWidth putNat(lengthAddr, length, lengthWidth) @@ -111,11 +112,11 @@ class NameBuffer extends TastyBuffer(10000) { override def assemble(): Unit = { var i = 0 - for ((name, ref) <- nameRefs) { + for name <- nameBuf do + val ref = nameRefs(name) assert(ref.index == i) i += 1 pickleNameContents(name) - } } } diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala index a3dedaaec685..f8f545304a32 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala @@ -109,10 +109,11 @@ class TreeBuffer extends TastyBuffer(50000) { /** Adjust all offsets according to previously computed deltas */ private def adjustOffsets(): Unit = - for (i <- 0 until numOffsets) { + var i = 0 + while i < numOffsets do val corrected = adjustedOffset(i) fillAddr(offset(i), corrected) - } + i += 1 /** Adjust deltas to also take account references that will shrink (and thereby * generate additional zeroes that can be skipped) due to previously @@ -122,12 +123,11 @@ class TreeBuffer extends TastyBuffer(50000) { val delta1 = new Array[Int](delta.length) var lastDelta = 0 var i = 0 - while (i < numOffsets) { + while i < numOffsets do val corrected = adjustedOffset(i) lastDelta += AddrWidth - TastyBuffer.natSize(corrected.index) delta1(i) = lastDelta i += 1 - } val saved = if (numOffsets == 0) 0 else delta1(numOffsets - 1) - delta(numOffsets - 1) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala index f5c0c678a6ec..8895ae90950b 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala @@ -816,7 +816,18 @@ class TreePickler(pickler: TastyPickler) { buf.compactify() def updateMapWithDeltas(mp: MutableSymbolMap[Addr]) = - for (key <- mp.keysIterator.toBuffer[Symbol]) mp(key) = adjusted(mp(key)) + val keys = new Array[Symbol](mp.size) + val it = mp.keysIterator + var i = 0 + while i < keys.length do + keys(i) = it.next + i += 1 + assert(!it.hasNext) + i = 0 + while i < keys.length do + val key = keys(i) + mp(key) = adjusted(mp(key)) + i += 1 updateMapWithDeltas(symRefs) } From cb40386c5f47cf3124c5717fe7dc9098dd2d9c1c Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 15:40:07 +0100 Subject: [PATCH 03/28] Reduce context creations for value class related ops --- .../community-projects/requests-scala | 2 +- compiler/src/dotty/tools/dotc/ast/tpd.scala | 2 +- .../tools/dotc/core/SymDenotations.scala | 20 ++++++++++++------- .../tools/dotc/transform/ValueClasses.scala | 18 ++++++++--------- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/community-build/community-projects/requests-scala b/community-build/community-projects/requests-scala index 23b4895710f1..6d4a223bc33d 160000 --- a/community-build/community-projects/requests-scala +++ b/community-build/community-projects/requests-scala @@ -1 +1 @@ -Subproject commit 23b4895710f17bf892563b28755b225c8be7f7e3 +Subproject commit 6d4a223bc33def14ae9a4def24a3f5c258451e8e diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index f278b4b610db..d9d2c2ac4d4e 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -857,7 +857,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { } /** After phase `trans`, set the owner of every definition in this tree that was formerly - * owner by `from` to `to`. + * owned by `from` to `to`. */ def changeOwnerAfter(from: Symbol, to: Symbol, trans: DenotTransformer)(using Context): ThisTree = if (ctx.phase == trans.next) { diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 89f06618a8c0..aad42b69e238 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -848,13 +848,8 @@ object SymDenotations { def isSerializable(using Context): Boolean = isClass && derivesFrom(defn.JavaSerializableClass) - /** Is this symbol a class that extends `AnyVal`? */ - final def isValueClass(using Context): Boolean = - val di = initial - di.isClass - && atPhase(di.validFor.firstPhaseId)(di.derivesFrom(defn.AnyValClass)) - // We call derivesFrom at the initial phase both because AnyVal does not exist - // after Erasure and to avoid cyclic references caused by forcing denotations + /** Is this symbol a class that extends `AnyVal`? Overridden in ClassDenotation */ + def isValueClass(using Context): Boolean = false /** Is this symbol a class of which `null` is a value? */ final def isNullableClass(using Context): Boolean = @@ -2006,6 +2001,17 @@ object SymDenotations { /** Hook to do a pre-enter test. Overridden in PackageDenotation */ protected def proceedWithEnter(sym: Symbol, mscope: MutableScope)(using Context): Boolean = true + final override def isValueClass(using Context): Boolean = + val di = initial.asClass + val anyVal = defn.AnyValClass + if di.baseDataCache.isValid && !ctx.erasedTypes then + // fast path that does not demand time travel + (symbol eq anyVal) || di.baseClassSet.contains(anyVal) + else + // We call derivesFrom at the initial phase both because AnyVal does not exist + // after Erasure and to avoid cyclic references caused by forcing denotations + atPhase(di.validFor.firstPhaseId)(di.derivesFrom(anyVal)) + /** Enter a symbol in current scope, and future scopes of same denotation. * Note: We require that this does not happen after the first time * someone does a findMember on a subclass. diff --git a/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala b/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala index 92cbd0e57ac5..28d1255eaa72 100644 --- a/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala +++ b/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala @@ -22,16 +22,14 @@ object ValueClasses { } def isMethodWithExtension(sym: Symbol)(using Context): Boolean = - atPhaseNoLater(extensionMethodsPhase) { - val d = sym.denot - d.validFor.containsPhaseId(ctx.phaseId) && - d.isRealMethod && - isDerivedValueClass(d.owner) && - !d.isConstructor && - !d.symbol.isSuperAccessor && - !d.isInlineMethod && - !d.is(Macro) - } + val d = sym.denot.initial + d.validFor.firstPhaseId <= extensionMethodsPhase.id + && d.isRealMethod + && isDerivedValueClass(d.owner) + && !d.isConstructor + && !d.symbol.isSuperAccessor + && !d.isInlineMethod + && !d.is(Macro) /** The member of a derived value class that unboxes it. */ def valueClassUnbox(cls: ClassSymbol)(using Context): Symbol = From 2fbb891eca12cb6cbe3ebdaa7f682821656cec2a Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 15:46:41 +0100 Subject: [PATCH 04/28] Refactor context pools Also have a context pool for committable typer states, which gets used in isFullyDefined. --- .../src/dotty/tools/dotc/core/Contexts.scala | 129 +++++++++++------- .../dotty/tools/dotc/typer/Inferencing.scala | 8 +- 2 files changed, 79 insertions(+), 58 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index f920027032a7..b861cef43f61 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -451,7 +451,6 @@ object Contexts { /** A fresh clone of this context embedded in the specified `outer` context. */ def freshOver(outer: Context): FreshContext = - util.Stats.record("Context.fresh") FreshContext(base).init(outer, this).setTyperState(this.typerState) final def withOwner(owner: Symbol): Context = @@ -501,7 +500,9 @@ object Contexts { implicitsCache = null related = null - /** Reuse this context as a fresh context nested inside `outer` */ + /** Reuse this context as a fresh context nested inside `outer` + * But keep the typerstate, this one has to be set explicitly if needed. + */ def reuseIn(outer: Context): this.type } @@ -517,6 +518,7 @@ object Contexts { * of its attributes using the with... methods. */ class FreshContext(base: ContextBase) extends Context(base) { + util.Stats.record("Context.fresh") private var _outer: Context = uninitialized def outer: Context = _outer @@ -579,7 +581,7 @@ object Contexts { def setPeriod(period: Period): this.type = util.Stats.record("Context.setPeriod") - assert(period.firstPhaseId == period.lastPhaseId, period) + //assert(period.firstPhaseId == period.lastPhaseId, period) this._period = period this @@ -752,56 +754,40 @@ object Contexts { final def retractMode(mode: Mode): c.type = c.setMode(c.mode &~ mode) } - private def exploreCtx(using Context): FreshContext = - util.Stats.record("explore") - val base = ctx.base - import base._ - val nestedCtx = - if exploresInUse < exploreContexts.size then - exploreContexts(exploresInUse).reuseIn(ctx) - else - val ts = TyperState() - .setReporter(ExploringReporter()) - .setCommittable(false) - val c = FreshContext(ctx.base).init(ctx, ctx).setTyperState(ts) - exploreContexts += c - c - exploresInUse += 1 - val nestedTS = nestedCtx.typerState - nestedTS.init(ctx.typerState, ctx.typerState.constraint) - nestedCtx - - private def wrapUpExplore(ectx: Context) = - ectx.reporter.asInstanceOf[ExploringReporter].reset() - ectx.base.exploresInUse -= 1 - + /** Run `op` with a pool-allocated context that has an ExporeTyperState. */ inline def explore[T](inline op: Context ?=> T)(using Context): T = - val ectx = exploreCtx - try op(using ectx) finally wrapUpExplore(ectx) + exploreInFreshCtx(op) + /** Run `op` with a pool-allocated FreshContext that has an ExporeTyperState. */ inline def exploreInFreshCtx[T](inline op: FreshContext ?=> T)(using Context): T = - val ectx = exploreCtx - try op(using ectx) finally wrapUpExplore(ectx) - - private def changeOwnerCtx(owner: Symbol)(using Context): Context = - val base = ctx.base - import base._ - val nestedCtx = - if changeOwnersInUse < changeOwnerContexts.size then - changeOwnerContexts(changeOwnersInUse).reuseIn(ctx) - else - val c = FreshContext(ctx.base).init(ctx, ctx) - changeOwnerContexts += c - c - changeOwnersInUse += 1 - nestedCtx.setOwner(owner).setTyperState(ctx.typerState) - - /** Run `op` in current context, with a mode is temporarily set as specified. + val pool = ctx.base.exploreContextPool + val nestedCtx = pool.next() + try op(using nestedCtx) + finally + nestedCtx.typerState.reporter.asInstanceOf[ExploringReporter].reset() + pool.free() + + /** Run `op` with a pool-allocated context that has a fresh typer state. + * Commit the typer state if `commit` applied to `op`'s result returns true. */ + inline def withFreshTyperState[T](inline op: Context ?=> T, inline commit: T => Context ?=> Boolean)(using Context): T = + val pool = ctx.base.freshTSContextPool + val nestedCtx = pool.next() + try + val result = op(using nestedCtx) + if commit(result)(using nestedCtx) then + nestedCtx.typerState.commit() + nestedCtx.typerState.setCommittable(true) + result + finally + pool.free() + + /** Run `op` with a pool-allocated context that has the given `owner`. */ inline def runWithOwner[T](owner: Symbol)(inline op: Context ?=> T)(using Context): T = if Config.reuseOwnerContexts then - try op(using changeOwnerCtx(owner)) - finally ctx.base.changeOwnersInUse -= 1 + val pool = ctx.base.generalContextPool + try op(using pool.next().setOwner(owner).setTyperState(ctx.typerState)) + finally pool.free() else op(using ctx.fresh.setOwner(owner)) @@ -893,6 +879,47 @@ object Contexts { allPhases.find(_.period.containsPhaseId(p.id)).getOrElse(NoPhase) } + class ContextPool: + protected def fresh()(using Context): FreshContext = + FreshContext(ctx.base).init(ctx, ctx) + + private var inUse: Int = 0 + private var pool = new mutable.ArrayBuffer[FreshContext] + + def next()(using Context): FreshContext = + val base = ctx.base + import base._ + val nestedCtx = + if inUse < pool.size then + pool(inUse).reuseIn(ctx) + else + val c = fresh() + pool += c + c + inUse += 1 + nestedCtx + + final def free(): Unit = + inUse -= 1 + end ContextPool + + class TSContextPool extends ContextPool: + override def next()(using Context) = + val nextCtx = super.next() + nextCtx.typerState.init(ctx.typerState, ctx.typerState.constraint) + nextCtx + + class FreshTSContextPool extends TSContextPool: + override protected def fresh()(using Context) = + super.fresh().setTyperState(ctx.typerState.fresh(committable = true)) + + class ExploreContextPool extends TSContextPool: + override protected def fresh()(using Context) = + val ts = TyperState() + .setReporter(ExploringReporter()) + .setCommittable(false) + super.fresh().setTyperState(ts) + /** The essential mutable state of a context base, collected into a common class */ class ContextState { // Symbols state @@ -975,11 +1002,9 @@ object Contexts { protected[dotc] val indentTab: String = " " - private[Contexts] val exploreContexts = new mutable.ArrayBuffer[FreshContext] - private[Contexts] var exploresInUse: Int = 0 - - private[Contexts] val changeOwnerContexts = new mutable.ArrayBuffer[FreshContext] - private[Contexts] var changeOwnersInUse: Int = 0 + val exploreContextPool = ExploreContextPool() + val freshTSContextPool = FreshTSContextPool() + val generalContextPool = ContextPool() private[Contexts] val comparers = new mutable.ArrayBuffer[TypeComparer] private[Contexts] var comparersInUse: Int = 0 diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala index 656a1266d5ab..2aef3433228b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala @@ -26,12 +26,8 @@ object Inferencing { * but only if the overall result of `isFullyDefined` is `true`. * Variables that are successfully minimized do not count as uninstantiated. */ - def isFullyDefined(tp: Type, force: ForceDegree.Value)(using Context): Boolean = { - val nestedCtx = ctx.fresh.setNewTyperState() - val result = new IsFullyDefinedAccumulator(force)(using nestedCtx).process(tp) - if (result) nestedCtx.typerState.commit() - result - } + def isFullyDefined(tp: Type, force: ForceDegree.Value)(using Context): Boolean = + withFreshTyperState(new IsFullyDefinedAccumulator(force).process(tp), x => x) /** Try to fully define `tp`. Return whether constraint has changed. * Any changed constraint is kept. From 3d21427c6e98cea414b5b6fde7c8d8328010f5f2 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 20 Dec 2022 18:27:10 +0100 Subject: [PATCH 05/28] Avoid creation of Type lists when assigning types to Apply nodes --- compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 6d225e2c7629..98e9cb638c17 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -279,14 +279,14 @@ trait TypeAssigner { def safeSubstMethodParams(mt: MethodType, argTypes: List[Type])(using Context): Type = if mt.isResultDependent then safeSubstParams(mt.resultType, mt.paramRefs, argTypes) - else if mt.isCaptureDependent then mt.resultType.substParams(mt, argTypes) else mt.resultType def assignType(tree: untpd.Apply, fn: Tree, args: List[Tree])(using Context): Apply = { val ownType = fn.tpe.widen match { case fntpe: MethodType => - if (fntpe.paramInfos.hasSameLengthAs(args) || ctx.phase.prev.relaxedTyping) - safeSubstMethodParams(fntpe, args.tpes) + if fntpe.paramInfos.hasSameLengthAs(args) || ctx.phase.prev.relaxedTyping then + if fntpe.isResultDependent then safeSubstMethodParams(fntpe, args.tpes) + else fntpe.resultType // fast path optimization else errorType(em"wrong number of arguments at ${ctx.phase.prev} for $fntpe: ${fn.tpe}, expected: ${fntpe.paramInfos.length}, found: ${args.length}", tree.srcPos) case t => From 47f5b02b41eb2de3cca28c0a330331c28f9373b0 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 16:08:42 +0100 Subject: [PATCH 06/28] Pickling reorganizations - Don't create additional threads if ParallelPickling = false - Reorganize pickling to use common scratch data between different pickles to conserve space --- .../dotc/core/tasty/CommentPickler.scala | 69 ++++--- .../dotc/core/tasty/PositionPickler.scala | 43 ++-- .../tools/dotc/core/tasty/ScratchData.scala | 20 ++ .../tools/dotc/core/tasty/TastyPickler.scala | 5 +- .../tools/dotc/core/tasty/TreeBuffer.scala | 192 +++++++++--------- .../tools/dotc/core/tasty/TreePickler.scala | 6 +- .../tools/dotc/quoted/PickledQuotes.scala | 4 +- .../dotty/tools/dotc/transform/Pickler.scala | 76 ++++--- tasty/src/dotty/tools/tasty/TastyBuffer.scala | 5 + tasty/src/dotty/tools/tasty/TastyHash.scala | 6 +- 10 files changed, 250 insertions(+), 176 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/core/tasty/ScratchData.scala diff --git a/compiler/src/dotty/tools/dotc/core/tasty/CommentPickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/CommentPickler.scala index df3e4df497f8..fde6c669045d 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/CommentPickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/CommentPickler.scala @@ -9,36 +9,43 @@ import dotty.tools.tasty.TastyFormat.CommentsSection import java.nio.charset.StandardCharsets -class CommentPickler(pickler: TastyPickler, addrOfTree: tpd.Tree => Addr, docString: untpd.MemberDef => Option[Comment]): - private val buf = new TastyBuffer(5000) - pickler.newSection(CommentsSection, buf) - - def pickleComment(root: tpd.Tree): Unit = traverse(root) - - private def pickleComment(addr: Addr, comment: Comment): Unit = - if addr != NoAddr then - val bytes = comment.raw.getBytes(StandardCharsets.UTF_8).nn - val length = bytes.length - buf.writeAddr(addr) - buf.writeNat(length) - buf.writeBytes(bytes, length) - buf.writeLongInt(comment.span.coords) - - private def traverse(x: Any): Unit = x match - case x: untpd.Tree @unchecked => - x match - case x: tpd.MemberDef @unchecked => // at this point all MembderDefs are t(y)p(e)d. - for comment <- docString(x) do pickleComment(addrOfTree(x), comment) - case _ => - val limit = x.productArity - var n = 0 - while n < limit do - traverse(x.productElement(n)) - n += 1 - case y :: ys => - traverse(y) - traverse(ys) - case _ => - +object CommentPickler: + + def pickleComments( + pickler: TastyPickler, + addrOfTree: PositionPickler.TreeToAddr, + docString: untpd.MemberDef => Option[Comment], + root: tpd.Tree, + buf: TastyBuffer = new TastyBuffer(5000)): Unit = + + pickler.newSection(CommentsSection, buf) + + def pickleComment(addr: Addr, comment: Comment): Unit = + if addr != NoAddr then + val bytes = comment.raw.getBytes(StandardCharsets.UTF_8).nn + val length = bytes.length + buf.writeAddr(addr) + buf.writeNat(length) + buf.writeBytes(bytes, length) + buf.writeLongInt(comment.span.coords) + + def traverse(x: Any): Unit = x match + case x: untpd.Tree @unchecked => + x match + case x: tpd.MemberDef @unchecked => // at this point all MembderDefs are t(y)p(e)d. + for comment <- docString(x) do pickleComment(addrOfTree(x), comment) + case _ => + val limit = x.productArity + var n = 0 + while n < limit do + traverse(x.productElement(n)) + n += 1 + case y :: ys => + traverse(y) + traverse(ys) + case _ => + + traverse(root) + end pickleComments end CommentPickler diff --git a/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala index 906cea0453c9..924b87bec003 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala @@ -16,25 +16,32 @@ import collection.mutable import util.Spans._ import reporting.Message -class PositionPickler( - pickler: TastyPickler, - addrOfTree: PositionPickler.TreeToAddr, - treeAnnots: untpd.MemberDef => List[tpd.Tree], - relativePathReference: String){ - +object PositionPickler: import ast.tpd._ - val buf: TastyBuffer = new TastyBuffer(5000) - pickler.newSection(PositionsSection, buf) - - private val pickledIndices = new mutable.BitSet + // Note: This could be just TreeToAddr => Addr if functions are specialized to value classes. + // We use a SAM type to avoid boxing of Addr + @FunctionalInterface + trait TreeToAddr: + def apply(x: untpd.Tree): Addr - def header(addrDelta: Int, hasStartDelta: Boolean, hasEndDelta: Boolean, hasPoint: Boolean): Int = { + def header(addrDelta: Int, hasStartDelta: Boolean, hasEndDelta: Boolean, hasPoint: Boolean): Int = def toInt(b: Boolean) = if (b) 1 else 0 (addrDelta << 3) | (toInt(hasStartDelta) << 2) | (toInt(hasEndDelta) << 1) | toInt(hasPoint) - } - def picklePositions(source: SourceFile, roots: List[Tree], warnings: mutable.ListBuffer[Message]): Unit = { + def picklePositions( + pickler: TastyPickler, + addrOfTree: TreeToAddr, + treeAnnots: untpd.MemberDef => List[tpd.Tree], + relativePathReference: String, + source: SourceFile, + roots: List[Tree], + warnings: mutable.ListBuffer[Message], + buf: TastyBuffer = new TastyBuffer(5000), + pickledIndices: mutable.BitSet = new mutable.BitSet) = + + pickler.newSection(PositionsSection, buf) + /** Pickle the number of lines followed by the length of each line */ def pickleLineOffsets(): Unit = { val content = source.content() @@ -129,10 +136,6 @@ class PositionPickler( } for (root <- roots) traverse(root, NoSource) - } -} -object PositionPickler: - // Note: This could be just TreeToAddr => Addr if functions are specialized to value classes. - // We use a SAM type to avoid boxing of Addr - @FunctionalInterface trait TreeToAddr: - def apply(x: untpd.Tree): Addr + end picklePositions +end PositionPickler + diff --git a/compiler/src/dotty/tools/dotc/core/tasty/ScratchData.scala b/compiler/src/dotty/tools/dotc/core/tasty/ScratchData.scala new file mode 100644 index 000000000000..b36c78a77ac6 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/core/tasty/ScratchData.scala @@ -0,0 +1,20 @@ +package dotty.tools.dotc.core.tasty +import dotty.tools.tasty.TastyBuffer +import collection.mutable +import java.util.Arrays + +class ScratchData: + var delta, delta1 = new Array[Int](0) + + val positionBuffer = new TastyBuffer(5000) + val pickledIndices = new mutable.BitSet + + val commentBuffer = new TastyBuffer(5000) + + def reset() = + assert(delta ne delta1) + assert(delta.length == delta1.length) + positionBuffer.reset() + pickledIndices.clear() + commentBuffer.reset() + diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyPickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyPickler.scala index aa657c393815..4f1e84ac9184 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyPickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyPickler.scala @@ -38,8 +38,9 @@ class TastyPickler(val rootCls: ClassSymbol) { nameBuffer.assemble() sections.foreach(_._2.assemble()) - val nameBufferHash = TastyHash.pjwHash64(nameBuffer.bytes) - val treeSectionHash +: otherSectionHashes = sections.map(x => TastyHash.pjwHash64(x._2.bytes)): @unchecked + val nameBufferHash = TastyHash.pjwHash64(nameBuffer.bytes, nameBuffer.length) + val treeSectionHash +: otherSectionHashes = + sections.map(x => TastyHash.pjwHash64(x._2.bytes, x._2.length)): @unchecked // Hash of name table and tree val uuidLow: Long = nameBufferHash ^ treeSectionHash diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala index f8f545304a32..d0f08379c114 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeBuffer.scala @@ -10,6 +10,7 @@ import TastyBuffer.{Addr, NoAddr, AddrWidth} import util.Util.bestFit import config.Printers.pickling import ast.untpd.Tree +import java.util.Arrays class TreeBuffer extends TastyBuffer(50000) { @@ -17,7 +18,6 @@ class TreeBuffer extends TastyBuffer(50000) { private val initialOffsetSize = bytes.length / (AddrWidth * ItemsOverOffsets) private var offsets = new Array[Int](initialOffsetSize) private var isRelative = new Array[Boolean](initialOffsetSize) - private var delta: Array[Int] = _ private var numOffsets = 0 /** A map from trees to the address at which a tree is pickled. */ @@ -68,109 +68,119 @@ class TreeBuffer extends TastyBuffer(50000) { } /** The amount by which the bytes at the given address are shifted under compression */ - def deltaAt(at: Addr): Int = { + def deltaAt(at: Addr, scratch: ScratchData): Int = { val idx = bestFit(offsets, numOffsets, at.index - 1) - if (idx < 0) 0 else delta(idx) + if (idx < 0) 0 else scratch.delta(idx) } /** The address to which `x` is translated under compression */ - def adjusted(x: Addr): Addr = x - deltaAt(x) + def adjusted(x: Addr, scratch: ScratchData): Addr = x - deltaAt(x, scratch) + + /** Final assembly, involving the following steps: + * - compute deltas + * - adjust deltas until additional savings are < 1% of total + * - adjust offsets according to the adjusted deltas + * - shrink buffer, skipping zeroes. + */ + def compactify(scratch: ScratchData): Unit = + + def reserve(arr: Array[Int]) = + if arr.length < numOffsets then + new Array[Int](numOffsets) + else + Arrays.fill(arr, 0, numOffsets, 0) + arr /** Compute all shift-deltas */ - private def computeDeltas() = { - delta = new Array[Int](numOffsets) - var lastDelta = 0 - var i = 0 - while (i < numOffsets) { - val off = offset(i) - val skippedOff = skipZeroes(off) - val skippedCount = skippedOff.index - off.index - assert(skippedCount < AddrWidth, s"unset field at position $off") - lastDelta += skippedCount - delta(i) = lastDelta - i += 1 + def computeDeltas() = { + scratch.delta = reserve(scratch.delta) + var lastDelta = 0 + var i = 0 + while (i < numOffsets) { + val off = offset(i) + val skippedOff = skipZeroes(off) + val skippedCount = skippedOff.index - off.index + assert(skippedCount < AddrWidth, s"unset field at position $off") + lastDelta += skippedCount + scratch.delta(i) = lastDelta + i += 1 + } } - } - /** The absolute or relative adjusted address at index `i` of `offsets` array*/ - private def adjustedOffset(i: Int): Addr = { - val at = offset(i) - val original = getAddr(at) - if (isRelative(i)) { - val start = skipNat(at) - val len1 = original + delta(i) - deltaAt(original + start.index) - val len2 = adjusted(original + start.index) - adjusted(start).index - assert(len1 == len2, - s"adjusting offset #$i: $at, original = $original, len1 = $len1, len2 = $len2") - len1 + /** The absolute or relative adjusted address at index `i` of `offsets` array*/ + def adjustedOffset(i: Int): Addr = { + val at = offset(i) + val original = getAddr(at) + if (isRelative(i)) { + val start = skipNat(at) + val len1 = original + scratch.delta(i) - deltaAt(original + start.index, scratch) + val len2 = adjusted(original + start.index, scratch) - adjusted(start, scratch).index + assert(len1 == len2, + s"adjusting offset #$i: $at, original = $original, len1 = $len1, len2 = $len2") + len1 + } + else adjusted(original, scratch) } - else adjusted(original) - } - /** Adjust all offsets according to previously computed deltas */ - private def adjustOffsets(): Unit = - var i = 0 - while i < numOffsets do - val corrected = adjustedOffset(i) - fillAddr(offset(i), corrected) - i += 1 - - /** Adjust deltas to also take account references that will shrink (and thereby - * generate additional zeroes that can be skipped) due to previously - * computed adjustments. - */ - private def adjustDeltas(): Int = { - val delta1 = new Array[Int](delta.length) - var lastDelta = 0 - var i = 0 - while i < numOffsets do - val corrected = adjustedOffset(i) - lastDelta += AddrWidth - TastyBuffer.natSize(corrected.index) - delta1(i) = lastDelta - i += 1 - val saved = - if (numOffsets == 0) 0 - else delta1(numOffsets - 1) - delta(numOffsets - 1) - delta = delta1 - saved - } + /** Adjust all offsets according to previously computed deltas */ + def adjustOffsets(): Unit = + var i = 0 + while i < numOffsets do + val corrected = adjustedOffset(i) + fillAddr(offset(i), corrected) + i += 1 + + /** Adjust deltas to also take account references that will shrink (and thereby + * generate additional zeroes that can be skipped) due to previously + * computed adjustments. + */ + def adjustDeltas(): Int = { + scratch.delta1 = reserve(scratch.delta1) + var lastDelta = 0 + var i = 0 + while i < numOffsets do + val corrected = adjustedOffset(i) + lastDelta += AddrWidth - TastyBuffer.natSize(corrected.index) + scratch.delta1(i) = lastDelta + i += 1 + val saved = + if (numOffsets == 0) 0 + else scratch.delta1(numOffsets - 1) - scratch.delta(numOffsets - 1) + val tmp = scratch.delta + scratch.delta = scratch.delta1 + scratch.delta1 = tmp + saved + } - /** Compress pickle buffer, shifting bytes to close all skipped zeroes. */ - private def compress(): Int = { - var lastDelta = 0 - var start = 0 - var i = 0 - var wasted = 0 - def shift(end: Int) = - System.arraycopy(bytes, start, bytes, start - lastDelta, end - start) - while (i < numOffsets) { - val next = offsets(i) - shift(next) - start = next + delta(i) - lastDelta - val pastZeroes = skipZeroes(Addr(next)).index - assert(pastZeroes >= start, s"something's wrong: eliminated non-zero") - wasted += (pastZeroes - start) - lastDelta = delta(i) - i += 1 + /** Compress pickle buffer, shifting bytes to close all skipped zeroes. */ + def compress(): Int = { + var lastDelta = 0 + var start = 0 + var i = 0 + var wasted = 0 + def shift(end: Int) = + System.arraycopy(bytes, start, bytes, start - lastDelta, end - start) + while (i < numOffsets) { + val next = offsets(i) + shift(next) + start = next + scratch.delta(i) - lastDelta + val pastZeroes = skipZeroes(Addr(next)).index + assert(pastZeroes >= start, s"something's wrong: eliminated non-zero") + wasted += (pastZeroes - start) + lastDelta = scratch.delta(i) + i += 1 + } + shift(length) + length -= lastDelta + wasted } - shift(length) - length -= lastDelta - wasted - } - def adjustTreeAddrs(): Unit = - var i = 0 - while i < treeAddrs.size do - treeAddrs.setValue(i, adjusted(Addr(treeAddrs.value(i))).index) - i += 1 + def adjustTreeAddrs(): Unit = + var i = 0 + while i < treeAddrs.size do + treeAddrs.setValue(i, adjusted(Addr(treeAddrs.value(i)), scratch).index) + i += 1 - /** Final assembly, involving the following steps: - * - compute deltas - * - adjust deltas until additional savings are < 1% of total - * - adjust offsets according to the adjusted deltas - * - shrink buffer, skipping zeroes. - */ - def compactify(): Unit = { val origLength = length computeDeltas() //println(s"offsets: ${offsets.take(numOffsets).deep}") @@ -185,5 +195,5 @@ class TreeBuffer extends TastyBuffer(50000) { adjustTreeAddrs() val wasted = compress() pickling.println(s"original length: $origLength, compressed to: $length, wasted: $wasted") // DEBUG, for now. - } + end compactify } diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala index 8895ae90950b..eba7356286da 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala @@ -812,8 +812,8 @@ class TreePickler(pickler: TastyPickler) { assert(forwardSymRefs.isEmpty, i"unresolved symbols: $missing%, % when pickling ${ctx.source}") } - def compactify(): Unit = { - buf.compactify() + def compactify(scratch: ScratchData = new ScratchData): Unit = { + buf.compactify(scratch) def updateMapWithDeltas(mp: MutableSymbolMap[Addr]) = val keys = new Array[Symbol](mp.size) @@ -826,7 +826,7 @@ class TreePickler(pickler: TastyPickler) { i = 0 while i < keys.length do val key = keys(i) - mp(key) = adjusted(mp(key)) + mp(key) = adjusted(mp(key), scratch) i += 1 updateMapWithDeltas(symRefs) diff --git a/compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala b/compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala index 5966d6b817ee..20bcba417a5e 100644 --- a/compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala +++ b/compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala @@ -223,8 +223,8 @@ object PickledQuotes { if tree.span.exists then val positionWarnings = new mutable.ListBuffer[Message]() val reference = ctx.settings.sourceroot.value - new PositionPickler(pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots, reference) - .picklePositions(ctx.compilationUnit.source, tree :: Nil, positionWarnings) + PositionPickler.picklePositions(pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots, reference, + ctx.compilationUnit.source, tree :: Nil, positionWarnings) positionWarnings.foreach(report.warning(_)) val pickled = pickler.assembleParts() diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 52cd59c9483a..2433071213d5 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -48,7 +48,7 @@ class Pickler extends Phase { // Maps that keep a record if -Ytest-pickler is set. private val beforePickling = new mutable.HashMap[ClassSymbol, String] - private val picklers = new mutable.HashMap[ClassSymbol, TastyPickler] + private val pickledBytes = new mutable.HashMap[ClassSymbol, Array[Byte]] /** Drop any elements of this list that are linked module classes of other elements in the list */ private def dropCompanionModuleClasses(clss: List[ClassSymbol])(using Context): List[ClassSymbol] = { @@ -57,6 +57,19 @@ class Pickler extends Phase { clss.filterNot(companionModuleClasses.contains) } + /** Runs given functions with a scratch data block in a serialized fashion (i.e. + * inside a synchronized block). Scratch data is re-used between calls. + * Used to conserve on memory usage by avoiding to create scratch data for each + * pickled unit. + */ + object serialized: + val scratch = new ScratchData + def run(body: ScratchData => Array[Byte]): Array[Byte] = + synchronized { + scratch.reset() + body(scratch) + } + override def run(using Context): Unit = { val unit = ctx.compilationUnit pickling.println(i"unpickling in run ${ctx.runId}") @@ -65,25 +78,30 @@ class Pickler extends Phase { cls <- dropCompanionModuleClasses(topLevelClasses(unit.tpdTree)) tree <- sliceTopLevel(unit.tpdTree, cls) do + if ctx.settings.YtestPickler.value then beforePickling(cls) = tree.show + val pickler = new TastyPickler(cls) - if ctx.settings.YtestPickler.value then - beforePickling(cls) = tree.show - picklers(cls) = pickler val treePkl = new TreePickler(pickler) treePkl.pickle(tree :: Nil) Profile.current.recordTasty(treePkl.buf.length) + val positionWarnings = new mutable.ListBuffer[Message]() - val pickledF = inContext(ctx.fresh) { - Future { - treePkl.compactify() + def reportPositionWarnings() = positionWarnings.foreach(report.warning(_)) + + def computePickled(): Array[Byte] = inContext(ctx.fresh) { + serialized.run { scratch => + treePkl.compactify(scratch) if tree.span.exists then val reference = ctx.settings.sourceroot.value - new PositionPickler(pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots, reference) - .picklePositions(unit.source, tree :: Nil, positionWarnings) + PositionPickler.picklePositions( + pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots, reference, + unit.source, tree :: Nil, positionWarnings, + scratch.positionBuffer, scratch.pickledIndices) if !ctx.settings.YdropComments.value then - new CommentPickler(pickler, treePkl.buf.addrOfTree, treePkl.docString) - .pickleComment(tree) + CommentPickler.pickleComments( + pickler, treePkl.buf.addrOfTree, treePkl.docString, tree, + scratch.commentBuffer) val pickled = pickler.assembleParts() @@ -94,21 +112,29 @@ class Pickler extends Phase { // println(i"rawBytes = \n$rawBytes%\n%") // DEBUG if pickling ne noPrinter then - pickling.synchronized { - println(i"**** pickled info of $cls") - println(TastyPrinter.showContents(pickled, ctx.settings.color.value == "never")) - } + println(i"**** pickled info of $cls") + println(TastyPrinter.showContents(pickled, ctx.settings.color.value == "never")) pickled - }(using ExecutionContext.global) + } } - def force(): Array[Byte] = - val result = Await.result(pickledF, Duration.Inf) - positionWarnings.foreach(report.warning(_)) - result - - if !Pickler.ParallelPickling || ctx.settings.YtestPickler.value then force() - unit.pickled += (cls -> force) + /** A function that returns the pickled bytes. Depending on `Pickler.ParallelPickling` + * either computes the pickled data in a future or eagerly before constructing the + * function value. + */ + val demandPickled: () => Array[Byte] = + if Pickler.ParallelPickling && !ctx.settings.YtestPickler.value then + val futurePickled = Future(computePickled())(using ExecutionContext.global) + () => + try Await.result(futurePickled, Duration.Inf) + finally reportPositionWarnings() + else + val pickled = computePickled() + reportPositionWarnings() + if ctx.settings.YtestPickler.value then pickledBytes(cls) = pickled + () => pickled + + unit.pickled += (cls -> demandPickled) end for } @@ -129,8 +155,8 @@ class Pickler extends Phase { pickling.println(i"testing unpickler at run ${ctx.runId}") ctx.initialize() val unpicklers = - for ((cls, pickler) <- picklers) yield { - val unpickler = new DottyUnpickler(pickler.assembleParts()) + for ((cls, bytes) <- pickledBytes) yield { + val unpickler = new DottyUnpickler(bytes) unpickler.enter(roots = Set.empty) cls -> unpickler } diff --git a/tasty/src/dotty/tools/tasty/TastyBuffer.scala b/tasty/src/dotty/tools/tasty/TastyBuffer.scala index 1d48027087f5..f9266cf23617 100644 --- a/tasty/src/dotty/tools/tasty/TastyBuffer.scala +++ b/tasty/src/dotty/tools/tasty/TastyBuffer.scala @@ -193,4 +193,9 @@ class TastyBuffer(initialSize: Int) { * After `assemble` no more output actions to this buffer are permitted. */ def assemble(): Unit = () + + def reset(): Unit = { + java.util.Arrays.fill(bytes, 0, length, 0.toByte) + length = 0 + } } diff --git a/tasty/src/dotty/tools/tasty/TastyHash.scala b/tasty/src/dotty/tools/tasty/TastyHash.scala index aff663f42a8d..701328d578a3 100644 --- a/tasty/src/dotty/tools/tasty/TastyHash.scala +++ b/tasty/src/dotty/tools/tasty/TastyHash.scala @@ -6,10 +6,10 @@ object TastyHash { * * from https://en.wikipedia.org/wiki/PJW_hash_function#Algorithm */ - def pjwHash64(data: Array[Byte]): Long = { + def pjwHash64(data: Array[Byte], length: Int): Long = { var h = 0L var i = 0 - while (i < data.length) { + while (i < length) { val d = data(i) & 0xFFL // Interpret byte as unsigned byte h = (h << 8) + d val high = h & 0xFF00000000000000L @@ -19,4 +19,6 @@ object TastyHash { } h } + def pjwHash64(data: Array[Byte]): Long = + pjwHash64(data, data.length) } From 8dee6795bb30e1822c0557de2274f4450bbe2e68 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 22 Dec 2022 12:25:17 +0100 Subject: [PATCH 07/28] Reduce string computations - Cache selectorNames - Avoid redundant expensive string computation in Definitions.FunType --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 2 +- compiler/src/dotty/tools/dotc/core/StdNames.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 8984c87ec8de..be49fe02e770 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -1415,8 +1415,8 @@ class Definitions { val classRefs1 = new Array[TypeRef | Null](classRefs.length * 2) Array.copy(classRefs, 0, classRefs1, 0, classRefs.length) classRefs = classRefs1 - val funName = s"scala.$prefix$n" if classRefs(n) == null then + val funName = s"scala.$prefix$n" classRefs(n) = if prefix.startsWith("Impure") then staticRef(funName.toTypeName).symbol.typeRef diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index dff423fd0bb4..cc11474ea038 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -826,7 +826,7 @@ object StdNames { def newBitmapName(bitmapPrefix: TermName, n: Int): TermName = bitmapPrefix ++ n.toString - def selectorName(n: Int): TermName = "_" + (n + 1) + def selectorName(n: Int): TermName = productAccessorName(n + 1) object primitive { val arrayApply: TermName = "[]apply" From ac95795ee37371615c80193719c48aeecc1ebf56 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 22 Dec 2022 11:09:24 +0100 Subject: [PATCH 08/28] Avoid recomputing hot requiredMethods We could also make other required methods lazy vals instead of defs. The `newArray` method showed up in the profiles that's why it was changed first. --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index be49fe02e770..aaebae5f30b3 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -518,8 +518,8 @@ class Definitions { def staticsMethod(name: PreName): TermSymbol = ScalaStaticsModule.requiredMethod(name) @tu lazy val DottyArraysModule: Symbol = requiredModule("scala.runtime.Arrays") - def newGenericArrayMethod(using Context): TermSymbol = DottyArraysModule.requiredMethod("newGenericArray") - def newArrayMethod(using Context): TermSymbol = DottyArraysModule.requiredMethod("newArray") + lazy val newGenericArrayMethod: TermSymbol = DottyArraysModule.requiredMethod("newGenericArray") + lazy val newArrayMethod: TermSymbol = DottyArraysModule.requiredMethod("newArray") def getWrapVarargsArrayModule: Symbol = ScalaRuntimeModule From 7036ed808cee80aab9d44bfe743ea02832b969e9 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 16:57:55 +0100 Subject: [PATCH 09/28] Don't have Diagnostic inherit from Exception Creating diagnostics should be cheap, whereas reportiong them can be expensive. The reason is that often diagnsotics are created nd then later discarded in normal backtracking during Typer. But the way it was set up, every diagnostic computed a stack trace, which is quite expensive. --- .../src/dotty/tools/dotc/reporting/Diagnostic.scala | 3 +-- .../dotty/tools/dotc/reporting/ThrowingReporter.scala | 10 +++++++--- compiler/src/dotty/tools/dotc/typer/Applications.scala | 2 +- tests/run-staging/i7142.scala | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/Diagnostic.scala b/compiler/src/dotty/tools/dotc/reporting/Diagnostic.scala index 6c8e9430e815..624aa93924e8 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Diagnostic.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Diagnostic.scala @@ -89,7 +89,7 @@ class Diagnostic( val msg: Message, val pos: SourcePosition, val level: Int -) extends Exception with interfaces.Diagnostic: +) extends interfaces.Diagnostic: private var verbose: Boolean = false def isVerbose: Boolean = verbose def setVerbose(): this.type = @@ -104,5 +104,4 @@ class Diagnostic( Collections.emptyList() override def toString: String = s"$getClass at $pos: $message" - override def getMessage(): String = message end Diagnostic diff --git a/compiler/src/dotty/tools/dotc/reporting/ThrowingReporter.scala b/compiler/src/dotty/tools/dotc/reporting/ThrowingReporter.scala index ad47a9d30536..153212522541 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ThrowingReporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ThrowingReporter.scala @@ -6,12 +6,16 @@ import core.Contexts._ import Diagnostic.Error /** - * This class implements a Reporter that throws all errors and sends warnings and other - * info to the underlying reporter. + * This class implements a Reporter that throws all errors as UnhandledError exceptions + * and sends warnings and other info to the underlying reporter. */ class ThrowingReporter(reportInfo: Reporter) extends Reporter { def doReport(dia: Diagnostic)(using Context): Unit = dia match { - case _: Error => throw dia + case dia: Error => throw UnhandledError(dia) case _ => reportInfo.doReport(dia) } } + +class UnhandledError(val diagnostic: Error) extends Exception: + override def getMessage = diagnostic.message + diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 3482f80389d8..cd33fe9cef24 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -2407,7 +2407,7 @@ trait Applications extends Compatibility { else None catch - case NonFatal(_) => None + case ex: UnhandledError => None def isApplicableExtensionMethod(methodRef: TermRef, receiverType: Type)(using Context): Boolean = methodRef.symbol.is(ExtensionMethod) && !receiverType.isBottomType && diff --git a/tests/run-staging/i7142.scala b/tests/run-staging/i7142.scala index 5ec9b0785d30..5ea9f4e98214 100644 --- a/tests/run-staging/i7142.scala +++ b/tests/run-staging/i7142.scala @@ -7,7 +7,7 @@ object Test { def main(args: Array[String]): Unit = try run {returning('{ { (x: Int) => ${ throwReturn('x) }} apply 0 })} catch { - case ex: dotty.tools.dotc.reporting.Diagnostic.Error => + case ex: dotty.tools.dotc.reporting.UnhandledError => assert(ex.getMessage == "While expanding a macro, a reference to parameter x was used outside the scope where it was defined", ex.getMessage) } } From f8e7f7802ee541f495ae96c6b607f2153957e3da Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 16:59:28 +0100 Subject: [PATCH 10/28] Reuse regex matcher in replaceAll calls Regex compilation is expensive, so we should re-use the matcher over multiple replaceAll calls in: - StdNames.str.sanitize - Text's lengthWithoutAnsi --- compiler/src/dotty/tools/dotc/core/StdNames.scala | 5 ++++- compiler/src/dotty/tools/dotc/printing/Texts.scala | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index cc11474ea038..448a35398082 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -3,6 +3,7 @@ package core import scala.collection.mutable import scala.annotation.switch +import scala.annotation.internal.sharable import Names._ import Symbols._ import Contexts._ @@ -40,7 +41,9 @@ object StdNames { inline val Tuple = "Tuple" inline val Product = "Product" - def sanitize(str: String): String = str.replaceAll("""[<>]""", """\$""").nn + @sharable + private val disallowed = java.util.regex.Pattern.compile("""[<>]""").nn + def sanitize(str: String): String = disallowed.matcher(str).nn.replaceAll("""\$""").nn } abstract class DefinedNames[N <: Name] { diff --git a/compiler/src/dotty/tools/dotc/printing/Texts.scala b/compiler/src/dotty/tools/dotc/printing/Texts.scala index 411fa74ebffa..7c040a78de5e 100644 --- a/compiler/src/dotty/tools/dotc/printing/Texts.scala +++ b/compiler/src/dotty/tools/dotc/printing/Texts.scala @@ -1,8 +1,12 @@ package dotty.tools.dotc package printing +import scala.annotation.internal.sharable object Texts { + @sharable + private val ansi = java.util.regex.Pattern.compile("\u001b\\[\\d+m").nn + sealed abstract class Text { protected def indentMargin: Int = 2 @@ -70,7 +74,7 @@ object Texts { else appendIndented(that)(width) private def lengthWithoutAnsi(str: String): Int = - str.replaceAll("\u001b\\[\\d+m", "").nn.length + ansi.matcher(str).nn.replaceAll("").nn.length def layout(width: Int): Text = this match { case Str(s, _) => From e0e703ac1462647f6f0aa67d249957950735e8a1 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 22 Dec 2022 12:56:25 +0100 Subject: [PATCH 11/28] Add specialized versions of tasty.Util.dble for common array element types --- tasty/src/dotty/tools/tasty/util/Util.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tasty/src/dotty/tools/tasty/util/Util.scala b/tasty/src/dotty/tools/tasty/util/Util.scala index 5726e65773b0..750f5956c5cc 100644 --- a/tasty/src/dotty/tools/tasty/util/Util.scala +++ b/tasty/src/dotty/tools/tasty/util/Util.scala @@ -11,4 +11,17 @@ object Util { arr1 } + /** Specialized version for bytes */ + def dble(arr: Array[Byte]): Array[Byte] = { + val arr1 = new Array[Byte](arr.length * 2) + System.arraycopy(arr, 0, arr1, 0, arr.length) + arr1 + } + + /** Specialized version for ints */ + def dble(arr: Array[Int]): Array[Int] = { + val arr1 = new Array[Int](arr.length * 2) + System.arraycopy(arr, 0, arr1, 0, arr.length) + arr1 + } } From c9a4670f3e346edba9e32ec7c657c7c1bc679bd6 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 17:22:47 +0100 Subject: [PATCH 12/28] Allow to reuse table of a util.{MutableHashSet,MutableHashMap} On demand, fill the array with zeroes instead of creating a fresh one. This can save some array allocations. --- .../src/dotty/tools/dotc/core/OrderingConstraint.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 2 +- compiler/src/dotty/tools/dotc/util/GenericHashMap.scala | 5 +++-- compiler/src/dotty/tools/dotc/util/HashSet.scala | 7 +++---- compiler/src/dotty/tools/dotc/util/MutableMap.scala | 6 +++++- compiler/src/dotty/tools/dotc/util/MutableSet.scala | 6 +++++- compiler/src/dotty/tools/dotc/util/WeakHashSet.scala | 2 +- 7 files changed, 19 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 4a4eee263e70..212b70336f4b 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -360,7 +360,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def adjustReferenced(bound: Type, isLower: Boolean, add: Boolean) = adjuster.variance = if isLower then 1 else -1 adjuster.add = add - adjuster.seen.clear() + adjuster.seen.clear(resetToInitial = false) adjuster.traverse(bound) /** Use an optimized strategy to adjust dependencies to account for the delta diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 3e14292419a5..3e0e7dd5879d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -630,7 +630,7 @@ trait ImplicitRunInfo: def apply(tp: Type): collection.Set[Type] = parts = mutable.LinkedHashSet() - partSeen.clear() + partSeen.clear(resetToInitial = false) traverse(tp) parts end collectParts diff --git a/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala b/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala index fd6518fcc15c..a21a4af37038 100644 --- a/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala +++ b/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala @@ -42,9 +42,10 @@ abstract class GenericHashMap[Key, Value] else 1 << (32 - Integer.numberOfLeadingZeros(n)) /** Remove all elements from this table and set back to initial configuration */ - def clear(): Unit = + def clear(resetToInitial: Boolean): Unit = used = 0 - allocate(roundToPower(initialCapacity)) + if resetToInitial then allocate(roundToPower(initialCapacity)) + else java.util.Arrays.fill(table, null) /** The number of elements in the set */ def size: Int = used diff --git a/compiler/src/dotty/tools/dotc/util/HashSet.scala b/compiler/src/dotty/tools/dotc/util/HashSet.scala index a524dd39a594..a6e1532c804f 100644 --- a/compiler/src/dotty/tools/dotc/util/HashSet.scala +++ b/compiler/src/dotty/tools/dotc/util/HashSet.scala @@ -44,11 +44,10 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu else if Integer.bitCount(n) == 1 then n else 1 << (32 - Integer.numberOfLeadingZeros(n)) - /** Remove all elements from this set and set back to initial configuration */ - def clear(): Unit = { + def clear(resetToInitial: Boolean): Unit = used = 0 - allocate(roundToPower(initialCapacity)) - } + if resetToInitial then allocate(roundToPower(initialCapacity)) + else java.util.Arrays.fill(table, null) /** The number of elements in the set */ def size: Int = used diff --git a/compiler/src/dotty/tools/dotc/util/MutableMap.scala b/compiler/src/dotty/tools/dotc/util/MutableMap.scala index ba912a312aea..283e28e7e04f 100644 --- a/compiler/src/dotty/tools/dotc/util/MutableMap.scala +++ b/compiler/src/dotty/tools/dotc/util/MutableMap.scala @@ -13,6 +13,10 @@ abstract class MutableMap[Key, Value] extends ReadOnlyMap[Key, Value]: remove(k) this - def clear(): Unit + /** Remove all bindings from this map. + * @param resetToInitial If true, set back to initial configuration, which includes + * reallocating tables. + */ + def clear(resetToInitial: Boolean = true): Unit def getOrElseUpdate(key: Key, value: => Value): Value diff --git a/compiler/src/dotty/tools/dotc/util/MutableSet.scala b/compiler/src/dotty/tools/dotc/util/MutableSet.scala index 6e3ae7628eb6..9529262fa5ec 100644 --- a/compiler/src/dotty/tools/dotc/util/MutableSet.scala +++ b/compiler/src/dotty/tools/dotc/util/MutableSet.scala @@ -15,7 +15,11 @@ abstract class MutableSet[T] extends ReadOnlySet[T]: /** Remove element `x` from the set */ def -=(x: T): Unit - def clear(): Unit + /** Remove all elements from this set. + * @param resetToInitial If true, set back to initial configuration, which includes + * reallocating tables. + */ + def clear(resetToInitial: Boolean = true): Unit def ++= (xs: IterableOnce[T]): Unit = xs.iterator.foreach(this += _) diff --git a/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala b/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala index 3c23b181a041..975826a87a37 100644 --- a/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala +++ b/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala @@ -204,7 +204,7 @@ abstract class WeakHashSet[A <: AnyRef](initialCapacity: Int = 8, loadFactor: Do linkedListLoop(null, table(bucket)) } - def clear(): Unit = { + def clear(resetToInitial: Boolean): Unit = { table = new Array[Entry[A] | Null](table.size) threshold = computeThreshold count = 0 From e459edaa9f3c30f2c1a5507f695dbddb24ed97bb Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 22 Dec 2022 16:59:15 +0100 Subject: [PATCH 13/28] Avoid some Some wrappers when accessing maps --- .../tools/dotc/core/tasty/TreePickler.scala | 34 +++++++++---------- .../tools/dotc/transform/PatternMatcher.scala | 8 ++--- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala index eba7356286da..bef28545592a 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala @@ -71,15 +71,12 @@ class TreePickler(pickler: TastyPickler) { case _ => } - def registerDef(sym: Symbol): Unit = { + def registerDef(sym: Symbol): Unit = symRefs(sym) = currentAddr - forwardSymRefs.get(sym) match { - case Some(refs) => - refs.foreach(fillRef(_, currentAddr, relative = false)) - forwardSymRefs -= sym - case None => - } - } + val refs = forwardSymRefs.lookup(sym) + if refs != null then + refs.foreach(fillRef(_, currentAddr, relative = false)) + forwardSymRefs -= sym def pickleName(name: Name): Unit = writeNat(nameIndex(name).index) @@ -88,17 +85,19 @@ class TreePickler(pickler: TastyPickler) { if (sig eq Signature.NotAMethod) name else SignedName(name.toTermName, sig, target.asTermName)) - private def pickleSymRef(sym: Symbol)(using Context) = symRefs.get(sym) match { - case Some(label) => - if (label != NoAddr) writeRef(label) else pickleForwardSymRef(sym) - case None => + private def pickleSymRef(sym: Symbol)(using Context) = + val label: Addr | Null = symRefs.lookup(sym) + if label == null then // See pos/t1957.scala for an example where this can happen. // I believe it's a bug in typer: the type of an implicit argument refers // to a closure parameter outside the closure itself. TODO: track this down, so that we // can eliminate this case. report.log(i"pickling reference to as yet undefined $sym in ${sym.owner}", sym.srcPos) pickleForwardSymRef(sym) - } + else if label == NoAddr then + pickleForwardSymRef(sym) + else + writeRef(label.uncheckedNN) // !!! Dotty problem: Not clear why nn or uncheckedNN is needed here private def pickleForwardSymRef(sym: Symbol)(using Context) = { val ref = reserveRef(relative = false) @@ -351,11 +350,10 @@ class TreePickler(pickler: TastyPickler) { throw ex if sym.is(Method) && sym.owner.isClass then profile.recordMethodSize(sym, currentAddr.index - addr.index, mdef.span) - for - docCtx <- ctx.docCtx - comment <- docCtx.docstring(sym) - do - docStrings(mdef) = comment + for docCtx <- ctx.docCtx do + val comment = docCtx.docstrings.lookup(sym) + if comment != null then + docStrings(mdef) = comment } def pickleParam(tree: Tree)(using Context): Unit = { diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index 70fa0e5cc513..63ffdffbddef 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -664,12 +664,12 @@ object PatternMatcher { val refCount = varRefCount(plan) val LetPlan(topSym, _) = plan: @unchecked - def toDrop(sym: Symbol) = initializer.get(sym) match { - case Some(rhs) => + def toDrop(sym: Symbol) = + val rhs = initializer.lookup(sym) + if rhs != null then isPatmatGenerated(sym) && refCount(sym) <= 1 && sym != topSym && isPureExpr(rhs) - case none => + else false - } object Inliner extends PlanTransform { override val treeMap = new TreeMap { From 10359a5bcb7fd95e879191e336099f1ff3fa60dd Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 22 Dec 2022 18:04:23 +0100 Subject: [PATCH 14/28] Avoid unnecessary uses of `parentSyms` `parentSyms` maps all parent types. We don't need that if we just want to work on the superclass. --- .../tools/dotc/core/SymDenotations.scala | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index aad42b69e238..66b41f31843a 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1877,19 +1877,21 @@ object SymDenotations { super.info_=(tp) } - /** The symbols of the parent classes. */ - def parentSyms(using Context): List[Symbol] = info match { - case classInfo: ClassInfo => classInfo.declaredParents.map(_.classSymbol) + /** The types of the parent classes. */ + def parentTypes(using Context): List[Type] = info match + case classInfo: ClassInfo => classInfo.declaredParents case _ => Nil - } + + /** The symbols of the parent classes. */ + def parentSyms(using Context): List[Symbol] = + parentTypes.map(_.classSymbol) /** The symbol of the superclass, NoSymbol if no superclass exists */ - def superClass(using Context): Symbol = parentSyms match { - case parent :: _ => - if (parent.is(Trait)) NoSymbol else parent - case _ => - NoSymbol - } + def superClass(using Context): Symbol = parentTypes match + case parentType :: _ => + val parentCls = parentType.classSymbol + if parentCls.is(Trait) then NoSymbol else parentCls + case _ => NoSymbol /** The explicitly given self type (self types of modules are assumed to be * explcitly given here). @@ -1951,20 +1953,20 @@ object SymDenotations { def computeBaseData(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { def emptyParentsExpected = is(Package) || (symbol == defn.AnyClass) || ctx.erasedTypes && (symbol == defn.ObjectClass) - val psyms = parentSyms - if (psyms.isEmpty && !emptyParentsExpected) + val parents = parentTypes + if (parents.isEmpty && !emptyParentsExpected) onBehalf.signalProvisional() val builder = new BaseDataBuilder - def traverse(parents: List[Symbol]): Unit = parents match { + def traverse(parents: List[Type]): Unit = parents match { case p :: parents1 => - p match { + p.classSymbol match { case pcls: ClassSymbol => builder.addAll(pcls.baseClasses) case _ => assert(isRefinementClass || p.isError || ctx.mode.is(Mode.Interactive), s"$this has non-class parent: $p") } traverse(parents1) case nil => } - traverse(psyms) + traverse(parents) (classSymbol :: builder.baseClasses, builder.baseClassSet) } @@ -2313,9 +2315,11 @@ object SymDenotations { var names = Set[Name]() def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name try { - for (p <- parentSyms if p.isClass) - for (name <- p.asClass.memberNames(keepOnly)) - maybeAdd(name) + for ptype <- parentTypes do + ptype.classSymbol match + case pcls: ClassSymbol => + for name <- pcls.memberNames(keepOnly) do + maybeAdd(name) val ownSyms = if (keepOnly eq implicitFilter) if (this.is(Package)) Iterator.empty From 03555e8180f7ec3732b4edb3fbc20b246a2f2e64 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 17:29:58 +0100 Subject: [PATCH 15/28] Avoid some boxings of vars --- .../src/dotty/tools/dotc/ast/Positioned.scala | 19 ++++++++------ .../src/dotty/tools/dotc/ast/TreeInfo.scala | 10 ++++--- .../dotty/tools/dotc/parsing/Scanners.scala | 10 ++++--- .../dotty/tools/dotc/transform/Erasure.scala | 26 ++++++++++--------- .../src/dotty/tools/dotc/typer/Typer.scala | 14 +++++----- 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Positioned.scala b/compiler/src/dotty/tools/dotc/ast/Positioned.scala index d14addb8c9c7..dd783be7a9e1 100644 --- a/compiler/src/dotty/tools/dotc/ast/Positioned.scala +++ b/compiler/src/dotty/tools/dotc/ast/Positioned.scala @@ -154,14 +154,17 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Src } } + private class LastPosRef: + var positioned: Positioned | Null = null + var span = NoSpan + /** Check that all positioned items in this tree satisfy the following conditions: * - Parent spans contain child spans * - If item is a non-empty tree, it has a position */ def checkPos(nonOverlapping: Boolean)(using Context): Unit = try { import untpd._ - var lastPositioned: Positioned | Null = null - var lastSpan = NoSpan + val last = LastPosRef() def check(p: Any): Unit = p match { case p: Positioned => assert(span contains p.span, @@ -181,19 +184,19 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Src case _: XMLBlock => // FIXME: Trees generated by the XML parser do not satisfy `checkPos` case _: WildcardFunction - if lastPositioned.isInstanceOf[ValDef] && !p.isInstanceOf[ValDef] => + if last.positioned.isInstanceOf[ValDef] && !p.isInstanceOf[ValDef] => // ignore transition from last wildcard parameter to body case _ => - assert(!lastSpan.exists || !p.span.exists || lastSpan.end <= p.span.start, + assert(!last.span.exists || !p.span.exists || last.span.end <= p.span.start, i"""position error, child positions overlap or in wrong order |parent = $this - |1st child = $lastPositioned - |1st child span = $lastSpan + |1st child = ${last.positioned} + |1st child span = ${last.span} |2nd child = $p |2nd child span = ${p.span}""".stripMargin) } - lastPositioned = p - lastSpan = p.span + last.positioned = p + last.span = p.span p.checkPos(nonOverlapping) case m: untpd.Modifiers => m.annotations.foreach(check) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index c2cb65b90ec4..11f126fd659f 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -830,10 +830,12 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => /** The symbols defined locally in a statement list */ def localSyms(stats: List[Tree])(using Context): List[Symbol] = - val locals = new mutable.ListBuffer[Symbol] - for stat <- stats do - if stat.isDef && stat.symbol.exists then locals += stat.symbol - locals.toList + if stats.isEmpty then Nil + else + val locals = new mutable.ListBuffer[Symbol] + for stat <- stats do + if stat.isDef && stat.symbol.exists then locals += stat.symbol + locals.toList /** If `tree` is a DefTree, the symbol defined by it, otherwise NoSymbol */ def definedSym(tree: Tree)(using Context): Symbol = diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index 2d1dd22aa28b..58b36c8e1bb9 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala @@ -553,7 +553,7 @@ object Scanners { // If nextWidth is an indentation level not yet seen by enclosing indentation // region, invoke `handler`. - def handleNewIndentWidth(r: Region, handler: Indented => Unit): Unit = r match + inline def handleNewIndentWidth(r: Region, inline handler: Indented => Unit): Unit = r match case r @ Indented(curWidth, prefix, outer) if curWidth < nextWidth && !r.otherIndentWidths.contains(nextWidth) && nextWidth != lastWidth => handler(r) @@ -571,7 +571,7 @@ object Scanners { * they start with `(`, `[` or `{`, or the last statement ends in a `return`. * The Scala 2 rules apply under source `3.0-migration` or under `-no-indent`. */ - def isContinuing = + inline def isContinuing = lastWidth < nextWidth && (openParensTokens.contains(token) || lastToken == RETURN) && !pastBlankLine @@ -608,10 +608,11 @@ object Scanners { case r: Indented => insert(OUTDENT, offset) handleNewIndentWidth(r.enclosing, ir => + val lw = lastWidth errorButContinue( em"""The start of this line does not match any of the previous indentation widths. |Indentation width of current line : $nextWidth - |This falls between previous widths: ${ir.width} and $lastWidth""")) + |This falls between previous widths: ${ir.width} and $lw""")) case r => if skipping then if r.enclosing.isClosedByUndentAt(nextWidth) then @@ -627,7 +628,8 @@ object Scanners { else if lastToken == SELFARROW then currentRegion.knownWidth = nextWidth else if (lastWidth != nextWidth) - errorButContinue(spaceTabMismatchMsg(lastWidth, nextWidth)) + val lw = lastWidth + errorButContinue(spaceTabMismatchMsg(lw, nextWidth)) if token != OUTDENT then handleNewIndentWidth(currentRegion, _.otherIndentWidths += nextWidth) if next.token == EMPTY then diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index 557b2ac5a4c6..129964557995 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala @@ -698,18 +698,20 @@ object Erasure { return tree.asInstanceOf[Tree] // we are re-typing a primitive array op val owner = mapOwner(origSym) - var sym = if (owner eq origSym.maybeOwner) origSym else owner.info.decl(tree.name).symbol - if !sym.exists then - // We fail the sym.exists test for pos/i15158.scala, where we pass an infinitely - // recurring match type to an overloaded constructor. An equivalent test - // with regular apply methods succeeds. It's at present unclear whether - // - the program should be rejected, or - // - there is another fix. - // Therefore, we apply the fix to use the pre-erasure symbol, but only - // for constructors, in order not to mask other possible bugs that would - // trigger the assert(sym.exists, ...) below. - val prevSym = tree.symbol(using preErasureCtx) - if prevSym.isConstructor then sym = prevSym + val sym = + (if (owner eq origSym.maybeOwner) origSym else owner.info.decl(tree.name).symbol) + .orElse { + // We fail the sym.exists test for pos/i15158.scala, where we pass an infinitely + // recurring match type to an overloaded constructor. An equivalent test + // with regular apply methods succeeds. It's at present unclear whether + // - the program should be rejected, or + // - there is another fix. + // Therefore, we apply the fix to use the pre-erasure symbol, but only + // for constructors, in order not to mask other possible bugs that would + // trigger the assert(sym.exists, ...) below. + val prevSym = tree.symbol(using preErasureCtx) + if prevSym.isConstructor then prevSym else NoSymbol + } assert(sym.exists, i"no owner from $owner/${origSym.showLocated} in $tree") diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 3384c27c0194..1a24a94e527e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -244,15 +244,17 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer imp.importSym.info match case ImportType(expr) => val pre = expr.tpe - var denot = pre.memberBasedOnFlags(name, required, excluded) + val denot0 = pre.memberBasedOnFlags(name, required, excluded) .accessibleFrom(pre)(using refctx) // Pass refctx so that any errors are reported in the context of the // reference instead of the context of the import scope - if denot.exists then - if checkBounds then - denot = denot.filterWithPredicate { mbr => - mbr.matchesImportBound(if mbr.symbol.is(Given) then imp.givenBound else imp.wildcardBound) - } + if denot0.exists then + val denot = + if checkBounds then + denot0.filterWithPredicate { mbr => + mbr.matchesImportBound(if mbr.symbol.is(Given) then imp.givenBound else imp.wildcardBound) + } + else denot0 def isScalaJsPseudoUnion = denot.name == tpnme.raw.BAR && ctx.settings.scalajs.value && denot.symbol == JSDefinitions.jsdefn.PseudoUnionClass // Just like Scala2Unpickler reinterprets Scala.js pseudo-unions From 4e7ab60921b4fcc5d5fec4587e4381ab61a747ac Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 22 Dec 2022 10:58:43 +0100 Subject: [PATCH 16/28] Avoid creating large CharBuffers in LookaheadScanners There are many LookaheadScanner objects, and most don't need a CharBuffer for literals or comments at all. --- compiler/src/dotty/tools/dotc/parsing/Scanners.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index 58b36c8e1bb9..b3d824a2efd2 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala @@ -128,9 +128,11 @@ object Scanners { // Setting token data ---------------------------------------------------- + protected def initialCharBufferSize = 1024 + /** A character buffer for literals */ - protected val litBuf = CharBuffer() + protected val litBuf = CharBuffer(initialCharBufferSize) /** append Unicode character to "litBuf" buffer */ @@ -244,7 +246,7 @@ object Scanners { def getDocComment(pos: Int): Option[Comment] = docstringMap.get(pos) /** A buffer for comments */ - private val commentBuf = CharBuffer() + private val commentBuf = CharBuffer(initialCharBufferSize) def toToken(identifier: SimpleName): Token = def handleMigration(keyword: Token): Token = @@ -1079,6 +1081,7 @@ object Scanners { next class LookaheadScanner(val allowIndent: Boolean = false) extends Scanner(source, offset, allowIndent = allowIndent) { + override protected def initialCharBufferSize = 8 override def languageImportContext = Scanner.this.languageImportContext } From 50c159527c556c2dcfc02d087413808d48bfce8e Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 23 Dec 2022 15:53:17 +0100 Subject: [PATCH 17/28] Avoid expensive settings lookup in `setDenot` --- compiler/src/dotty/tools/dotc/Run.scala | 3 +++ compiler/src/dotty/tools/dotc/config/Config.scala | 7 ------- compiler/src/dotty/tools/dotc/core/Contexts.scala | 5 +++++ compiler/src/dotty/tools/dotc/core/Types.scala | 5 ++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index f7a08d1640ee..8cd1d160b42c 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -231,6 +231,9 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint ctx.settings.Yskip.value, ctx.settings.YstopBefore.value, stopAfter, ctx.settings.Ycheck.value) ctx.base.usePhases(phases) + if ctx.settings.YnoDoubleBindings.value then + ctx.base.checkNoDoubleBindings = true + def runPhases(using Context) = { var lastPrintedTree: PrintedTree = NoPrintedTree val profiler = ctx.profiler diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index cbd50429492e..247fa28efbda 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -83,13 +83,6 @@ object Config { */ inline val failOnInstantiationToNothing = false - /** Enable noDoubleDef checking if option "-YnoDoubleDefs" is set. - * The reason to have an option as well as the present global switch is - * that the noDoubleDef checking is done in a hotspot, and we do not - * want to incur the overhead of checking an option each time. - */ - inline val checkNoDoubleBindings = true - /** Check positions for consistency after parsing */ inline val checkPositions = true diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index b861cef43f61..5f82e8c8b6ce 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -986,6 +986,11 @@ object Contexts { /** Flag to suppress inlining, set after overflow */ private[dotc] var stopInlining: Boolean = false + /** Cached -Yno-double-bindings setting. This is accessed from `setDenot`, which + * is fairly hot, so we don't want to lookup the setting each time it is called. + */ + private[dotc] var checkNoDoubleBindings = false + /** A variable that records that some error was reported in a globally committable context. * The error will not necessarlily be emitted, since it could still be that * the enclosing context will be aborted. The variable is used as a smoke test diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index b8b6b7248ee2..26803f8a4e58 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2443,9 +2443,8 @@ object Types { setDenot(memberDenot(name, allowPrivate = !symbol.exists || symbol.is(Private))) private def setDenot(denot: Denotation)(using Context): Unit = { - if (Config.checkNoDoubleBindings) - if (ctx.settings.YnoDoubleBindings.value) - checkSymAssign(denot.symbol) + if ctx.base.checkNoDoubleBindings then + checkSymAssign(denot.symbol) lastDenotation = denot lastSymbol = denot.symbol From 9f13e56654da0902f9ef968a95c034320e6392f8 Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 23 Dec 2022 15:56:05 +0100 Subject: [PATCH 18/28] Make `validFor` monomorphic Profiles showed that it accounted for a significant percentage of vtable_stub time. --- .../dotty/tools/dotc/core/Denotations.scala | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index b34416a095db..bb8e764541d3 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -194,9 +194,6 @@ object Denotations { */ def infoOrCompleter: Type - /** The period during which this denotation is valid. */ - def validFor: Period - /** Is this a reference to a type symbol? */ def isType: Boolean @@ -229,6 +226,15 @@ object Denotations { */ def current(using Context): Denotation + /** The period during which this denotation is valid. */ + private var myValidFor: Period = Nowhere + + final def validFor: Period = myValidFor + final def validFor_=(p: Period): Unit = { + myValidFor = p + symbol.invalidateDenotCache() + } + /** Is this denotation different from NoDenotation or an ErrorDenotation? */ def exists: Boolean = true @@ -664,14 +670,6 @@ object Denotations { // ------ Transformations ----------------------------------------- - private var myValidFor: Period = Nowhere - - def validFor: Period = myValidFor - def validFor_=(p: Period): Unit = { - myValidFor = p - symbol.invalidateDenotCache() - } - /** The next SingleDenotation in this run, with wrap-around from last to first. * * There may be several `SingleDenotation`s with different validity @@ -695,7 +693,7 @@ object Denotations { if (validFor.firstPhaseId <= 1) this else { var current = nextInRun - while (current.validFor.code > this.myValidFor.code) current = current.nextInRun + while (current.validFor.code > this.validFor.code) current = current.nextInRun current } @@ -776,7 +774,7 @@ object Denotations { * are otherwise undefined. */ def skipRemoved(using Context): SingleDenotation = - if (myValidFor.code <= 0) nextDefined else this + if (validFor.code <= 0) nextDefined else this /** Produce a denotation that is valid for the given context. * Usually called when !(validFor contains ctx.period) @@ -793,7 +791,7 @@ object Denotations { def current(using Context): SingleDenotation = util.Stats.record("current") val currentPeriod = ctx.period - val valid = myValidFor + val valid = validFor def assertNotPackage(d: SingleDenotation, transformer: DenotTransformer) = d match case d: ClassDenotation => @@ -963,7 +961,7 @@ object Denotations { case denot: SymDenotation => s"in ${denot.owner}" case _ => "" } - s"stale symbol; $this#${symbol.id} $ownerMsg, defined in ${myValidFor}, is referred to in run ${ctx.period}" + s"stale symbol; $this#${symbol.id} $ownerMsg, defined in ${validFor}, is referred to in run ${ctx.period}" } /** The period (interval of phases) for which there exists @@ -1248,8 +1246,9 @@ object Denotations { /** An overloaded denotation consisting of the alternatives of both given denotations. */ case class MultiDenotation(denot1: Denotation, denot2: Denotation) extends Denotation(NoSymbol, NoType) with MultiPreDenotation { + validFor = denot1.validFor & denot2.validFor + final def infoOrCompleter: Type = multiHasNot("info") - final def validFor: Period = denot1.validFor & denot2.validFor final def isType: Boolean = false final def hasUniqueSym: Boolean = false final def name(using Context): Name = denot1.name From 84c4fe0c841cb5c703054e05dcf9ca2313dabafc Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 17:46:54 +0100 Subject: [PATCH 19/28] Cache `isType` in SymDenotations isType also made up for a significant part of vtable stubs. We now compute it when a Symbol is created and keep it around as a field. --- .../src/dotty/tools/dotc/core/Denotations.scala | 16 ++++++---------- .../dotty/tools/dotc/core/SymDenotations.scala | 6 +----- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index bb8e764541d3..56e9686a11a7 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -175,7 +175,7 @@ object Denotations { * * @param symbol The referencing symbol, or NoSymbol is none exists */ - abstract class Denotation(val symbol: Symbol, protected var myInfo: Type) extends PreDenotation with printing.Showable { + abstract class Denotation(val symbol: Symbol, protected var myInfo: Type, val isType: Boolean) extends PreDenotation with printing.Showable { type AsSeenFromResult <: Denotation /** The type info. @@ -194,9 +194,6 @@ object Denotations { */ def infoOrCompleter: Type - /** Is this a reference to a type symbol? */ - def isType: Boolean - /** Is this a reference to a term symbol? */ def isTerm: Boolean = !isType @@ -577,7 +574,7 @@ object Denotations { end infoMeet /** A non-overloaded denotation */ - abstract class SingleDenotation(symbol: Symbol, initInfo: Type) extends Denotation(symbol, initInfo) { + abstract class SingleDenotation(symbol: Symbol, initInfo: Type, isType: Boolean) extends Denotation(symbol, initInfo, isType) { protected def newLikeThis(symbol: Symbol, info: Type, pre: Type, isRefinedMethod: Boolean): SingleDenotation final def name(using Context): Name = symbol.name @@ -1147,9 +1144,9 @@ object Denotations { acc(false, symbol.info) } - abstract class NonSymSingleDenotation(symbol: Symbol, initInfo: Type, override val prefix: Type) extends SingleDenotation(symbol, initInfo) { + abstract class NonSymSingleDenotation(symbol: Symbol, initInfo: Type, override val prefix: Type) + extends SingleDenotation(symbol, initInfo, initInfo.isInstanceOf[TypeType]) { def infoOrCompleter: Type = initInfo - def isType: Boolean = infoOrCompleter.isInstanceOf[TypeType] } class UniqueRefDenotation( @@ -1245,11 +1242,10 @@ object Denotations { /** An overloaded denotation consisting of the alternatives of both given denotations. */ - case class MultiDenotation(denot1: Denotation, denot2: Denotation) extends Denotation(NoSymbol, NoType) with MultiPreDenotation { + case class MultiDenotation(denot1: Denotation, denot2: Denotation) extends Denotation(NoSymbol, NoType, isType = false) with MultiPreDenotation { validFor = denot1.validFor & denot2.validFor - + final def infoOrCompleter: Type = multiHasNot("info") - final def isType: Boolean = false final def hasUniqueSym: Boolean = false final def name(using Context): Name = denot1.name final def signature(using Context): Signature = Signature.OverloadedSignature diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 66b41f31843a..64171cdf422b 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -39,7 +39,7 @@ object SymDenotations { final val name: Name, initFlags: FlagSet, initInfo: Type, - initPrivateWithin: Symbol = NoSymbol) extends SingleDenotation(symbol, initInfo) { + initPrivateWithin: Symbol = NoSymbol) extends SingleDenotation(symbol, initInfo, name.isTypeName) { //assert(symbol.id != 4940, name) @@ -576,9 +576,6 @@ object SymDenotations { // ----- Tests ------------------------------------------------- - /** Is this denotation a type? */ - override def isType: Boolean = name.isTypeName - /** Is this denotation a class? */ final def isClass: Boolean = isInstanceOf[ClassDenotation] @@ -2574,7 +2571,6 @@ object SymDenotations { @sharable object NoDenotation extends SymDenotation(NoSymbol, NoSymbol, "".toTermName, Permanent, NoType) { - override def isType: Boolean = false override def isTerm: Boolean = false override def exists: Boolean = false override def owner: Symbol = throw new AssertionError("NoDenotation.owner") From b28faf99eac2b77a00665fb2d8752da783ba3f95 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 18:27:32 +0100 Subject: [PATCH 20/28] Streamline some hot compuations Make it cheaper to compute whether a Period is Nowhere, and also make the symbol and denot computations on NamedType as small as possible. --- .../src/dotty/tools/dotc/core/Periods.scala | 3 +- .../src/dotty/tools/dotc/core/Types.scala | 28 +++++++++---------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Periods.scala b/compiler/src/dotty/tools/dotc/core/Periods.scala index 44d83dcb5278..aaacd21db05d 100644 --- a/compiler/src/dotty/tools/dotc/core/Periods.scala +++ b/compiler/src/dotty/tools/dotc/core/Periods.scala @@ -118,7 +118,8 @@ object Periods { apply(rid, 0, PhaseMask) } - final val Nowhere: Period = new Period(0) + inline val NowhereCode = 0 + final val Nowhere: Period = new Period(NowhereCode) final val InitialPeriod: Period = Period(InitialRunId, FirstPhaseId) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 26803f8a4e58..fed1e167f70f 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2266,15 +2266,17 @@ object Types { final def symbol(using Context): Symbol = // We can rely on checkedPeriod (unlike in the definition of `denot` below) // because SymDenotation#installAfter never changes the symbol - if (checkedPeriod == ctx.period) lastSymbol.nn else computeSymbol + if (checkedPeriod == ctx.period) lastSymbol.asInstanceOf[Symbol] + else computeSymbol private def computeSymbol(using Context): Symbol = - designator match { + val result = designator match case sym: Symbol => if (sym.isValidInCurrentRun) sym else denot.symbol case name => - (if (denotationIsCurrent) lastDenotation.nn else denot).symbol - } + (if (denotationIsCurrent) lastDenotation.asInstanceOf[Denotation] else denot).symbol + if checkedPeriod.code != NowhereCode then checkedPeriod = ctx.period + result /** There is a denotation computed which is valid (somewhere in) the * current run. @@ -2306,18 +2308,16 @@ object Types { def info(using Context): Type = denot.info - /** The denotation currently denoted by this type */ - final def denot(using Context): Denotation = { + /** The denotation currently denoted by this type. Extremely hot. Carefully optimized + * to be as small as possible. + */ + final def denot(using Context): Denotation = util.Stats.record("NamedType.denot") - val now = ctx.period + val lastd = lastDenotation.asInstanceOf[Denotation] // Even if checkedPeriod == now we still need to recheck lastDenotation.validFor // as it may have been mutated by SymDenotation#installAfter - if (checkedPeriod != Nowhere && lastDenotation.nn.validFor.contains(now)) { - checkedPeriod = now - lastDenotation.nn - } + if checkedPeriod.code != NowhereCode && lastd.validFor.contains(ctx.period) then lastd else computeDenot - } private def computeDenot(using Context): Denotation = { util.Stats.record("NamedType.computeDenot") @@ -2353,11 +2353,11 @@ object Types { lastDenotation match { case lastd0: SingleDenotation => val lastd = lastd0.skipRemoved - if lastd.validFor.runId == ctx.runId && checkedPeriod != Nowhere then + if lastd.validFor.runId == ctx.runId && checkedPeriod.code != NowhereCode then finish(lastd.current) else lastd match { case lastd: SymDenotation => - if (stillValid(lastd) && (checkedPeriod != Nowhere)) finish(lastd.current) + if stillValid(lastd) && checkedPeriod.code != NowhereCode then finish(lastd.current) else finish(memberDenot(lastd.initial.name, allowPrivate = false)) case _ => fromDesignator From f992f7a447f50239c6b616155d88223f894c9fd0 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 18:30:18 +0100 Subject: [PATCH 21/28] Inline rollbackGadtUnless in GadtConstraint It's not that large, is only used twice, and inlining it saves two argument closures per call. --- compiler/src/dotty/tools/dotc/core/GadtConstraint.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala index 37c984b86934..7515898a36df 100644 --- a/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/GadtConstraint.scala @@ -209,7 +209,7 @@ sealed trait GadtConstraint ( this.reverseMapping = revMapping this.wasConstrained = wasConstrained - def rollbackGadtUnless(op: => Boolean): Boolean = + inline def rollbackGadtUnless(inline op: Boolean): Boolean = val savedConstr = myConstraint val savedMapping = mapping val savedReverseMapping = reverseMapping From c92089656b7613bbbf967f6b19e4b82d08bd3462 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 25 Dec 2022 12:58:36 +0100 Subject: [PATCH 22/28] Replace tpd.mapInline by flattenMapConserve The new version performs better also for long lists of trees. --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 49 ++++++++++--------- .../tools/dotc/transform/MegaPhase.scala | 2 +- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index d9d2c2ac4d4e..dd1e46c62223 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -1144,35 +1144,38 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { expand(tree, tree.tpe.widen) } - inline val MapRecursionLimit = 10 - extension (trees: List[Tree]) - /** A map that expands to a recursive function. It's equivalent to + /** Equivalent (but faster) to * * flatten(trees.mapConserve(op)) * - * and falls back to it after `MaxRecursionLimit` recursions. - * Before that it uses a simpler method that uses stackspace - * instead of heap. - * Note `op` is duplicated in the generated code, so it should be - * kept small. + * assuming that `trees` does not contain `Thicket`s to start with. */ - inline def mapInline(inline op: Tree => Tree): List[Tree] = - def recur(trees: List[Tree], count: Int): List[Tree] = - if count > MapRecursionLimit then - // use a slower implementation that avoids stack overflows - flatten(trees.mapConserve(op)) - else trees match - case tree :: rest => - val tree1 = op(tree) - val rest1 = recur(rest, count + 1) - if (tree1 eq tree) && (rest1 eq rest) then trees - else tree1 match - case Thicket(elems1) => elems1 ::: rest1 - case _ => tree1 :: rest1 - case nil => nil - recur(trees, 0) + inline def flattenedMapConserve(inline f: Tree => Tree): List[Tree] = + @tailrec + def loop(mapped: ListBuffer[Tree] | Null, unchanged: List[Tree], pending: List[Tree]): List[Tree] = + if pending.isEmpty then + if mapped == null then unchanged + else mapped.prependToList(unchanged) + else + val head0 = pending.head + val head1 = f(head0) + + if head1 eq head0 then + loop(mapped, unchanged, pending.tail) + else + val buf = if mapped == null then new ListBuffer[Tree] else mapped + var xc = unchanged + while xc ne pending do + buf += xc.head + xc = xc.tail + head1 match + case Thicket(elems1) => buf ++= elems1 + case _ => buf += head1 + val tail0 = pending.tail + loop(buf, tail0, tail0) + loop(null, trees, trees) /** Transform statements while maintaining import contexts and expression contexts * in the same way as Typer does. The code addresses additional concerns: diff --git a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala index 9d241216bdaa..d4dd911241d3 100644 --- a/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala +++ b/compiler/src/dotty/tools/dotc/transform/MegaPhase.scala @@ -456,7 +456,7 @@ class MegaPhase(val miniPhases: Array[MiniPhase]) extends Phase { } def transformTrees(trees: List[Tree], start: Int)(using Context): List[Tree] = - trees.mapInline(transformTree(_, start)) + trees.flattenedMapConserve(transformTree(_, start)) def transformSpecificTrees[T <: Tree](trees: List[T], start: Int)(using Context): List[T] = transformTrees(trees, start).asInstanceOf[List[T]] From bc8df3eb9c3e054ff2df757e68ca5a7f2383fd07 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 25 Dec 2022 14:09:39 +0100 Subject: [PATCH 23/28] Optimize period equality tests --- compiler/src/dotty/tools/dotc/core/Periods.scala | 2 +- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- compiler/src/dotty/tools/dotc/core/Symbols.scala | 2 +- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Periods.scala b/compiler/src/dotty/tools/dotc/core/Periods.scala index aaacd21db05d..ee877fb538d4 100644 --- a/compiler/src/dotty/tools/dotc/core/Periods.scala +++ b/compiler/src/dotty/tools/dotc/core/Periods.scala @@ -20,7 +20,7 @@ object Periods { /** Are all base types in the current period guaranteed to be the same as in period `p`? */ def currentHasSameBaseTypesAs(p: Period)(using Context): Boolean = val period = ctx.period - period == p || + period.code == p.code || period.runId == p.runId && unfusedPhases(period.phaseId).sameBaseTypesStartId == unfusedPhases(p.phaseId).sameBaseTypesStartId diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 64171cdf422b..9d7a3945a1ca 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2867,7 +2867,7 @@ object SymDenotations { } def isValidAt(phase: Phase)(using Context) = - checkedPeriod == ctx.period || + checkedPeriod.code == ctx.period.code || createdAt.runId == ctx.runId && createdAt.phaseId < unfusedPhases.length && sameGroup(unfusedPhases(createdAt.phaseId), phase) && diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index 11513d8698c4..d14be2b0dfb9 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -103,7 +103,7 @@ object Symbols { /** The current denotation of this symbol */ final def denot(using Context): SymDenotation = { util.Stats.record("Symbol.denot") - if (checkedPeriod == ctx.period) lastDenot + if checkedPeriod.code == ctx.period.code then lastDenot else computeDenot(lastDenot) } diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index fed1e167f70f..15b0b00ed0f3 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -2266,7 +2266,7 @@ object Types { final def symbol(using Context): Symbol = // We can rely on checkedPeriod (unlike in the definition of `denot` below) // because SymDenotation#installAfter never changes the symbol - if (checkedPeriod == ctx.period) lastSymbol.asInstanceOf[Symbol] + if (checkedPeriod.code == ctx.period.code) lastSymbol.asInstanceOf[Symbol] else computeSymbol private def computeSymbol(using Context): Symbol = From b64b73c4e39e8443c97b398074c78f3cdf9f18a7 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 18:39:25 +0100 Subject: [PATCH 24/28] Drop expensive escapeToNext The `ensuring` seems to be expensive. Omiting it does not seem to cause a problem since a denotation that's valid nowhere would certainly produce other errors when accessed. --- compiler/src/dotty/tools/dotc/core/Denotations.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 56e9686a11a7..0974f6251cdf 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -795,8 +795,6 @@ object Denotations { assert(!d.is(Package), s"illegal transformation of package denotation by transformer $transformer") case _ => - def escapeToNext = nextDefined.ensuring(_.validFor != Nowhere) - def toNewRun = util.Stats.record("current.bringForward") if exists then initial.bringForward().current else this @@ -871,7 +869,7 @@ object Denotations { // can happen if we sit on a stale denotation which has been replaced // wholesale by an installAfter; in this case, proceed to the next // denotation and try again. - escapeToNext + nextDefined else if valid.runId != currentPeriod.runId then toNewRun else if currentPeriod.code > valid.code then From 88c84ea4dae9fe7199e1c962eda27ab5878fb22c Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 26 Dec 2022 18:26:15 +0100 Subject: [PATCH 25/28] Use home-brewed futures for parallel pickling Uses just one thread for the rest of pickling. One thread is sufficient since there is not that much to do and we have time until the backend finishes. We might want to partially revise that decision when we support pipelined computation. In that case producing tasty early could be a win. But even in that case we might want to fine-tune the number of worker threads instead of relying on some executor. Adding more workers is easy in the new design. --- .../dotty/tools/dotc/transform/Pickler.scala | 23 +++++-- .../dotty/tools/dotc/util/concurrent.scala | 62 +++++++++++++++++++ 2 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/util/concurrent.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 2433071213d5..f5fe34bafc2f 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -14,8 +14,8 @@ import Symbols._ import Flags.Module import reporting.{ThrowingReporter, Profile, Message} import collection.mutable -import scala.concurrent.{Future, Await, ExecutionContext} -import scala.concurrent.duration.Duration +import util.concurrent.{Executor, Future} +import compiletime.uninitialized object Pickler { val name: String = "pickler" @@ -70,6 +70,11 @@ class Pickler extends Phase { body(scratch) } + private val executor = Executor[Array[Byte]]() + + private def useExecutor(using Context) = + Pickler.ParallelPickling && !ctx.settings.YtestPickler.value + override def run(using Context): Unit = { val unit = ctx.compilationUnit pickling.println(i"unpickling in run ${ctx.runId}") @@ -123,10 +128,10 @@ class Pickler extends Phase { * function value. */ val demandPickled: () => Array[Byte] = - if Pickler.ParallelPickling && !ctx.settings.YtestPickler.value then - val futurePickled = Future(computePickled())(using ExecutionContext.global) + if useExecutor then + val futurePickled = executor.schedule(computePickled) () => - try Await.result(futurePickled, Duration.Inf) + try futurePickled.force.get finally reportPositionWarnings() else val pickled = computePickled() @@ -139,7 +144,13 @@ class Pickler extends Phase { } override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] = { - val result = super.runOn(units) + val result = + if useExecutor then + executor.start() + try super.runOn(units) + finally executor.close() + else + super.runOn(units) if ctx.settings.YtestPickler.value then val ctx2 = ctx.fresh.setSetting(ctx.settings.YreadComments, true) testUnpickler( diff --git a/compiler/src/dotty/tools/dotc/util/concurrent.scala b/compiler/src/dotty/tools/dotc/util/concurrent.scala new file mode 100644 index 000000000000..2710aae6c9b0 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/util/concurrent.scala @@ -0,0 +1,62 @@ +package dotty.tools.dotc.util +import scala.util.{Try, Failure, Success} +import scala.collection.mutable.ArrayBuffer + +object concurrent: + + class NoCompletion extends RuntimeException + + class Future[T](exec: Executor[T]): + private var result: Option[Try[T]] = None + def force: Try[T] = synchronized { + while result.isEmpty && exec.isAlive do wait(1000 /*ms*/) + result.getOrElse(Failure(NoCompletion())) + } + def complete(r: Try[T]): Unit = synchronized { + result = Some(r) + notifyAll() + } + end Future + + class Executor[T] extends Thread: + private type WorkItem = (Future[T], () => T) + + private var allScheduled = false + private val pending = new ArrayBuffer[WorkItem] + + def schedule(op: () => T): Future[T] = synchronized { + assert(!allScheduled) + val f = Future[T](this) + pending += ((f, op)) + notifyAll() + f + } + + def close(): Unit = synchronized { + allScheduled = true + notifyAll() + } + + private def nextPending(): Option[WorkItem] = synchronized { + while pending.isEmpty && !allScheduled do wait(1000 /*ms*/) + if pending.isEmpty then None + else + val item = pending.head + pending.dropInPlace(1) + Some(item) + } + + override def run(): Unit = + while + nextPending() match + case Some((f, op)) => + f.complete(Try(op())) + true + case None => + false + do () + end Executor +end concurrent + + + From 78f1be0853e4b98bbfe85cc687b7f53dcdbe5f69 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 18:53:30 +0100 Subject: [PATCH 26/28] Drop redundant catch and re-throw --- compiler/src/dotty/tools/dotc/core/Denotations.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 0974f6251cdf..723f9408d805 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -830,9 +830,6 @@ object Denotations { // creations that way, and also avoid phase caches in contexts to get large. // To work correctly, we need to demand that the context with the new phase // is not retained in the result. - catch case ex: CyclicReference => - // println(s"error while transforming $this") - throw ex finally mutCtx.setPeriod(savedPeriod) if next eq cur then From 586c4598042ed306f934a1ac06e8f7e6b6697cd4 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 5 Jan 2023 19:11:27 +0100 Subject: [PATCH 27/28] Revert some changes in NameBuffer This reverts soke changes from commit d4a5515e8c2bfb3a6b1668a5c0defb4438706221. Use again a LinkedHashMap instead of a combination of EqHashMap and ArrayBuffer --- .../tools/dotc/core/tasty/NameBuffer.scala | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala b/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala index 33d4d0ea258e..1ddcf9afe1dc 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala @@ -6,7 +6,7 @@ package tasty import dotty.tools.tasty.TastyBuffer import TastyBuffer._ -import collection.mutable.ArrayBuffer +import collection.mutable import Names.{Name, chrs, SimpleName, DerivedName, TypeName} import NameKinds._ import NameOps._ @@ -16,36 +16,38 @@ import NameTags.{SIGNED, TARGETSIGNED} class NameBuffer extends TastyBuffer(10000) { import NameBuffer._ - private val nameRefs = new util.EqHashMap[Name, NameRef] - private val nameBuf = new ArrayBuffer[Name] + private val nameRefs = new mutable.LinkedHashMap[Name, NameRef] - def nameIndex(name: Name): NameRef = + def nameIndex(name: Name): NameRef = { val name1 = name.toTermName - val ref: NameRef | Null = nameRefs.lookup(name1) - if ref != null then ref.uncheckedNN - else - name1 match - case SignedName(original, Signature(params, result), target) => - nameIndex(original) - if !original.matchesTargetName(target) then nameIndex(target) - nameIndex(result) - params.foreach { - case param: TypeName => - nameIndex(param) - case _ => - } - case AnyQualifiedName(prefix, name) => - nameIndex(prefix); nameIndex(name) - case AnyUniqueName(original, separator, num) => - nameIndex(separator) - if (!original.isEmpty) nameIndex(original) - case DerivedName(original, _) => - nameIndex(original) - case _ => - val ref1 = NameRef(nameRefs.size) - nameRefs(name1) = ref1 - nameBuf += name1 - ref1 + nameRefs.get(name1) match { + case Some(ref) => + ref + case None => + name1 match { + case SignedName(original, Signature(params, result), target) => + nameIndex(original) + if !original.matchesTargetName(target) then nameIndex(target) + nameIndex(result) + params.foreach { + case param: TypeName => + nameIndex(param) + case _ => + } + case AnyQualifiedName(prefix, name) => + nameIndex(prefix); nameIndex(name) + case AnyUniqueName(original, separator, num) => + nameIndex(separator) + if (!original.isEmpty) nameIndex(original) + case DerivedName(original, _) => + nameIndex(original) + case _ => + } + val ref = NameRef(nameRefs.size) + nameRefs(name1) = ref + ref + } + } private inline def withLength(inline op: Unit, lengthWidth: Int = 1): Unit = { val lengthAddr = currentAddr @@ -112,7 +114,7 @@ class NameBuffer extends TastyBuffer(10000) { override def assemble(): Unit = { var i = 0 - for name <- nameBuf do + for (name, ref) <- nameRefs do val ref = nameRefs(name) assert(ref.index == i) i += 1 From f2caf058cd480ee984a3c7942e7fe1108a050c4b Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 20 Jan 2023 12:05:51 +0100 Subject: [PATCH 28/28] Make lazy vals threadunsafe. --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index aaebae5f30b3..d412f5c36677 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -518,8 +518,8 @@ class Definitions { def staticsMethod(name: PreName): TermSymbol = ScalaStaticsModule.requiredMethod(name) @tu lazy val DottyArraysModule: Symbol = requiredModule("scala.runtime.Arrays") - lazy val newGenericArrayMethod: TermSymbol = DottyArraysModule.requiredMethod("newGenericArray") - lazy val newArrayMethod: TermSymbol = DottyArraysModule.requiredMethod("newArray") + @tu lazy val newGenericArrayMethod: TermSymbol = DottyArraysModule.requiredMethod("newGenericArray") + @tu lazy val newArrayMethod: TermSymbol = DottyArraysModule.requiredMethod("newArray") def getWrapVarargsArrayModule: Symbol = ScalaRuntimeModule