From bf9123d5685bdb26aafab2cfc079cd160bd60344 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 12 May 2017 13:37:47 +0200 Subject: [PATCH 01/22] Use proper cache for memberNames --- .../tools/dotc/core/SymDenotations.scala | 164 ++++++++++++++---- .../src/dotty/tools/dotc/core/Types.scala | 14 +- .../dotty/tools/dotc/typer/RefChecks.scala | 2 +- 3 files changed, 140 insertions(+), 40 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index d3865d1e0ce1..1098ce2ac8e7 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -15,6 +15,7 @@ import annotation.tailrec import CheckRealizable._ import util.SimpleMap import util.Stats +import java.util.WeakHashMap import config.Config import config.Printers.{completions, incremental, noPrinter} @@ -1250,6 +1251,7 @@ object SymDenotations { override protected[dotc] final def info_=(tp: Type) = { super.info_=(tp) + invalidateMemberNamesCache() myTypeParams = null // changing the info might change decls, and with it typeParams } @@ -1322,7 +1324,7 @@ object SymDenotations { myMemberFingerPrint = FingerPrint.unknown myMemberCache = null myMemberCachePeriod = Nowhere - memberNamesCache = SimpleMap.Empty + invalidateMemberNamesCache() } // ------ class-specific operations ----------------------------------- @@ -1534,6 +1536,7 @@ object SymDenotations { myMemberFingerPrint.include(sym.name) if (myMemberCache != null) myMemberCache invalidate sym.name + invalidateMemberNamesCache() } /** Replace symbol `prev` (if defined in current class) by symbol `replacement`. @@ -1556,6 +1559,7 @@ object SymDenotations { info.decls.openForMutations.unlink(sym) myMemberFingerPrint = FingerPrint.unknown if (myMemberCache != null) myMemberCache invalidate sym.name + invalidateMemberNamesCache() } /** Make sure the type parameters of this class appear in the order given @@ -1720,35 +1724,37 @@ object SymDenotations { } } - private[this] var memberNamesCache: SimpleMap[NameFilter, Set[Name]] = SimpleMap.Empty - - def memberNames(keepOnly: NameFilter)(implicit ctx: Context): Set[Name] = { - def computeMemberNames: Set[Name] = { - var names = Set[Name]() - def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name - for (p <- classParents) - for (name <- p.memberNames(keepOnly, thisType)) maybeAdd(name) - val ownSyms = - if (keepOnly == implicitFilter) - if (this is Package) Iterator.empty - else info.decls.iterator filter (_ is Implicit) - else info.decls.iterator - for (sym <- ownSyms) maybeAdd(sym.name) - names - } - if ((this is PackageClass) || !Config.cacheMemberNames) - computeMemberNames // don't cache package member names; they might change + private[this] var memberNamesCache: MemberNames = null + + private def memberNamesCacheValid = memberNamesCache != null && memberNamesCache.isValid + + def memberNames(keepOnly: NameFilter, onBehalf: MemberNames)(implicit ctx: Context): Set[Name] = + if ((this is PackageClass) || !Config.cacheMemberNames) + computeMemberNames(keepOnly, onBehalf) // don't cache package member names; they might change else { - val cached = memberNamesCache(keepOnly) - if (cached != null) cached - else { - val names = computeMemberNames - if (isFullyCompleted) { - setFlag(Frozen) - memberNamesCache = memberNamesCache.updated(keepOnly, names) - } - names - } + if (!memberNamesCacheValid) memberNamesCache = new MemberNames + memberNamesCache(keepOnly, this, onBehalf) + } + + def computeMemberNames(keepOnly: NameFilter, onBehalf: MemberNames)(implicit ctx: Context): Set[Name] = { + var names = Set[Name]() + def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name + for (p <- classParents) + for (name <- p.memberNames(keepOnly, onBehalf, thisType)) + maybeAdd(name) + val ownSyms = + if (keepOnly eq implicitFilter) + if (this is Package) Iterator.empty + else info.decls.iterator filter (_ is Implicit) + else info.decls.iterator + for (sym <- ownSyms) maybeAdd(sym.name) + names + } + + def invalidateMemberNamesCache() = { + if (memberNamesCacheValid) { + memberNamesCache.invalidate() + memberNamesCache = null } } @@ -1849,10 +1855,10 @@ object SymDenotations { } /** The union of the member names of the package and the package object */ - override def memberNames(keepOnly: NameFilter)(implicit ctx: Context): Set[Name] = { - val ownNames = super.memberNames(keepOnly) + override def memberNames(keepOnly: NameFilter, onBehalf: MemberNames)(implicit ctx: Context): Set[Name] = { + val ownNames = super.memberNames(keepOnly, onBehalf) packageObj.moduleClass.denot match { - case pcls: ClassDenotation => ownNames union pcls.memberNames(keepOnly) + case pcls: ClassDenotation => ownNames union pcls.memberNames(keepOnly, onBehalf) case _ => ownNames } } @@ -2017,6 +2023,100 @@ object SymDenotations { } } + abstract class InheritedCache { + def invalidate(): Unit + + private[this] var dependent: WeakHashMap[InheritedCache, Unit] = null + + protected[this] def invalidateDependents() = { + if (dependent != null) { + val it = dependent.keySet.iterator() + while (it.hasNext()) it.next().invalidate() + } + dependent = null + } + + protected[this] def addDependent(dep: InheritedCache) = { + if (dependent == null) dependent = new WeakHashMap + dependent.put(dep, ()) + } + } + + class MemberNames extends InheritedCache { + private[this] var cache: SimpleMap[NameFilter, Set[Name]] = SimpleMap.Empty + + final def isValid: Boolean = cache != null + + private var locked = false + + /** Computing parent member names might force parents, which could invalidate + * the cache itself. In that case we should cancel invalidation and + * proceed as usual. However, all cache entries should be cleared. + */ + def invalidate(): Unit = + if (isValid) + if (locked) cache = SimpleMap.Empty + else { + cache = null + invalidateDependents() + } + + def apply(keepOnly: NameFilter, clsd: ClassDenotation, onBehalf: MemberNames)(implicit ctx: Context) = { + assert(isValid) + val cached = cache(keepOnly) + try + if (cached != null) cached + else { + locked = true + val computed = + try clsd.computeMemberNames(keepOnly, this) + finally locked = false + cache = cache.updated(keepOnly, computed) + computed + } + finally if (onBehalf != null) addDependent(onBehalf) + } + } + + class BaseClasses extends InheritedCache { + private[this] var listCache: List[ClassSymbol] = null + private[this] var setCache: BitSet = null + + private var valid = true + var locked = false + + final def isValid: Boolean = listCache != null + + def invalidate(): Unit = + if (isValid && !locked) { + listCache = null + setCache = null + invalidateDependents() + } + + private def compute(clsd: ClassDenotation)(implicit ctx: Context) = { + assert(valid) + locked = true + val (computedList, computedSet) = + try clsd.computeBases(this) + finally locked = false + listCache = computedList + setCache = computedSet + } + + def asList(clsd: ClassDenotation, onBehalf: BaseClasses)(implicit ctx: Context) = + try { + if (listCache == null) compute(clsd) + listCache + } + finally if (onBehalf != null) addDependent(onBehalf) + + def asSet(clsd: ClassDenotation, onBehalf: BaseClasses)(implicit ctx: Context): BitSet = { + asList(clsd, onBehalf) + setCache + } + } + object FingerPrint { def apply() = new FingerPrint(new Array[Long](NumWords)) val unknown = new FingerPrint(null) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index ac3245c37db3..50394dac3db6 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -646,25 +646,25 @@ object Types { * @note: OK to use a Set[Name] here because Name hashcodes are replayable, * hence the Set will always give the same names in the same order. */ - final def memberNames(keepOnly: NameFilter, pre: Type = this)(implicit ctx: Context): Set[Name] = this match { + final def memberNames(keepOnly: NameFilter, onBehalf: MemberNames, pre: Type = this)(implicit ctx: Context): Set[Name] = this match { case tp: ClassInfo => - tp.cls.memberNames(keepOnly) filter (keepOnly(pre, _)) + tp.cls.memberNames(keepOnly, onBehalf) filter (keepOnly(pre, _)) case tp: RefinedType => - val ns = tp.parent.memberNames(keepOnly, pre) + val ns = tp.parent.memberNames(keepOnly, onBehalf, pre) if (keepOnly(pre, tp.refinedName)) ns + tp.refinedName else ns case tp: TypeProxy => - tp.underlying.memberNames(keepOnly, pre): @tailrec + tp.underlying.memberNames(keepOnly, onBehalf, pre): @tailrec case tp: AndType => - tp.tp1.memberNames(keepOnly, pre) | tp.tp2.memberNames(keepOnly, pre) + tp.tp1.memberNames(keepOnly, onBehalf, pre) | tp.tp2.memberNames(keepOnly, onBehalf, pre) case tp: OrType => - tp.tp1.memberNames(keepOnly, pre) & tp.tp2.memberNames(keepOnly, pre) + tp.tp1.memberNames(keepOnly, onBehalf, pre) & tp.tp2.memberNames(keepOnly, onBehalf, pre) case _ => Set() } def memberDenots(keepOnly: NameFilter, f: (Name, mutable.Buffer[SingleDenotation]) => Unit)(implicit ctx: Context): Seq[SingleDenotation] = { val buf = mutable.ArrayBuffer[SingleDenotation]() - for (name <- memberNames(keepOnly)) f(name, buf) + for (name <- memberNames(keepOnly, onBehalf = null)) f(name, buf) buf } diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index ebe79cdb4f41..2130c55cf105 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -41,7 +41,7 @@ object RefChecks { defaultGetterClass <- List(clazz, clazz.companionModule.moduleClass); if defaultGetterClass.isClass ) { - val defaultGetterNames = defaultGetterClass.asClass.memberNames(defaultMethodFilter) + val defaultGetterNames = defaultGetterClass.asClass.memberNames(defaultMethodFilter, onBehalf = null) val defaultMethodNames = defaultGetterNames map { _ rewrite { case DefaultGetterName(methName, _) => methName }} From 6d525f1d7a8ae26d33179c8cd873a210182dd71a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 12 May 2017 14:56:26 +0200 Subject: [PATCH 02/22] Improve caching of baseClasses and superClassBits --- .../tools/dotc/core/SymDenotations.scala | 187 ++++++++++++++---- 1 file changed, 145 insertions(+), 42 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 1098ce2ac8e7..64bbc279becf 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1174,6 +1174,7 @@ object SymDenotations { val annotations1 = if (annotations != null) annotations else this.annotations val d = ctx.SymDenotation(symbol, owner, name, initFlags1, info1, privateWithin1) d.annotations = annotations1 + // TODO: Copy memberCache if info does not change d } @@ -1250,7 +1251,16 @@ object SymDenotations { } override protected[dotc] final def info_=(tp: Type) = { + val parentChange = infoOrCompleter match { + case info: ClassInfo => + tp match { + case tp: ClassInfo => info.classParents ne tp.classParents + case _ => true + } + case _ => true + } super.info_=(tp) + if (parentChange || true) invalidateBaseDataCache() invalidateMemberNamesCache() myTypeParams = null // changing the info might change decls, and with it typeParams } @@ -1275,7 +1285,7 @@ object SymDenotations { * Packages are never fully completed since members can be added at any time. * @see Namer#ClassCompleter */ - private def isFullyCompleted(implicit ctx: Context): Boolean = { + private[core] def isFullyCompleted(implicit ctx: Context): Boolean = { def isFullyCompletedRef(tp: TypeRef) = tp.denot match { case d: ClassDenotation => d.isFullyCompleted case _ => false @@ -1324,6 +1334,7 @@ object SymDenotations { myMemberFingerPrint = FingerPrint.unknown myMemberCache = null myMemberCachePeriod = Nowhere + invalidateBaseDataCache() invalidateMemberNamesCache() } @@ -1362,6 +1373,42 @@ object SymDenotations { private[this] var myBaseClasses: List[ClassSymbol] = null private[this] var mySuperClassBits: BitSet = null + private[this] var baseClassesCache: BaseData = null + + private def baseClassesCacheValid = baseClassesCache != null && baseClassesCache.isValid + + private def invalidateBaseDataCache() = { + if (baseClassesCacheValid) { + baseClassesCache.invalidate() + baseClassesCache = null + } + } + + private def baseData(onBehalf: BaseData)(implicit ctx: Context): (List[ClassSymbol], BaseClassSet) = { + if (!baseClassesCacheValid) baseClassesCache = new BaseData + baseClassesCache(this, onBehalf) + } + + /** The base classes of this class in linearization order, + * with the class itself as first element. + */ + def baseClasses(onBehalf: BaseData)(implicit ctx: Context): List[ClassSymbol] = + baseData(onBehalf)._1 + + def baseClasses(implicit ctx: Context): List[ClassSymbol] = { + //val was = baseClassesOLD + val now = baseClasses(null: BaseData) + //assert(now == was || + // was.nonEmpty && now == was.init, i"""diff in baseclasses for $this at ${ctx.phase}, + // |was: $was%, % + // |now: $now%, %""") + now + } + + /** A bitset that contains the superId's of all base classes */ + private def baseClassSet(onBehalf: BaseData = null)(implicit ctx: Context): BaseClassSet = + baseData(onBehalf)._2 + /** Invalidate baseTypeRefCache, baseClasses and superClassBits on new run */ private def checkBasesUpToDate()(implicit ctx: Context) = if (baseTypeRefValid != ctx.runId) { @@ -1374,7 +1421,37 @@ object SymDenotations { def invalidateBaseTypeRefCache() = baseTypeRefCache = new java.util.HashMap[CachedType, Type] - private def computeBases(implicit ctx: Context): (List[ClassSymbol], BitSet) = { + def computeBases(onBehalf: BaseData)(implicit ctx: Context): (List[ClassSymbol], BaseClassSet) = { + val seen = mutable.SortedSet[Int]() + def addBaseClasses(bcs: List[ClassSymbol], to: List[ClassSymbol]) + : List[ClassSymbol] = bcs match { + case bc :: bcs1 => + val bcs1added = addBaseClasses(bcs1, to) + if (seen contains bc.id) bcs1added + else { + seen += bc.id + bc :: bcs1added + } + case nil => + to + } + def addParentBaseClasses(ps: List[TypeRef], to: List[ClassSymbol]): List[ClassSymbol] = ps match { + case p :: ps1 => + addParentBaseClasses(ps1, + addBaseClasses(p.symbol.asClass.baseClasses(onBehalf), to)) + case nil => + to + } + if (classParents.isEmpty && + !is(Package) && !symbol.eq(defn.AnyClass)) { + if (onBehalf != null) onBehalf.signalProvisional() + (classSymbol :: Nil, Set()) + } + (classSymbol :: addParentBaseClasses(classParents, Nil), + new BaseClassSet(seen.toArray)) + } + + private def computeBasesOLD(implicit ctx: Context): (List[ClassSymbol], BitSet) = { if (myBaseClasses eq Nil) throw CyclicReference(this) myBaseClasses = Nil val seen = new mutable.BitSet @@ -1392,9 +1469,10 @@ object SymDenotations { case nil => to } - def addParentBaseClasses(ps: List[Type], to: List[ClassSymbol]): List[ClassSymbol] = ps match { + def addParentBaseClasses(ps: List[TypeRef], to: List[ClassSymbol]): List[ClassSymbol] = ps match { case p :: ps1 => - addParentBaseClasses(ps1, addBaseClasses(p.baseClasses, to)) + addParentBaseClasses(ps1, + addBaseClasses(p.symbol.asClass.baseClasses, to)) case nil => to } @@ -1404,33 +1482,43 @@ object SymDenotations { myBaseClasses = bcs mySuperClassBits = scbits } - else myBaseClasses = null + else { + println(i"RETRY $this # ${symbol.id}") + myBaseClasses = null + } (bcs, scbits) } /** A bitset that contains the superId's of all base classes */ - private def superClassBits(implicit ctx: Context): BitSet = + private def superClassBitsOLD(implicit ctx: Context): BitSet = if (classParents.isEmpty) BitSet() // can happen when called too early in Namers else { checkBasesUpToDate() - if (mySuperClassBits != null) mySuperClassBits else computeBases._2 + if (mySuperClassBits != null) mySuperClassBits else computeBasesOLD._2 } /** The base classes of this class in linearization order, * with the class itself as first element. */ - def baseClasses(implicit ctx: Context): List[ClassSymbol] = + def baseClassesOLD(implicit ctx: Context): List[ClassSymbol] = if (classParents.isEmpty) classSymbol :: Nil // can happen when called too early in Namers else { checkBasesUpToDate() - if (myBaseClasses != null) myBaseClasses else computeBases._1 + if (myBaseClasses != null) myBaseClasses else computeBasesOLD._1 } + def properlyDerivesFrom(base: Symbol)(implicit ctx: Context) = { + //val was = superClassBitsOLD contains base.superId + val now = baseClassSet() contains base + //assert(was == now, i"diff for $this derivesFrom $base, was: $was, npw: $now, $superClassBitsOLD: ${base.superId}// ${baseClassSet().classIds.deep}: ${base.id}") + now + } + final override def derivesFrom(base: Symbol)(implicit ctx: Context): Boolean = !isAbsent && base.isClass && ( (symbol eq base) - || (superClassBits contains base.superId) + || properlyDerivesFrom(base) || (this is Erroneous) || (base is Erroneous) ) @@ -1680,7 +1768,7 @@ object SymDenotations { tp else subcls.denot match { case cdenot: ClassDenotation => - if (cdenot.superClassBits contains symbol.superId) foldGlb(NoType, tp.parents) + if (cdenot.properlyDerivesFrom(symbol)) foldGlb(NoType, tp.parents) else NoType case _ => baseTypeRefOf(tp.superType) @@ -1728,6 +1816,13 @@ object SymDenotations { private def memberNamesCacheValid = memberNamesCache != null && memberNamesCache.isValid + private def invalidateMemberNamesCache() = { + if (memberNamesCacheValid) { + memberNamesCache.invalidate() + memberNamesCache = null + } + } + def memberNames(keepOnly: NameFilter, onBehalf: MemberNames)(implicit ctx: Context): Set[Name] = if ((this is PackageClass) || !Config.cacheMemberNames) computeMemberNames(keepOnly, onBehalf) // don't cache package member names; they might change @@ -1751,13 +1846,6 @@ object SymDenotations { names } - def invalidateMemberNamesCache() = { - if (memberNamesCacheValid) { - memberNamesCache.invalidate() - memberNamesCache = null - } - } - private[this] var fullNameCache: SimpleMap[QualifiedNameKind, Name] = SimpleMap.Empty override final def fullNameSeparated(kind: QualifiedNameKind)(implicit ctx: Context): Name = { val cached = fullNameCache(kind) @@ -2078,42 +2166,42 @@ object SymDenotations { } } - class BaseClasses extends InheritedCache { - private[this] var listCache: List[ClassSymbol] = null - private[this] var setCache: BitSet = null + class BaseData extends InheritedCache { + private[this] var cache: (List[ClassSymbol], BaseClassSet) = null - private var valid = true - var locked = false + private[this] var valid = true + private[this] var locked = false + private[this] var provisional = false - final def isValid: Boolean = listCache != null + final def isValid: Boolean = valid def invalidate(): Unit = if (isValid && !locked) { - listCache = null - setCache = null + cache = null + valid = false invalidateDependents() } - private def compute(clsd: ClassDenotation)(implicit ctx: Context) = { - assert(valid) - locked = true - val (computedList, computedSet) = - try clsd.computeBases(this) - finally locked = false - listCache = computedList - setCache = computedSet - } + def signalProvisional() = provisional = true - def asList(clsd: ClassDenotation, onBehalf: BaseClasses)(implicit ctx: Context) = + def apply(clsd: ClassDenotation, onBehalf: BaseData)(implicit ctx: Context) + : (List[ClassSymbol], BaseClassSet) = { + assert(isValid) try { - if (listCache == null) compute(clsd) - listCache + if (cache != null) cache + else { + if (locked) throw CyclicReference(clsd) + locked = true + provisional = false + val computed = + try clsd.computeBases(this) + finally locked = false + if (!provisional) cache = computed + else if (onBehalf != null) onBehalf.signalProvisional() + computed + } } finally if (onBehalf != null) addDependent(onBehalf) - - def asSet(clsd: ClassDenotation, onBehalf: BaseClasses)(implicit ctx: Context): BitSet = { - asList(clsd, onBehalf) - setCache } } @@ -2126,6 +2214,21 @@ object SymDenotations { private final val Mask = NumBits - 1 } + class BaseClassSet(val classIds: Array[Int]) extends AnyVal { + def contains(sym: Symbol): Boolean = { + val id = sym.id + var lo = 0 + var hi = classIds.length - 1 + while (lo <= hi) { + val mid = (lo + hi) / 2 + if (id < classIds(mid)) hi = mid - 1 + else if (id > classIds(mid)) lo = mid + 1 + else return true + } + false + } + } + private val AccessorOrLabel = Accessor | Label @sharable private var indent = 0 // for completions printing From bf84d5a56dac0bd74c108f6d250ba729c95b2420 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 11:49:44 +0200 Subject: [PATCH 03/22] Make onBehalf implicit parameters Also introduce dummy onBehalfs as sentinels to avoid the need for `null` or `Option`. --- .../src/dotty/tools/dotc/core/Contexts.scala | 4 + .../src/dotty/tools/dotc/core/Flags.scala | 3 + .../tools/dotc/core/SymDenotations.scala | 79 ++++++++----------- .../src/dotty/tools/dotc/core/Types.scala | 14 ++-- .../dotty/tools/dotc/typer/RefChecks.scala | 2 +- 5 files changed, 49 insertions(+), 53 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index b299de4340c3..10ee4599a1c9 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -575,6 +575,10 @@ object Contexts { /** The standard definitions */ val definitions = new Definitions + /** Sinks for "onBehalf" chains */ + val dummyMemberNames = new MemberNames + val dummyBaseData = new BaseData + /** Initializes the `ContextBase` with a starting context. * This initializes the `platform` and the `definitions`. */ diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index c1929d882b21..5d4ab9922910 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -558,6 +558,9 @@ object Flags { /** A lazy or deferred value */ final val LazyOrDeferred = Lazy | Deferred + /** An accessor or label */ + final val AccessorOrLabel = Accessor | Label + /** A synthetic or private definition */ final val SyntheticOrPrivate = Synthetic | Private diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 64bbc279becf..4348ce1194d4 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1384,30 +1384,20 @@ object SymDenotations { } } - private def baseData(onBehalf: BaseData)(implicit ctx: Context): (List[ClassSymbol], BaseClassSet) = { + private def baseData(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { if (!baseClassesCacheValid) baseClassesCache = new BaseData - baseClassesCache(this, onBehalf) + baseClassesCache(this) } /** The base classes of this class in linearization order, * with the class itself as first element. */ - def baseClasses(onBehalf: BaseData)(implicit ctx: Context): List[ClassSymbol] = - baseData(onBehalf)._1 - - def baseClasses(implicit ctx: Context): List[ClassSymbol] = { - //val was = baseClassesOLD - val now = baseClasses(null: BaseData) - //assert(now == was || - // was.nonEmpty && now == was.init, i"""diff in baseclasses for $this at ${ctx.phase}, - // |was: $was%, % - // |now: $now%, %""") - now - } + def baseClasses(implicit onBehalf: BaseData, ctx: Context): List[ClassSymbol] = + baseData._1 /** A bitset that contains the superId's of all base classes */ - private def baseClassSet(onBehalf: BaseData = null)(implicit ctx: Context): BaseClassSet = - baseData(onBehalf)._2 + private def baseClassSet(implicit onBehalf: BaseData, ctx: Context): BaseClassSet = + baseData._2 /** Invalidate baseTypeRefCache, baseClasses and superClassBits on new run */ private def checkBasesUpToDate()(implicit ctx: Context) = @@ -1421,7 +1411,7 @@ object SymDenotations { def invalidateBaseTypeRefCache() = baseTypeRefCache = new java.util.HashMap[CachedType, Type] - def computeBases(onBehalf: BaseData)(implicit ctx: Context): (List[ClassSymbol], BaseClassSet) = { + def computeBases(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { val seen = mutable.SortedSet[Int]() def addBaseClasses(bcs: List[ClassSymbol], to: List[ClassSymbol]) : List[ClassSymbol] = bcs match { @@ -1438,13 +1428,13 @@ object SymDenotations { def addParentBaseClasses(ps: List[TypeRef], to: List[ClassSymbol]): List[ClassSymbol] = ps match { case p :: ps1 => addParentBaseClasses(ps1, - addBaseClasses(p.symbol.asClass.baseClasses(onBehalf), to)) + addBaseClasses(p.symbol.asClass.baseClasses, to)) case nil => to } if (classParents.isEmpty && !is(Package) && !symbol.eq(defn.AnyClass)) { - if (onBehalf != null) onBehalf.signalProvisional() + onBehalf.signalProvisional() (classSymbol :: Nil, Set()) } (classSymbol :: addParentBaseClasses(classParents, Nil), @@ -1507,18 +1497,11 @@ object SymDenotations { if (myBaseClasses != null) myBaseClasses else computeBasesOLD._1 } - def properlyDerivesFrom(base: Symbol)(implicit ctx: Context) = { - //val was = superClassBitsOLD contains base.superId - val now = baseClassSet() contains base - //assert(was == now, i"diff for $this derivesFrom $base, was: $was, npw: $now, $superClassBitsOLD: ${base.superId}// ${baseClassSet().classIds.deep}: ${base.id}") - now - } - final override def derivesFrom(base: Symbol)(implicit ctx: Context): Boolean = !isAbsent && base.isClass && ( (symbol eq base) - || properlyDerivesFrom(base) + || (baseClassSet contains base) || (this is Erroneous) || (base is Erroneous) ) @@ -1768,7 +1751,7 @@ object SymDenotations { tp else subcls.denot match { case cdenot: ClassDenotation => - if (cdenot.properlyDerivesFrom(symbol)) foldGlb(NoType, tp.parents) + if (cdenot.baseClassSet contains symbol) foldGlb(NoType, tp.parents) else NoType case _ => baseTypeRefOf(tp.superType) @@ -1823,19 +1806,19 @@ object SymDenotations { } } - def memberNames(keepOnly: NameFilter, onBehalf: MemberNames)(implicit ctx: Context): Set[Name] = + def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = if ((this is PackageClass) || !Config.cacheMemberNames) - computeMemberNames(keepOnly, onBehalf) // don't cache package member names; they might change + computeMemberNames(keepOnly) // don't cache package member names; they might change else { if (!memberNamesCacheValid) memberNamesCache = new MemberNames - memberNamesCache(keepOnly, this, onBehalf) + memberNamesCache(keepOnly, this) } - def computeMemberNames(keepOnly: NameFilter, onBehalf: MemberNames)(implicit ctx: Context): Set[Name] = { + def computeMemberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = { var names = Set[Name]() def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name for (p <- classParents) - for (name <- p.memberNames(keepOnly, onBehalf, thisType)) + for (name <- p.symbol.asClass.memberNames(keepOnly)) maybeAdd(name) val ownSyms = if (keepOnly eq implicitFilter) @@ -1943,10 +1926,10 @@ object SymDenotations { } /** The union of the member names of the package and the package object */ - override def memberNames(keepOnly: NameFilter, onBehalf: MemberNames)(implicit ctx: Context): Set[Name] = { - val ownNames = super.memberNames(keepOnly, onBehalf) + override def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = { + val ownNames = super.memberNames(keepOnly) packageObj.moduleClass.denot match { - case pcls: ClassDenotation => ownNames union pcls.memberNames(keepOnly, onBehalf) + case pcls: ClassDenotation => ownNames union pcls.memberNames(keepOnly) case _ => ownNames } } @@ -2149,7 +2132,7 @@ object SymDenotations { invalidateDependents() } - def apply(keepOnly: NameFilter, clsd: ClassDenotation, onBehalf: MemberNames)(implicit ctx: Context) = { + def apply(keepOnly: NameFilter, clsd: ClassDenotation)(implicit onBehalf: MemberNames, ctx: Context) = { assert(isValid) val cached = cache(keepOnly) try @@ -2157,15 +2140,19 @@ object SymDenotations { else { locked = true val computed = - try clsd.computeMemberNames(keepOnly, this) + try clsd.computeMemberNames(keepOnly)(this, ctx) finally locked = false cache = cache.updated(keepOnly, computed) computed } - finally if (onBehalf != null) addDependent(onBehalf) + finally addDependent(onBehalf) } } + object MemberNames { + implicit def onBehalfOfNone(implicit ctx: Context): MemberNames = ctx.dummyMemberNames + } + class BaseData extends InheritedCache { private[this] var cache: (List[ClassSymbol], BaseClassSet) = null @@ -2184,7 +2171,7 @@ object SymDenotations { def signalProvisional() = provisional = true - def apply(clsd: ClassDenotation, onBehalf: BaseData)(implicit ctx: Context) + def apply(clsd: ClassDenotation)(implicit onBehalf: BaseData, ctx: Context) : (List[ClassSymbol], BaseClassSet) = { assert(isValid) try { @@ -2194,17 +2181,21 @@ object SymDenotations { locked = true provisional = false val computed = - try clsd.computeBases(this) + try clsd.computeBases(this, ctx) finally locked = false if (!provisional) cache = computed - else if (onBehalf != null) onBehalf.signalProvisional() + else onBehalf.signalProvisional() computed } } - finally if (onBehalf != null) addDependent(onBehalf) + finally addDependent(onBehalf) } } + object BaseData { + implicit def onBehalfOfNone(implicit ctx: Context): BaseData = ctx.dummyBaseData + } + object FingerPrint { def apply() = new FingerPrint(new Array[Long](NumWords)) val unknown = new FingerPrint(null) @@ -2229,7 +2220,5 @@ object SymDenotations { } } - private val AccessorOrLabel = Accessor | Label - @sharable private var indent = 0 // for completions printing } diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 50394dac3db6..ac3245c37db3 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -646,25 +646,25 @@ object Types { * @note: OK to use a Set[Name] here because Name hashcodes are replayable, * hence the Set will always give the same names in the same order. */ - final def memberNames(keepOnly: NameFilter, onBehalf: MemberNames, pre: Type = this)(implicit ctx: Context): Set[Name] = this match { + final def memberNames(keepOnly: NameFilter, pre: Type = this)(implicit ctx: Context): Set[Name] = this match { case tp: ClassInfo => - tp.cls.memberNames(keepOnly, onBehalf) filter (keepOnly(pre, _)) + tp.cls.memberNames(keepOnly) filter (keepOnly(pre, _)) case tp: RefinedType => - val ns = tp.parent.memberNames(keepOnly, onBehalf, pre) + val ns = tp.parent.memberNames(keepOnly, pre) if (keepOnly(pre, tp.refinedName)) ns + tp.refinedName else ns case tp: TypeProxy => - tp.underlying.memberNames(keepOnly, onBehalf, pre): @tailrec + tp.underlying.memberNames(keepOnly, pre): @tailrec case tp: AndType => - tp.tp1.memberNames(keepOnly, onBehalf, pre) | tp.tp2.memberNames(keepOnly, onBehalf, pre) + tp.tp1.memberNames(keepOnly, pre) | tp.tp2.memberNames(keepOnly, pre) case tp: OrType => - tp.tp1.memberNames(keepOnly, onBehalf, pre) & tp.tp2.memberNames(keepOnly, onBehalf, pre) + tp.tp1.memberNames(keepOnly, pre) & tp.tp2.memberNames(keepOnly, pre) case _ => Set() } def memberDenots(keepOnly: NameFilter, f: (Name, mutable.Buffer[SingleDenotation]) => Unit)(implicit ctx: Context): Seq[SingleDenotation] = { val buf = mutable.ArrayBuffer[SingleDenotation]() - for (name <- memberNames(keepOnly, onBehalf = null)) f(name, buf) + for (name <- memberNames(keepOnly)) f(name, buf) buf } diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 2130c55cf105..ebe79cdb4f41 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -41,7 +41,7 @@ object RefChecks { defaultGetterClass <- List(clazz, clazz.companionModule.moduleClass); if defaultGetterClass.isClass ) { - val defaultGetterNames = defaultGetterClass.asClass.memberNames(defaultMethodFilter, onBehalf = null) + val defaultGetterNames = defaultGetterClass.asClass.memberNames(defaultMethodFilter) val defaultMethodNames = defaultGetterNames map { _ rewrite { case DefaultGetterName(methName, _) => methName }} From e9b98cf3f15f53740cda7850c4cb479d8307e279 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 11:57:12 +0200 Subject: [PATCH 04/22] Get rid of some old code that's no longer used --- .../tools/dotc/core/SymDenotations.scala | 72 +------------------ 1 file changed, 3 insertions(+), 69 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 4348ce1194d4..29c401eb7331 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1329,8 +1329,6 @@ object SymDenotations { /** Invalidate all caches and fields that depend on base classes and their contents */ override def invalidateInheritedInfo(): Unit = { - myBaseClasses = null - mySuperClassBits = null myMemberFingerPrint = FingerPrint.unknown myMemberCache = null myMemberCachePeriod = Nowhere @@ -1370,9 +1368,6 @@ object SymDenotations { myTypeRef } - private[this] var myBaseClasses: List[ClassSymbol] = null - private[this] var mySuperClassBits: BitSet = null - private[this] var baseClassesCache: BaseData = null private def baseClassesCacheValid = baseClassesCache != null && baseClassesCache.isValid @@ -1403,15 +1398,13 @@ object SymDenotations { private def checkBasesUpToDate()(implicit ctx: Context) = if (baseTypeRefValid != ctx.runId) { invalidateBaseTypeRefCache() - myBaseClasses = null - mySuperClassBits = null baseTypeRefValid = ctx.runId } def invalidateBaseTypeRefCache() = baseTypeRefCache = new java.util.HashMap[CachedType, Type] - def computeBases(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { + def computeBaseData(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { val seen = mutable.SortedSet[Int]() def addBaseClasses(bcs: List[ClassSymbol], to: List[ClassSymbol]) : List[ClassSymbol] = bcs match { @@ -1432,71 +1425,12 @@ object SymDenotations { case nil => to } - if (classParents.isEmpty && - !is(Package) && !symbol.eq(defn.AnyClass)) { + if (classParents.isEmpty && !is(Package) && !symbol.eq(defn.AnyClass)) onBehalf.signalProvisional() - (classSymbol :: Nil, Set()) - } (classSymbol :: addParentBaseClasses(classParents, Nil), new BaseClassSet(seen.toArray)) } - private def computeBasesOLD(implicit ctx: Context): (List[ClassSymbol], BitSet) = { - if (myBaseClasses eq Nil) throw CyclicReference(this) - myBaseClasses = Nil - val seen = new mutable.BitSet - val locked = new mutable.BitSet - def addBaseClasses(bcs: List[ClassSymbol], to: List[ClassSymbol]) - : List[ClassSymbol] = bcs match { - case bc :: bcs1 => - val bcs1added = addBaseClasses(bcs1, to) - val id = bc.superId - if (seen contains id) bcs1added - else { - seen += id - bc :: bcs1added - } - case nil => - to - } - def addParentBaseClasses(ps: List[TypeRef], to: List[ClassSymbol]): List[ClassSymbol] = ps match { - case p :: ps1 => - addParentBaseClasses(ps1, - addBaseClasses(p.symbol.asClass.baseClasses, to)) - case nil => - to - } - val bcs = classSymbol :: addParentBaseClasses(classParents, Nil) - val scbits = seen - if (isFullyCompleted) { - myBaseClasses = bcs - mySuperClassBits = scbits - } - else { - println(i"RETRY $this # ${symbol.id}") - myBaseClasses = null - } - (bcs, scbits) - } - - /** A bitset that contains the superId's of all base classes */ - private def superClassBitsOLD(implicit ctx: Context): BitSet = - if (classParents.isEmpty) BitSet() // can happen when called too early in Namers - else { - checkBasesUpToDate() - if (mySuperClassBits != null) mySuperClassBits else computeBasesOLD._2 - } - - /** The base classes of this class in linearization order, - * with the class itself as first element. - */ - def baseClassesOLD(implicit ctx: Context): List[ClassSymbol] = - if (classParents.isEmpty) classSymbol :: Nil // can happen when called too early in Namers - else { - checkBasesUpToDate() - if (myBaseClasses != null) myBaseClasses else computeBasesOLD._1 - } - final override def derivesFrom(base: Symbol)(implicit ctx: Context): Boolean = !isAbsent && base.isClass && @@ -2181,7 +2115,7 @@ object SymDenotations { locked = true provisional = false val computed = - try clsd.computeBases(this, ctx) + try clsd.computeBaseData(this, ctx) finally locked = false if (!provisional) cache = computed else onBehalf.signalProvisional() From ff7bdd5056cebba83f35f9ff247826126edecf13 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 12:42:47 +0200 Subject: [PATCH 05/22] Void basedata computation inefficiency After erasure all basedata computations were marked as provisional because Object has empty parents then. This caused slowdowns of up to 100% (~100% was observed for stdlib). --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 29c401eb7331..e10dd853328a 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1425,7 +1425,9 @@ object SymDenotations { case nil => to } - if (classParents.isEmpty && !is(Package) && !symbol.eq(defn.AnyClass)) + def emptyParentsExpected = + is(Package) || (symbol == defn.AnyClass) || ctx.erasedTypes && (symbol == defn.ObjectClass) + if (classParents.isEmpty && !emptyParentsExpected) onBehalf.signalProvisional() (classSymbol :: addParentBaseClasses(classParents, Nil), new BaseClassSet(seen.toArray)) From c1254ed36287d461450d4345b54a2e3f1a853523 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 12:43:24 +0200 Subject: [PATCH 06/22] Instrumentation to get basedata computation counts --- compiler/src/dotty/tools/dotc/Run.scala | 2 ++ compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index acd885dd3afe..a3de94454210 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -92,6 +92,8 @@ class Run(comp: Compiler)(implicit ctx: Context) { for (unit <- units) Stats.record("retained typed trees at end", unit.tpdTree.treeSize) Stats.record("total trees at end", ast.Trees.ntrees) + println(s"bts counts: ${SymDenotations.btsCount.deep}") + println(s"invalidate count: ${SymDenotations.invalidateCount}") } private sealed trait PrintedTree diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index e10dd853328a..69b16d0bf4a1 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1427,6 +1427,7 @@ object SymDenotations { } def emptyParentsExpected = is(Package) || (symbol == defn.AnyClass) || ctx.erasedTypes && (symbol == defn.ObjectClass) + btsCount(ctx.phaseId) += 1 if (classParents.isEmpty && !emptyParentsExpected) onBehalf.signalProvisional() (classSymbol :: addParentBaseClasses(classParents, Nil), @@ -2100,6 +2101,7 @@ object SymDenotations { def invalidate(): Unit = if (isValid && !locked) { + invalidateCount += 1 cache = null valid = false invalidateDependents() @@ -2157,4 +2159,7 @@ object SymDenotations { } @sharable private var indent = 0 // for completions printing + + @sharable val btsCount = new Array[Int](Periods.MaxPossiblePhaseId + 1) + @sharable var invalidateCount = 0 } From e72c0529c4054c244db9e053e152e67349f17b16 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 14:47:11 +0200 Subject: [PATCH 07/22] Have phases declare whether they can change members or parents --- .../src/dotty/tools/dotc/core/Phases.scala | 12 +++++++++ .../tools/dotc/core/SymDenotations.scala | 27 ++++++++++++------- .../src/dotty/tools/dotc/core/Symbols.scala | 2 ++ .../dotc/transform/AugmentScala2Traits.scala | 2 ++ .../tools/dotc/transform/ByNameClosures.scala | 2 +- .../tools/dotc/transform/CapturedVars.scala | 3 +++ .../tools/dotc/transform/ElimRepeated.scala | 3 ++- .../dotty/tools/dotc/transform/Erasure.scala | 3 +++ .../tools/dotc/transform/ExpandPrivate.scala | 2 ++ .../tools/dotc/transform/ExplicitOuter.scala | 2 ++ .../dotc/transform/ExtensionMethods.scala | 2 ++ .../tools/dotc/transform/FirstTransform.scala | 2 ++ .../dotty/tools/dotc/transform/Flatten.scala | 2 ++ .../dotty/tools/dotc/transform/LazyVals.scala | 17 ++++++------ .../dotty/tools/dotc/transform/Mixin.scala | 2 ++ .../tools/dotc/transform/PatternMatcher.scala | 6 ++--- .../tools/dotc/transform/PostTyper.scala | 2 ++ .../dotc/transform/PrimitiveForwarders.scala | 2 ++ .../tools/dotc/transform/ResolveSuper.scala | 2 ++ .../tools/dotc/transform/RestoreScopes.scala | 2 ++ .../dotc/transform/ShortcutImplicits.scala | 3 +++ 21 files changed, 76 insertions(+), 24 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Phases.scala b/compiler/src/dotty/tools/dotc/core/Phases.scala index 44608296a2b1..841fd9013227 100644 --- a/compiler/src/dotty/tools/dotc/core/Phases.scala +++ b/compiler/src/dotty/tools/dotc/core/Phases.scala @@ -305,6 +305,12 @@ object Phases { */ def isTyper = false + /** Can this transform create or delete non-private members? */ + def changesMembers: Boolean = false + + /** Can this transform change the parents of a class? */ + def changesParents: Boolean = false + def exists: Boolean = true private var myPeriod: Period = Periods.InvalidPeriod @@ -315,6 +321,8 @@ object Phases { private var mySymbolicRefs = false private var myLabelsReordered = false + private var myMembersGroup = 0 + private var myParentsGroup = 0 /** The sequence position of this phase in the given context where 0 * is reserved for NoPhase and the first real phase is at position 1. @@ -331,6 +339,8 @@ object Phases { final def refChecked = myRefChecked // Phase is after RefChecks final def symbolicRefs = mySymbolicRefs // Phase is after ResolveSuper, newly generated TermRefs should be symbolic final def labelsReordered = myLabelsReordered // Phase is after LabelDefs, labels are flattened and owner chains don't mirror this + final def membersGroup = myMembersGroup // group id for phases where all symbols have the same non-private members + final def parentsGroup = myParentsGroup // group id for phases where all symbols have the same base classes protected[Phases] def init(base: ContextBase, start: Int, end:Int): Unit = { if (start >= FirstPhaseId) @@ -342,6 +352,8 @@ object Phases { myRefChecked = prev.getClass == classOf[RefChecks] || prev.refChecked mySymbolicRefs = prev.getClass == classOf[ResolveSuper] || prev.symbolicRefs myLabelsReordered = prev.getClass == classOf[LabelDefs] || prev.labelsReordered + myMembersGroup = if (changesMembers) prev.membersGroup + 1 else prev.membersGroup + myParentsGroup = if (changesParents) prev.parentsGroup + 1 else prev.parentsGroup } protected[Phases] def init(base: ContextBase, id: Int): Unit = init(base, id, id) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 69b16d0bf4a1..cdd38f9663ef 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1170,6 +1170,8 @@ object SymDenotations { { // simulate default parameters, while also passing implicit context ctx to the default values val initFlags1 = (if (initFlags != UndefinedFlags) initFlags else this.flags) &~ Frozen val info1 = if (info != null) info else this.info + if (ctx.isAfterTyper && changedClassParents(info, info1)) + assert(ctx.phase.changesParents, i"undeclared parent change at ${ctx.phase} for $this, was: $info, now: $info1") val privateWithin1 = if (privateWithin != null) privateWithin else this.privateWithin val annotations1 = if (annotations != null) annotations else this.annotations val d = ctx.SymDenotation(symbol, owner, name, initFlags1, info1, privateWithin1) @@ -1178,6 +1180,17 @@ object SymDenotations { d } + /** Are `info1` and `info2` ClassInfo types with different parents? */ + protected def changedClassParents(info1: Type, info2: Type): Boolean = + info2 match { + case info2: ClassInfo => + info1 match { + case info1: ClassInfo => info1.classParents ne info2.classParents + case _ => false + } + case _ => false + } + override def initial: SymDenotation = super.initial.asSymDenotation /** Install this denotation as the result of the given denotation transformer. */ @@ -1251,16 +1264,10 @@ object SymDenotations { } override protected[dotc] final def info_=(tp: Type) = { - val parentChange = infoOrCompleter match { - case info: ClassInfo => - tp match { - case tp: ClassInfo => info.classParents ne tp.classParents - case _ => true - } - case _ => true - } super.info_=(tp) - if (parentChange || true) invalidateBaseDataCache() + if (changedClassParents(infoOrCompleter, tp) || true) { + invalidateBaseDataCache() + } invalidateMemberNamesCache() myTypeParams = null // changing the info might change decls, and with it typeParams } @@ -1544,7 +1551,7 @@ object SymDenotations { myMemberFingerPrint.include(sym.name) if (myMemberCache != null) myMemberCache invalidate sym.name - invalidateMemberNamesCache() + if (!sym.flagsUNSAFE.is(Private)) invalidateMemberNamesCache() } /** Replace symbol `prev` (if defined in current class) by symbol `replacement`. diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index e0d9aca2bbc0..b4691525109a 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -467,6 +467,8 @@ object Symbols { if (this is Module) this.moduleClass.validFor |= InitialPeriod } else this.owner.asClass.ensureFreshScopeAfter(phase) + if (!this.flagsUNSAFE.is(Private)) + assert(phase.changesMembers, i"$this entered in ${this.owner} after undeclared phase $phase") entered } diff --git a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala index f2ffaff5da61..1cca3fea5af5 100644 --- a/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala +++ b/compiler/src/dotty/tools/dotc/transform/AugmentScala2Traits.scala @@ -31,6 +31,8 @@ import ast.Trees._ class AugmentScala2Traits extends MiniPhaseTransform with IdentityDenotTransformer with FullParameterization { thisTransform => import ast.tpd._ + override def changesMembers = true + override def phaseName: String = "augmentScala2Traits" override def rewiredTarget(referenced: Symbol, derived: Symbol)(implicit ctx: Context) = NoSymbol diff --git a/compiler/src/dotty/tools/dotc/transform/ByNameClosures.scala b/compiler/src/dotty/tools/dotc/transform/ByNameClosures.scala index 4f8c7cfceac8..d8499369d051 100644 --- a/compiler/src/dotty/tools/dotc/transform/ByNameClosures.scala +++ b/compiler/src/dotty/tools/dotc/transform/ByNameClosures.scala @@ -14,7 +14,7 @@ import core.StdNames.nme /** This phase translates arguments to call-by-name parameters, using the rules * - * x ==> x if x is a => parameter + * x ==> x if x is a => parameter * e.apply() ==> (e) if e is pure * e ==> (() => e) for all other arguments * diff --git a/compiler/src/dotty/tools/dotc/transform/CapturedVars.scala b/compiler/src/dotty/tools/dotc/transform/CapturedVars.scala index b9a9544f513f..61e3722d7ec5 100644 --- a/compiler/src/dotty/tools/dotc/transform/CapturedVars.scala +++ b/compiler/src/dotty/tools/dotc/transform/CapturedVars.scala @@ -18,6 +18,9 @@ import SymUtils._ import collection.{ mutable, immutable } import collection.mutable.{ LinkedHashMap, LinkedHashSet, TreeSet } +/** This phase translates variables that are captured in closures to + * heap-allocated refs. + */ class CapturedVars extends MiniPhase with IdentityDenotTransformer { thisTransform => import ast.tpd._ diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index 9a768e464483..1a05dc9f48b8 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -29,10 +29,11 @@ class ElimRepeated extends MiniPhaseTransform with InfoTransformer with Annotati override def phaseName = "elimRepeated" + override def changesMembers = true // the phase adds vararg bridges + def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context): Type = elimRepeated(tp) - override def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation = super.transform(ref) match { case ref1: SymDenotation if (ref1 ne ref) && overridesJava(ref1.symbol) => diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index cc41ce00329e..b870d37c47b4 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala @@ -37,6 +37,9 @@ class Erasure extends Phase with DenotTransformer { thisTransformer => /** List of names of phases that should precede this phase */ override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[InterceptedMethods], classOf[Splitter], classOf[ElimRepeated]) + override def changesMembers: Boolean = true // the phase adds bridges + override def changesParents: Boolean = true // the phase drops Any + def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation = ref match { case ref: SymDenotation => def isCompacted(sym: Symbol) = diff --git a/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala b/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala index 8f99ef1d0805..3a3188ffff58 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala @@ -44,6 +44,8 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t // This phase moves methods around (in infotransform) so it may need to make other methods public override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[MoveStatics]) + override def changesMembers = true + override def checkPostCondition(tree: Tree)(implicit ctx: Context): Unit = { tree match { case t: DefDef => diff --git a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala index 60086031edff..937490ab060a 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala @@ -48,6 +48,8 @@ class ExplicitOuter extends MiniPhaseTransform with InfoTransformer { thisTransf */ override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[PatternMatcher], classOf[HoistSuperArgs]) + override def changesMembers = true // the phase adds outer accessors + /** Add outer accessors if a class always needs an outer pointer */ override def transformInfo(tp: Type, sym: Symbol)(implicit ctx: Context) = tp match { case tp @ ClassInfo(_, cls, _, decls, _) if needsOuterAlways(cls) && !sym.is(JavaDefined) => diff --git a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala index 61f32edaefff..84b214a23ab3 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala @@ -49,6 +49,8 @@ class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with Ful override def runsAfterGroupsOf = Set(classOf[FirstTransform]) // need companion objects to exist + override def changesMembers = true // the pahse adds extension methods + override def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation = ref match { case moduleClassSym: ClassDenotation if moduleClassSym is ModuleClass => moduleClassSym.linkedClass match { diff --git a/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala b/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala index a3cf71ef2203..dc85e6db736e 100644 --- a/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -42,6 +42,8 @@ class FirstTransform extends MiniPhaseTransform with InfoTransformer with Annota private var addCompanionPhases: List[NeedsCompanions] = _ + override def changesMembers = true // the phase adds companion objects + def needsCompanion(cls: ClassSymbol)(implicit ctx: Context) = addCompanionPhases.exists(_.isCompanionNeeded(cls)) diff --git a/compiler/src/dotty/tools/dotc/transform/Flatten.scala b/compiler/src/dotty/tools/dotc/transform/Flatten.scala index f0104e715602..da287bfcac7f 100644 --- a/compiler/src/dotty/tools/dotc/transform/Flatten.scala +++ b/compiler/src/dotty/tools/dotc/transform/Flatten.scala @@ -16,6 +16,8 @@ class Flatten extends MiniPhaseTransform with SymTransformer { thisTransform => import ast.tpd._ override def phaseName = "flatten" + override def changesMembers = true // the phase removes inner classes + def transformSym(ref: SymDenotation)(implicit ctx: Context) = { if (ref.isClass && !ref.is(Package) && !ref.owner.is(Package)) { ref.copySymDenotation( diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index f64006d73c51..9afb3051478c 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -29,16 +29,8 @@ import Erasure.Boxing.adaptToType class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { import LazyVals._ - import tpd._ - def transformer = new LazyVals - - val containerFlags = Flags.Synthetic | Flags.Mutable | Flags.Lazy - val initFlags = Flags.Synthetic | Flags.Method - - val containerFlagsMask = Flags.Method | Flags.Lazy | Flags.Accessor | Flags.Module - /** this map contains mutable state of transformation: OffsetDefs to be appended to companion object definitions, * and number of bits currently used */ class OffsetInfo(var defs: List[Tree], var ord:Int) @@ -50,6 +42,15 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer { * before this phase starts processing same tree */ override def runsAfter = Set(classOf[Mixin]) + override def changesMembers = true // the phase adds lazy val accessors + + def transformer = new LazyVals + + val containerFlags = Flags.Synthetic | Flags.Mutable | Flags.Lazy + val initFlags = Flags.Synthetic | Flags.Method + + val containerFlagsMask = Flags.Method | Flags.Lazy | Flags.Accessor | Flags.Module + override def transformDefDef(tree: tpd.DefDef)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = transformLazyVal(tree) diff --git a/compiler/src/dotty/tools/dotc/transform/Mixin.scala b/compiler/src/dotty/tools/dotc/transform/Mixin.scala index f90778df009a..48037e89a29c 100644 --- a/compiler/src/dotty/tools/dotc/transform/Mixin.scala +++ b/compiler/src/dotty/tools/dotc/transform/Mixin.scala @@ -98,6 +98,8 @@ class Mixin extends MiniPhaseTransform with SymTransformer { thisTransform => override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Erasure]) + override def changesMembers = true // the phase adds implementions of mixin accessors + override def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation = if (sym.is(Accessor, butNot = Deferred) && sym.owner.is(Trait)) { val sym1 = diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index 4472c9f5e170..4e12023a565f 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -31,10 +31,8 @@ import dotty.tools.dotc.util.Positions.Position import dotty.tools.dotc.core.Decorators._ import dotty.tools.dotc.core.Flags -/** This transform eliminates patterns. Right now it's a dummy. - * Awaiting the real pattern matcher. - * elimRepeated is required - * TODO: outer tests are not generated yet. +/** This phase rewrites pattern matches. + * FIXME: A more detailed explanation would be good. */ class PatternMatcher extends MiniPhaseTransform with DenotTransformer { import dotty.tools.dotc.ast.tpd._ diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 049ccb22e00d..d8a4b5d0c4af 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -58,6 +58,8 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran /** the following two members override abstract members in Transform */ override def phaseName: String = "posttyper" + override def changesMembers = true // the phase adds super accessors and synthetic methods + override def transformPhase(implicit ctx: Context) = thisTransformer.next protected def newTransformer(implicit ctx: Context): Transformer = diff --git a/compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala b/compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala index 7c51ba59388f..fec7bd741dab 100644 --- a/compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala +++ b/compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala @@ -37,6 +37,8 @@ class PrimitiveForwarders extends MiniPhaseTransform with IdentityDenotTransform override def runsAfter = Set(classOf[ResolveSuper]) + override def changesMembers = true // the phase adds primitive forwarders + override def transformTemplate(impl: Template)(implicit ctx: Context, info: TransformerInfo) = { val cls = impl.symbol.owner.asClass val ops = new MixinOps(cls, thisTransform) diff --git a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala index de0242f79b27..64bcc4a63afd 100644 --- a/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala +++ b/compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala @@ -53,6 +53,8 @@ class ResolveSuper extends MiniPhaseTransform with IdentityDenotTransformer { th override def runsAfter = Set(classOf[ElimByName], // verified empirically, need to figure out what the reason is. classOf[AugmentScala2Traits]) + override def changesMembers = true // the phase adds super accessors and method forwarders + override def transformTemplate(impl: Template)(implicit ctx: Context, info: TransformerInfo) = { val cls = impl.symbol.owner.asClass val ops = new MixinOps(cls, thisTransform) diff --git a/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala b/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala index 8b9d2be0d021..12aa18bd2734 100644 --- a/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala +++ b/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala @@ -22,6 +22,8 @@ class RestoreScopes extends MiniPhaseTransform with IdentityDenotTransformer { t import ast.tpd._ override def phaseName = "restoreScopes" + override def changesMembers = true + /* Note: We need to wait until we see a package definition because * DropEmptyConstructors changes template members when analyzing the * enclosing package definitions. So by the time RestoreScopes gets to diff --git a/compiler/src/dotty/tools/dotc/transform/ShortcutImplicits.scala b/compiler/src/dotty/tools/dotc/transform/ShortcutImplicits.scala index ec82b5d33b74..91301c0cd86e 100644 --- a/compiler/src/dotty/tools/dotc/transform/ShortcutImplicits.scala +++ b/compiler/src/dotty/tools/dotc/transform/ShortcutImplicits.scala @@ -48,6 +48,9 @@ class ShortcutImplicits extends MiniPhase with IdentityDenotTransformer { thisTr import tpd._ override def phaseName: String = "shortcutImplicits" + + override def changesMembers = true // the phase adds "direct" methods + val treeTransform = new Transform /** If this option is true, we don't specialize symbols that are known to be only From d2b3fffecb783a6af27fa6bc6539bdb0e186633e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 17:54:53 +0200 Subject: [PATCH 08/22] Refactorings to caches - transformers caches as long as they are valid in new phase - avoid state in invalid dummy caches, so that they don't need to be put in contexts - refactor caches into traits and implementation classes --- .../src/dotty/tools/dotc/core/Contexts.scala | 4 - .../tools/dotc/core/SymDenotations.scala | 172 ++++++++++++------ .../src/dotty/tools/dotc/core/Symbols.scala | 2 +- .../tools/dotc/transform/TreeTransform.scala | 2 +- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- 5 files changed, 117 insertions(+), 65 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 10ee4599a1c9..b299de4340c3 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -575,10 +575,6 @@ object Contexts { /** The standard definitions */ val definitions = new Definitions - /** Sinks for "onBehalf" chains */ - val dummyMemberNames = new MemberNames - val dummyBaseData = new BaseData - /** Initializes the `ContextBase` with a starting context. * This initializes the `platform` and the `definitions`. */ diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index cdd38f9663ef..2cd996be3c24 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -4,7 +4,7 @@ package core import Periods._, Contexts._, Symbols._, Denotations._, Names._, NameOps._, Annotations._ import Types._, Flags._, Decorators._, DenotTransformers._, StdNames._, Scopes._, Comments._ -import NameOps._, NameKinds._ +import NameOps._, NameKinds._, Phases._ import Scopes.Scope import collection.mutable import collection.BitSet @@ -1180,6 +1180,11 @@ object SymDenotations { d } + /** Copy mamberNames and baseData caches from given denotation, provided + * they are valid at given `phase`. + */ + def copyCaches(from: SymDenotation, phase: Phase)(implicit ctx: Context): this.type = this + /** Are `info1` and `info2` ClassInfo types with different parents? */ protected def changedClassParents(info1: Type, info2: Type): Boolean = info2 match { @@ -1224,6 +1229,57 @@ object SymDenotations { import util.LRUCache + // ----- caches ------------------------------------------------------- + + private[this] var myTypeParams: List[TypeSymbol] = null + + private[this] var myMemberCache: LRUCache[Name, PreDenotation] = null + private[this] var myMemberCachePeriod: Period = Nowhere + + private[this] var baseTypeRefCache: java.util.HashMap[CachedType, Type] = null + private[this] var baseTypeRefValid: RunId = NoRunId + + private var baseDataCache: BaseData = null + private var memberNamesCache: MemberNames = null + + private def memberCache(implicit ctx: Context): LRUCache[Name, PreDenotation] = { + if (myMemberCachePeriod != ctx.period) { + myMemberCache = new LRUCache + myMemberCachePeriod = ctx.period + } + myMemberCache + } + + private def baseDataCacheValid(implicit ctx: Context) = + baseDataCache != null && baseDataCache.isValid + + private def invalidateBaseDataCache() = + if (baseDataCache != null) { + baseDataCache.invalidate() + baseDataCache = null + } + + private def memberNamesCacheValid(implicit ctx: Context) = + memberNamesCache != null && memberNamesCache.isValid + + private def invalidateMemberNamesCache() = + if (memberNamesCache != null) { + memberNamesCache.invalidate() + memberNamesCache = null + } + + override def copyCaches(from: SymDenotation, phase: Phase)(implicit ctx: Context): this.type = { + from match { + case from: ClassDenotation => + if (from.memberNamesCache != null && from.memberNamesCache.isValidAt(phase)) + memberNamesCache = from.memberNamesCache + if (from.baseDataCache != null && from.baseDataCache.isValidAt(phase)) + baseDataCache = from.baseDataCache + case _ => + } + this + } + // ----- denotation fields and accessors ------------------------------ if (initFlags is (Module, butNot = Package)) @@ -1235,9 +1291,6 @@ object SymDenotations { /** The info asserted to have type ClassInfo */ def classInfo(implicit ctx: Context): ClassInfo = info.asInstanceOf[ClassInfo] - /** TODO: Document why caches are supposedly safe to use */ - private[this] var myTypeParams: List[TypeSymbol] = _ - /** The type parameters in this class, in the order they appear in the current * scope `decls`. This might be temporarily the incorrect order when * reading Scala2 pickled info. The problem is fixed by `ensureTypeParamsInCorrectOrder`, @@ -1375,20 +1428,9 @@ object SymDenotations { myTypeRef } - private[this] var baseClassesCache: BaseData = null - - private def baseClassesCacheValid = baseClassesCache != null && baseClassesCache.isValid - - private def invalidateBaseDataCache() = { - if (baseClassesCacheValid) { - baseClassesCache.invalidate() - baseClassesCache = null - } - } - private def baseData(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { - if (!baseClassesCacheValid) baseClassesCache = new BaseData - baseClassesCache(this) + if (!baseDataCacheValid) baseDataCache = BaseData() + baseDataCache(this) } /** The base classes of this class in linearization order, @@ -1494,17 +1536,6 @@ object SymDenotations { fp } - private[this] var myMemberCache: LRUCache[Name, PreDenotation] = null - private[this] var myMemberCachePeriod: Period = Nowhere - - private def memberCache(implicit ctx: Context): LRUCache[Name, PreDenotation] = { - if (myMemberCachePeriod != ctx.period) { - myMemberCache = new LRUCache - myMemberCachePeriod = ctx.period - } - myMemberCache - } - /** Hook to do a pre-enter test. Overridden in PackageDenotation */ protected def proceedWithEnter(sym: Symbol, mscope: MutableScope)(implicit ctx: Context): Boolean = true @@ -1654,9 +1685,6 @@ object SymDenotations { raw.filterExcluded(excluded).asSeenFrom(pre).toDenot(pre) } - private[this] var baseTypeRefCache: java.util.HashMap[CachedType, Type] = null - private[this] var baseTypeRefValid: RunId = NoRunId - /** Compute tp.baseTypeRef(this) */ final def baseTypeRefOf(tp: Type)(implicit ctx: Context): Type = { @@ -1739,22 +1767,11 @@ object SymDenotations { } } - private[this] var memberNamesCache: MemberNames = null - - private def memberNamesCacheValid = memberNamesCache != null && memberNamesCache.isValid - - private def invalidateMemberNamesCache() = { - if (memberNamesCacheValid) { - memberNamesCache.invalidate() - memberNamesCache = null - } - } - def memberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = if ((this is PackageClass) || !Config.cacheMemberNames) computeMemberNames(keepOnly) // don't cache package member names; they might change else { - if (!memberNamesCacheValid) memberNamesCache = new MemberNames + if (!memberNamesCacheValid) memberNamesCache = MemberNames() memberNamesCache(keepOnly, this) } @@ -1811,6 +1828,7 @@ object SymDenotations { val ClassInfo(pre, _, ps, decls, selfInfo) = classInfo if (classInfo(prevCtx).decls eq decls) copySymDenotation(info = ClassInfo(pre, classSymbol, ps, decls.cloneScope, selfInfo)) + .copyCaches(this, phase.next) .installAfter(phase) } } @@ -2038,12 +2056,44 @@ object SymDenotations { } } - abstract class InheritedCache { + trait InheritedCache { + def isValid(implicit ctx: Context): Boolean + def isValidAt(phase: Phase)(implicit ctx: Context): Boolean def invalidate(): Unit + } + + trait MemberNames extends InheritedCache { + def apply(keepOnly: NameFilter, clsd: ClassDenotation) + (implicit onBehalf: MemberNames, ctx: Context): Set[Name] + } + + object MemberNames { + implicit val None: MemberNames = new InvalidCache with MemberNames { + def apply(keepOnly: NameFilter, clsd: ClassDenotation)(implicit onBehalf: MemberNames, ctx: Context) = ??? + } + def apply()(implicit ctx: Context): MemberNames = new MemberNamesImpl(ctx.period) + } + + trait BaseData extends InheritedCache { + def apply(clsd: ClassDenotation) + (implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) + def signalProvisional(): Unit + } + + object BaseData { + implicit val None: BaseData = new InvalidCache with BaseData { + def apply(clsd: ClassDenotation)(implicit onBehalf: BaseData, ctx: Context) = ??? + def signalProvisional() = () + } + def apply()(implicit ctx: Context): BaseData = new BaseDataImpl(ctx.period) + } + + private abstract class InheritedCacheImpl(val createdAt: Period) extends InheritedCache { + protected def sameGroup(p1: Phase, p2: Phase): Boolean private[this] var dependent: WeakHashMap[InheritedCache, Unit] = null - protected[this] def invalidateDependents() = { + protected def invalidateDependents() = { if (dependent != null) { val it = dependent.keySet.iterator() while (it.hasNext()) it.next().invalidate() @@ -2051,16 +2101,26 @@ object SymDenotations { dependent = null } - protected[this] def addDependent(dep: InheritedCache) = { + protected def addDependent(dep: InheritedCache) = { if (dependent == null) dependent = new WeakHashMap dependent.put(dep, ()) } + + def isValidAt(phase: Phase)(implicit ctx: Context) = + createdAt.runId == ctx.runId && sameGroup(ctx.phases(createdAt.phaseId), phase) + } + + private class InvalidCache extends InheritedCache { + def isValid(implicit ctx: Context) = false + def isValidAt(phase: Phase)(implicit ctx: Context) = false + def invalidate(): Unit = () } - class MemberNames extends InheritedCache { + private class MemberNamesImpl(createdAt: Period) extends InheritedCacheImpl(createdAt) with MemberNames { private[this] var cache: SimpleMap[NameFilter, Set[Name]] = SimpleMap.Empty - final def isValid: Boolean = cache != null + final def isValid(implicit ctx: Context): Boolean = + cache != null && isValidAt(ctx.phase) private var locked = false @@ -2069,7 +2129,7 @@ object SymDenotations { * proceed as usual. However, all cache entries should be cleared. */ def invalidate(): Unit = - if (isValid) + if (cache != null) if (locked) cache = SimpleMap.Empty else { cache = null @@ -2091,23 +2151,21 @@ object SymDenotations { } finally addDependent(onBehalf) } - } - object MemberNames { - implicit def onBehalfOfNone(implicit ctx: Context): MemberNames = ctx.dummyMemberNames + def sameGroup(p1: Phase, p2: Phase) = p1.membersGroup == p2.membersGroup } - class BaseData extends InheritedCache { + private class BaseDataImpl(createdAt: Period) extends InheritedCacheImpl(createdAt) with BaseData { private[this] var cache: (List[ClassSymbol], BaseClassSet) = null private[this] var valid = true private[this] var locked = false private[this] var provisional = false - final def isValid: Boolean = valid + final def isValid(implicit ctx: Context): Boolean = valid && isValidAt(ctx.phase) def invalidate(): Unit = - if (isValid && !locked) { + if (valid && !locked) { invalidateCount += 1 cache = null valid = false @@ -2135,10 +2193,8 @@ object SymDenotations { } finally addDependent(onBehalf) } - } - object BaseData { - implicit def onBehalfOfNone(implicit ctx: Context): BaseData = ctx.dummyBaseData + def sameGroup(p1: Phase, p2: Phase) = p1.parentsGroup == p2.parentsGroup } object FingerPrint { diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index b4691525109a..a360ee15abfe 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -468,7 +468,7 @@ object Symbols { } else this.owner.asClass.ensureFreshScopeAfter(phase) if (!this.flagsUNSAFE.is(Private)) - assert(phase.changesMembers, i"$this entered in ${this.owner} after undeclared phase $phase") + assert(phase.changesMembers, i"$this entered in ${this.owner} at undeclared phase $phase") entered } diff --git a/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala b/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala index b0bd40578c17..feacd2774b53 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala @@ -187,7 +187,7 @@ object TreeTransforms { else { val derivedAnnots = (annots, annotTrees1).zipped.map((annot, annotTree1) => annot.derivedAnnotation(annotTree1)) - ref1.copySymDenotation(annotations = derivedAnnots) + ref1.copySymDenotation(annotations = derivedAnnots).copyCaches(ref1, ctx.phase.next) } case ref1 => ref1 diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 1ce97098a61b..05539f58d1b2 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -911,7 +911,7 @@ class Namer { typer: Typer => Checking.checkWellFormed(cls) if (isDerivedValueClass(cls)) cls.setFlag(Final) cls.info = avoidPrivateLeaks(cls, cls.pos) - cls.baseClasses.foreach(_.invalidateBaseTypeRefCache) + cls.baseClasses.foreach(_.invalidateBaseTypeRefCache) // TODO: needed? } } From 7a2f158fb2dcfbb4b9dc68f0ff42db700c6b8ec1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 18:16:14 +0200 Subject: [PATCH 09/22] Don't use null for inherited caches The lgoxi is much nicer without it. --- .../tools/dotc/core/SymDenotations.scala | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 2cd996be3c24..ca80fd2d15d1 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1239,8 +1239,8 @@ object SymDenotations { private[this] var baseTypeRefCache: java.util.HashMap[CachedType, Type] = null private[this] var baseTypeRefValid: RunId = NoRunId - private var baseDataCache: BaseData = null - private var memberNamesCache: MemberNames = null + private var baseDataCache: BaseData = BaseData.None + private var memberNamesCache: MemberNames = MemberNames.None private def memberCache(implicit ctx: Context): LRUCache[Name, PreDenotation] = { if (myMemberCachePeriod != ctx.period) { @@ -1250,31 +1250,21 @@ object SymDenotations { myMemberCache } - private def baseDataCacheValid(implicit ctx: Context) = - baseDataCache != null && baseDataCache.isValid - - private def invalidateBaseDataCache() = - if (baseDataCache != null) { - baseDataCache.invalidate() - baseDataCache = null - } - - private def memberNamesCacheValid(implicit ctx: Context) = - memberNamesCache != null && memberNamesCache.isValid + private def invalidateBaseDataCache() = { + baseDataCache.invalidate() + baseDataCache = BaseData.None + } - private def invalidateMemberNamesCache() = - if (memberNamesCache != null) { - memberNamesCache.invalidate() - memberNamesCache = null - } + private def invalidateMemberNamesCache() = { + memberNamesCache.invalidate() + memberNamesCache = MemberNames.None + } override def copyCaches(from: SymDenotation, phase: Phase)(implicit ctx: Context): this.type = { from match { case from: ClassDenotation => - if (from.memberNamesCache != null && from.memberNamesCache.isValidAt(phase)) - memberNamesCache = from.memberNamesCache - if (from.baseDataCache != null && from.baseDataCache.isValidAt(phase)) - baseDataCache = from.baseDataCache + if (from.memberNamesCache.isValidAt(phase)) memberNamesCache = from.memberNamesCache + if (from.baseDataCache.isValidAt(phase)) baseDataCache = from.baseDataCache case _ => } this @@ -1429,7 +1419,7 @@ object SymDenotations { } private def baseData(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { - if (!baseDataCacheValid) baseDataCache = BaseData() + if (!baseDataCache.isValid) baseDataCache = BaseData.newCache() baseDataCache(this) } @@ -1771,7 +1761,7 @@ object SymDenotations { if ((this is PackageClass) || !Config.cacheMemberNames) computeMemberNames(keepOnly) // don't cache package member names; they might change else { - if (!memberNamesCacheValid) memberNamesCache = MemberNames() + if (!memberNamesCache.isValid) memberNamesCache = MemberNames.newCache() memberNamesCache(keepOnly, this) } @@ -2071,7 +2061,7 @@ object SymDenotations { implicit val None: MemberNames = new InvalidCache with MemberNames { def apply(keepOnly: NameFilter, clsd: ClassDenotation)(implicit onBehalf: MemberNames, ctx: Context) = ??? } - def apply()(implicit ctx: Context): MemberNames = new MemberNamesImpl(ctx.period) + def newCache()(implicit ctx: Context): MemberNames = new MemberNamesImpl(ctx.period) } trait BaseData extends InheritedCache { @@ -2085,7 +2075,7 @@ object SymDenotations { def apply(clsd: ClassDenotation)(implicit onBehalf: BaseData, ctx: Context) = ??? def signalProvisional() = () } - def apply()(implicit ctx: Context): BaseData = new BaseDataImpl(ctx.period) + def newCache()(implicit ctx: Context): BaseData = new BaseDataImpl(ctx.period) } private abstract class InheritedCacheImpl(val createdAt: Period) extends InheritedCache { From 12b91f9dea98065b4f1998b3b912406d7ae2ad22 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 18:49:41 +0200 Subject: [PATCH 10/22] Invalidate less on info_= --- .../tools/dotc/core/DenotTransformers.scala | 6 ++++-- .../dotty/tools/dotc/core/SymDenotations.scala | 17 +++++++++-------- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/DenotTransformers.scala b/compiler/src/dotty/tools/dotc/core/DenotTransformers.scala index 02d27ea3389e..c62008e6eada 100644 --- a/compiler/src/dotty/tools/dotc/core/DenotTransformers.scala +++ b/compiler/src/dotty/tools/dotc/core/DenotTransformers.scala @@ -44,8 +44,10 @@ object DenotTransformers { val info1 = transformInfo(ref.info, ref.symbol) if (info1 eq ref.info) ref else ref match { - case ref: SymDenotation => ref.copySymDenotation(info = info1) - case _ => ref.derivedSingleDenotation(ref.symbol, info1) + case ref: SymDenotation => + ref.copySymDenotation(info = info1).copyCaches(ref, ctx.phase.next) + case _ => + ref.derivedSingleDenotation(ref.symbol, info1) } } } diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index ca80fd2d15d1..50bccb5b0489 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1170,7 +1170,7 @@ object SymDenotations { { // simulate default parameters, while also passing implicit context ctx to the default values val initFlags1 = (if (initFlags != UndefinedFlags) initFlags else this.flags) &~ Frozen val info1 = if (info != null) info else this.info - if (ctx.isAfterTyper && changedClassParents(info, info1)) + if (ctx.isAfterTyper && changedClassParents(info, info1, completersMatter = false)) assert(ctx.phase.changesParents, i"undeclared parent change at ${ctx.phase} for $this, was: $info, now: $info1") val privateWithin1 = if (privateWithin != null) privateWithin else this.privateWithin val annotations1 = if (annotations != null) annotations else this.annotations @@ -1185,15 +1185,17 @@ object SymDenotations { */ def copyCaches(from: SymDenotation, phase: Phase)(implicit ctx: Context): this.type = this - /** Are `info1` and `info2` ClassInfo types with different parents? */ - protected def changedClassParents(info1: Type, info2: Type): Boolean = + /** Are `info1` and `info2` ClassInfo types with different parents? + * @param completersMatter if `true`, consider parents changed if `info1` or `info2 `is a type completer + */ + protected def changedClassParents(info1: Type, info2: Type, completersMatter: Boolean): Boolean = info2 match { case info2: ClassInfo => info1 match { case info1: ClassInfo => info1.classParents ne info2.classParents - case _ => false + case _ => completersMatter } - case _ => false + case _ => completersMatter } override def initial: SymDenotation = super.initial.asSymDenotation @@ -1307,12 +1309,11 @@ object SymDenotations { } override protected[dotc] final def info_=(tp: Type) = { - super.info_=(tp) - if (changedClassParents(infoOrCompleter, tp) || true) { + if (changedClassParents(infoOrCompleter, tp, completersMatter = true)) invalidateBaseDataCache() - } invalidateMemberNamesCache() myTypeParams = null // changing the info might change decls, and with it typeParams + super.info_=(tp) } /** The denotations of all parents in this class. */ diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 05539f58d1b2..b8cf2dc4f286 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -911,7 +911,7 @@ class Namer { typer: Typer => Checking.checkWellFormed(cls) if (isDerivedValueClass(cls)) cls.setFlag(Final) cls.info = avoidPrivateLeaks(cls, cls.pos) - cls.baseClasses.foreach(_.invalidateBaseTypeRefCache) // TODO: needed? + cls.baseClasses.foreach(_.invalidateBaseTypeRefCache) // we might have looked before and found nothing } } From 7afda828a994ed301c4717314b6aa6f2ed91c738 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 18:58:47 +0200 Subject: [PATCH 11/22] Remove unnecessary cache invalidation --- .../dotty/tools/dotc/core/Denotations.scala | 2 +- .../tools/dotc/core/SymDenotations.scala | 55 +++---------------- 2 files changed, 8 insertions(+), 49 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index c0bad8ded708..061ec56e6589 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -805,7 +805,7 @@ object Denotations { val transformer = ctx.denotTransformers(nextTransformerId) //println(s"transforming $this with $transformer") try { - next = transformer.transform(cur)(ctx.withPhase(transformer)).syncWithParents + next = transformer.transform(cur)(ctx.withPhase(transformer)) } catch { case ex: CyclicReference => println(s"error while transforming $this") // DEBUG diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 50bccb5b0489..d7a69b37f9a8 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1262,6 +1262,9 @@ object SymDenotations { memberNamesCache = MemberNames.None } + def invalidateBaseTypeRefCache() = + baseTypeRefCache = new java.util.HashMap[CachedType, Type] + override def copyCaches(from: SymDenotation, phase: Phase)(implicit ctx: Context): this.type = { from match { case from: ClassDenotation => @@ -1348,45 +1351,6 @@ object SymDenotations { isCompleted && testFullyCompleted && { setFlag(FullyCompleted); true } } - // ------ syncing inheritance-related info ----------------------------- - - private var firstRunId: RunId = initRunId - - /** invalidate caches influenced by parent classes if one of the parents - * is younger than the denotation itself. - */ - override def syncWithParents(implicit ctx: Context): SingleDenotation = { - def isYounger(tref: TypeRef) = tref.symbol.denot match { - case denot: ClassDenotation => - if (denot.validFor.runId < ctx.runId) denot.current // syncs with its parents in turn - val result = denot.firstRunId > this.firstRunId - if (result) incremental.println(s"$denot is younger than $this") - result - case _ => false - } - val parentIsYounger = (firstRunId < ctx.runId) && { - infoOrCompleter match { - case cinfo: ClassInfo => cinfo.classParents exists isYounger - case _ => false - } - } - if (parentIsYounger) { - incremental.println(s"parents of $this are invalid; symbol id = ${symbol.id}, copying ...\n") - invalidateInheritedInfo() - } - firstRunId = ctx.runId - this - } - - /** Invalidate all caches and fields that depend on base classes and their contents */ - override def invalidateInheritedInfo(): Unit = { - myMemberFingerPrint = FingerPrint.unknown - myMemberCache = null - myMemberCachePeriod = Nowhere - invalidateBaseDataCache() - invalidateMemberNamesCache() - } - // ------ class-specific operations ----------------------------------- private[this] var myThisType: Type = null @@ -1441,9 +1405,6 @@ object SymDenotations { baseTypeRefValid = ctx.runId } - def invalidateBaseTypeRefCache() = - baseTypeRefCache = new java.util.HashMap[CachedType, Type] - def computeBaseData(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { val seen = mutable.SortedSet[Int]() def addBaseClasses(bcs: List[ClassSymbol], to: List[ClassSymbol]) @@ -1571,8 +1532,7 @@ object SymDenotations { if (myMemberFingerPrint != FingerPrint.unknown) myMemberFingerPrint.include(sym.name) - if (myMemberCache != null) - myMemberCache invalidate sym.name + if (myMemberCache != null) myMemberCache.invalidate(sym.name) if (!sym.flagsUNSAFE.is(Private)) invalidateMemberNamesCache() } @@ -1583,8 +1543,7 @@ object SymDenotations { def replace(prev: Symbol, replacement: Symbol)(implicit ctx: Context): Unit = { require(!(this is Frozen)) unforcedDecls.openForMutations.replace(prev, replacement) - if (myMemberCache != null) - myMemberCache invalidate replacement.name + if (myMemberCache != null) myMemberCache.invalidate(replacement.name) } /** Delete symbol from current scope. @@ -1595,8 +1554,8 @@ object SymDenotations { require(!(this is Frozen)) info.decls.openForMutations.unlink(sym) myMemberFingerPrint = FingerPrint.unknown - if (myMemberCache != null) myMemberCache invalidate sym.name - invalidateMemberNamesCache() + if (myMemberCache != null) myMemberCache.invalidate(sym.name) + if (!sym.flagsUNSAFE.is(Private)) invalidateMemberNamesCache() } /** Make sure the type parameters of this class appear in the order given From ddbf8f70fb0c61330048715328675b34361831c5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 19:08:52 +0200 Subject: [PATCH 12/22] Remove FingerPrints --- .../src/dotty/tools/dotc/config/Config.scala | 1 - .../tools/dotc/core/SymDenotations.scala | 119 ++++-------------- 2 files changed, 22 insertions(+), 98 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index f19708c4ec97..13dd45037813 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -4,7 +4,6 @@ object Config { final val cacheMembersNamed = true final val cacheAsSeenFrom = true - final val useFingerPrints = true // note: it currently seems to be slightly faster not to use them! my junit test: 548s without, 560s with. final val cacheMemberNames = true final val cacheImplicitScopes = true diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index d7a69b37f9a8..02eacd55b2d0 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1452,42 +1452,6 @@ object SymDenotations { final override def typeParamCreationFlags = ClassTypeParamCreationFlags - private[this] var myMemberFingerPrint: FingerPrint = FingerPrint.unknown - - private def computeMemberFingerPrint(implicit ctx: Context): FingerPrint = { - var fp = FingerPrint() - var e = info.decls.lastEntry - while (e != null) { - fp.include(e.name) - e = e.prev - } - var ps = classParents - while (ps.nonEmpty) { - val parent = ps.head.typeSymbol - parent.denot match { - case parentDenot: ClassDenotation => - fp.include(parentDenot.memberFingerPrint) - if (parentDenot.isFullyCompleted) parentDenot.setFlag(Frozen) - case _ => - } - ps = ps.tail - } - fp - } - - /** A bloom filter for the names of all members in this class. - * Makes sense only for parent classes, and should definitely - * not be used for package classes because cache never - * gets invalidated. - */ - def memberFingerPrint(implicit ctx: Context): FingerPrint = - if (myMemberFingerPrint != FingerPrint.unknown) myMemberFingerPrint - else { - val fp = computeMemberFingerPrint - if (isFullyCompleted) myMemberFingerPrint = fp - fp - } - /** Hook to do a pre-enter test. Overridden in PackageDenotation */ protected def proceedWithEnter(sym: Symbol, mscope: MutableScope)(implicit ctx: Context): Boolean = true @@ -1530,8 +1494,6 @@ object SymDenotations { scope.enter(sym) - if (myMemberFingerPrint != FingerPrint.unknown) - myMemberFingerPrint.include(sym.name) if (myMemberCache != null) myMemberCache.invalidate(sym.name) if (!sym.flagsUNSAFE.is(Private)) invalidateMemberNamesCache() } @@ -1553,7 +1515,6 @@ object SymDenotations { def delete(sym: Symbol)(implicit ctx: Context) = { require(!(this is Frozen)) info.decls.openForMutations.unlink(sym) - myMemberFingerPrint = FingerPrint.unknown if (myMemberCache != null) myMemberCache.invalidate(sym.name) if (!sym.flagsUNSAFE.is(Private)) invalidateMemberNamesCache() } @@ -1603,31 +1564,27 @@ object SymDenotations { } private[core] def computeNPMembersNamed(name: Name, inherited: Boolean)(implicit ctx: Context): PreDenotation = /*>|>*/ Stats.track("computeNPMembersNamed") /*<|<*/ { - if (!inherited || - !Config.useFingerPrints || - (memberFingerPrint contains name)) { - Stats.record("computeNPMembersNamed after fingerprint") - ensureCompleted() - val ownDenots = info.decls.denotsNamed(name, selectNonPrivate) - if (debugTrace) // DEBUG - println(s"$this.member($name), ownDenots = $ownDenots") - def collect(denots: PreDenotation, parents: List[TypeRef]): PreDenotation = parents match { - case p :: ps => - val denots1 = collect(denots, ps) - p.symbol.denot match { - case parentd: ClassDenotation => - denots1 union - parentd.nonPrivateMembersNamed(name, inherited = true) - .mapInherited(ownDenots, denots1, thisType) - case _ => - denots1 - } - case nil => - denots - } - if (name.isConstructorName) ownDenots - else collect(ownDenots, classParents) - } else NoDenotation + Stats.record("computeNPMembersNamed after fingerprint") + ensureCompleted() + val ownDenots = info.decls.denotsNamed(name, selectNonPrivate) + if (debugTrace) // DEBUG + println(s"$this.member($name), ownDenots = $ownDenots") + def collect(denots: PreDenotation, parents: List[TypeRef]): PreDenotation = parents match { + case p :: ps => + val denots1 = collect(denots, ps) + p.symbol.denot match { + case parentd: ClassDenotation => + denots1 union + parentd.nonPrivateMembersNamed(name, inherited = true) + .mapInherited(ownDenots, denots1, thisType) + case _ => + denots1 + } + case nil => + denots + } + if (name.isConstructorName) ownDenots + else collect(ownDenots, classParents) } override final def findMember(name: Name, pre: Type, excluded: FlagSet)(implicit ctx: Context): Denotation = { @@ -1981,30 +1938,7 @@ object SymDenotations { } } - // ---- Fingerprints ----------------------------------------------------- - - /** A fingerprint is a bitset that acts as a bloom filter for sets - * of names. - */ - class FingerPrint(val bits: Array[Long]) extends AnyVal { - import FingerPrint._ - - /** Include some bits of name's hashcode in set */ - def include(name: Name): Unit = { - val hash = name.hashCode & Mask - bits(hash >> WordSizeLog) |= (1L << hash) - } - - /** Include all bits of `that` fingerprint in set */ - def include(that: FingerPrint): Unit = - for (i <- 0 until NumWords) bits(i) |= that.bits(i) - - /** Does set contain hash bits of given name? */ - def contains(name: Name): Boolean = { - val hash = name.hashCode & Mask - (bits(hash >> WordSizeLog) & (1L << hash)) != 0 - } - } + // ---- Caches for inherited info ----------------------------------------- trait InheritedCache { def isValid(implicit ctx: Context): Boolean @@ -2147,15 +2081,6 @@ object SymDenotations { def sameGroup(p1: Phase, p2: Phase) = p1.parentsGroup == p2.parentsGroup } - object FingerPrint { - def apply() = new FingerPrint(new Array[Long](NumWords)) - val unknown = new FingerPrint(null) - private final val WordSizeLog = 6 - private final val NumWords = 32 - private final val NumBits = NumWords << WordSizeLog - private final val Mask = NumBits - 1 - } - class BaseClassSet(val classIds: Array[Int]) extends AnyVal { def contains(sym: Symbol): Boolean = { val id = sym.id From e86e314d9f103ebe808d4e4790b24f8d620b91ef Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 19:15:04 +0200 Subject: [PATCH 13/22] Get rid of Frozen flag No longer needed because we have proper cache invalidation now --- .../src/dotty/tools/dotc/core/Denotations.scala | 1 - compiler/src/dotty/tools/dotc/core/Flags.scala | 5 +---- .../src/dotty/tools/dotc/core/SymDenotations.scala | 14 +------------- compiler/src/dotty/tools/dotc/core/Symbols.scala | 2 +- 4 files changed, 3 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 061ec56e6589..64c0bedddc6b 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -817,7 +817,6 @@ object Denotations { next match { case next: ClassDenotation => assert(!next.is(Package), s"illegal transformation of package denotation by transformer ${ctx.withPhase(transformer).phase}") - next.resetFlag(Frozen) case _ => } next.insertAfter(cur) diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 5d4ab9922910..4d50025a4694 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -375,9 +375,6 @@ object Flags { /** Denotation is in train of being loaded and completed, used to catch cyclic dependencies */ final val Touched = commonFlag(48, "") - /** Class is not allowed to accept new members because fingerprint of subclass has been taken */ - final val Frozen = commonFlag(49, "") - /** An error symbol */ final val Erroneous = commonFlag(50, "") @@ -450,7 +447,7 @@ object Flags { Module | Package | Deferred | MethodOrHKCommon | Param | ParamAccessor | Scala2ExistentialCommon | Mutable.toCommonFlags | Touched | JavaStatic | CovariantOrOuter | ContravariantOrLabel | CaseAccessorOrBaseTypeArg | - Fresh | Frozen | Erroneous | ImplicitCommon | Permanent | Synthetic | + Fresh | Erroneous | ImplicitCommon | Permanent | Synthetic | SuperAccessorOrScala2x | Inline /** Flags guaranteed to be set upon symbol creation, or, if symbol is a top-level diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 02eacd55b2d0..5fc99d8fe50f 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1168,7 +1168,7 @@ object SymDenotations { privateWithin: Symbol = null, annotations: List[Annotation] = null)(implicit ctx: Context) = { // simulate default parameters, while also passing implicit context ctx to the default values - val initFlags1 = (if (initFlags != UndefinedFlags) initFlags else this.flags) &~ Frozen + val initFlags1 = (if (initFlags != UndefinedFlags) initFlags else this.flags) val info1 = if (info != null) info else this.info if (ctx.isAfterTyper && changedClassParents(info, info1, completersMatter = false)) assert(ctx.phase.changesParents, i"undeclared parent change at ${ctx.phase} for $this, was: $info, now: $info1") @@ -1483,17 +1483,7 @@ object SymDenotations { /** Enter a symbol in given `scope` without potentially replacing the old copy. */ def enterNoReplace(sym: Symbol, scope: MutableScope)(implicit ctx: Context): Unit = { - def isUsecase = ctx.docCtx.isDefined && sym.name.show.takeRight(4) == "$doc" - require( - (sym.denot.flagsUNSAFE is Private) || - !(this is Frozen) || - (scope ne this.unforcedDecls) || - sym.hasAnnotation(defn.ScalaStaticAnnot) || - sym.name.is(InlineAccessorName) || - isUsecase, i"trying to enter $sym in $this, frozen = ${this is Frozen}") - scope.enter(sym) - if (myMemberCache != null) myMemberCache.invalidate(sym.name) if (!sym.flagsUNSAFE.is(Private)) invalidateMemberNamesCache() } @@ -1503,7 +1493,6 @@ object SymDenotations { * @pre `prev` and `replacement` have the same name. */ def replace(prev: Symbol, replacement: Symbol)(implicit ctx: Context): Unit = { - require(!(this is Frozen)) unforcedDecls.openForMutations.replace(prev, replacement) if (myMemberCache != null) myMemberCache.invalidate(replacement.name) } @@ -1513,7 +1502,6 @@ object SymDenotations { * someone does a findMember on a subclass. */ def delete(sym: Symbol)(implicit ctx: Context) = { - require(!(this is Frozen)) info.decls.openForMutations.unlink(sym) if (myMemberCache != null) myMemberCache.invalidate(sym.name) if (!sym.flagsUNSAFE.is(Private)) invalidateMemberNamesCache() diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index a360ee15abfe..e0c09baa5ea3 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -343,7 +343,7 @@ trait Symbols { this: Context => copy.denot = odenot.copySymDenotation( symbol = copy, owner = ttmap1.mapOwner(odenot.owner), - initFlags = odenot.flags &~ (Frozen | Touched) | Fresh, + initFlags = odenot.flags &~ Touched | Fresh, info = completer, privateWithin = ttmap1.mapOwner(odenot.privateWithin), // since this refers to outer symbols, need not include copies (from->to) in ownermap here. annotations = odenot.annotations) From 6c13d544745545d695b770b2ff0ce3db60fa55c2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 19:28:33 +0200 Subject: [PATCH 14/22] Get rid of FullyCompleted flag --- .../src/dotty/tools/dotc/core/Flags.scala | 3 --- .../tools/dotc/core/SymDenotations.scala | 19 +------------------ 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 4d50025a4694..61b057f85b08 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -349,9 +349,6 @@ object Flags { /** A bridge method. Set by Erasure */ final val Bridge = termFlag(34, "") - /** All class attributes are fully defined */ - final val FullyCompleted = typeFlag(34, "") - /** Symbol is a Java varargs bridge */ // (needed?) final val VBridge = termFlag(35, "") // TODO remove diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 5fc99d8fe50f..e2f729cbefa5 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1334,23 +1334,6 @@ object SymDenotations { NoSymbol } - /** The denotation is fully completed: all attributes are fully defined. - * ClassDenotations compiled from source are first completed, then fully completed. - * Packages are never fully completed since members can be added at any time. - * @see Namer#ClassCompleter - */ - private[core] def isFullyCompleted(implicit ctx: Context): Boolean = { - def isFullyCompletedRef(tp: TypeRef) = tp.denot match { - case d: ClassDenotation => d.isFullyCompleted - case _ => false - } - def testFullyCompleted = - if (classParents.isEmpty) !is(Package) && symbol.eq(defn.AnyClass) - else classParents.forall(isFullyCompletedRef) - flagsUNSAFE.is(FullyCompleted) || - isCompleted && testFullyCompleted && { setFlag(FullyCompleted); true } - } - // ------ class-specific operations ----------------------------------- private[this] var myThisType: Type = null @@ -1542,7 +1525,7 @@ object SymDenotations { var denots: PreDenotation = memberCache lookup name if (denots == null) { denots = computeNPMembersNamed(name, inherited) - if (isFullyCompleted) memberCache.enter(name, denots) + memberCache.enter(name, denots) } else if (Config.checkCacheMembersNamed) { val denots1 = computeNPMembersNamed(name, inherited) assert(denots.exists == denots1.exists, s"cache inconsistency: cached: $denots, computed $denots1, name = $name, owner = $this") From 91c901330308bd996f78136d9c1550f2cb2c9f70 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 19:31:35 +0200 Subject: [PATCH 15/22] Mive fullNameCache to caches section --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index e2f729cbefa5..2859553614d8 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1234,6 +1234,7 @@ object SymDenotations { // ----- caches ------------------------------------------------------- private[this] var myTypeParams: List[TypeSymbol] = null + private[this] var fullNameCache: SimpleMap[QualifiedNameKind, Name] = SimpleMap.Empty private[this] var myMemberCache: LRUCache[Name, PreDenotation] = null private[this] var myMemberCachePeriod: Period = Nowhere @@ -1668,7 +1669,6 @@ object SymDenotations { names } - private[this] var fullNameCache: SimpleMap[QualifiedNameKind, Name] = SimpleMap.Empty override final def fullNameSeparated(kind: QualifiedNameKind)(implicit ctx: Context): Name = { val cached = fullNameCache(kind) if (cached != null) cached From d648fd19193dd0a2955e797288364221b69ec9aa Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 20:05:21 +0200 Subject: [PATCH 16/22] Document caches for inherited info --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 2859553614d8..a792284c3f74 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1176,7 +1176,6 @@ object SymDenotations { val annotations1 = if (annotations != null) annotations else this.annotations val d = ctx.SymDenotation(symbol, owner, name, initFlags1, info1, privateWithin1) d.annotations = annotations1 - // TODO: Copy memberCache if info does not change d } @@ -1911,12 +1910,20 @@ object SymDenotations { // ---- Caches for inherited info ----------------------------------------- + /** Base trait for caches that keep info dependent on inherited classes */ trait InheritedCache { + + /** Is the cache valid in current period? */ def isValid(implicit ctx: Context): Boolean + + /** is the cache valid in current run at given phase? */ def isValidAt(phase: Phase)(implicit ctx: Context): Boolean + + /** Render invalid this cache and all cache that depend on it */ def invalidate(): Unit } + /** A cache for sets of member names, indexed by a NameFilter */ trait MemberNames extends InheritedCache { def apply(keepOnly: NameFilter, clsd: ClassDenotation) (implicit onBehalf: MemberNames, ctx: Context): Set[Name] @@ -1929,6 +1936,9 @@ object SymDenotations { def newCache()(implicit ctx: Context): MemberNames = new MemberNamesImpl(ctx.period) } + /** A cache for baseclasses, as a sequence in linearization order and as a set that + * can be queried efficiently for containment. + */ trait BaseData extends InheritedCache { def apply(clsd: ClassDenotation) (implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) From 44b1a9b905af554f85fca8209e38a6b11375487e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 20:07:47 +0200 Subject: [PATCH 17/22] Get rid of superIds --- .../src/dotty/tools/dotc/core/Contexts.scala | 23 ------------------- .../src/dotty/tools/dotc/core/Symbols.scala | 21 ----------------- 2 files changed, 44 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index b299de4340c3..ff11ec35cf54 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -597,26 +597,6 @@ object Contexts { def nextId = { _nextId += 1; _nextId } - /** A map from a superclass id to the typeref of the class that has it */ - private[core] var classOfId = new Array[ClassSymbol](Config.InitialSuperIdsSize) - - /** A map from a the typeref of a class to its superclass id */ - private[core] val superIdOfClass = new mutable.AnyRefMap[ClassSymbol, Int] - - /** The last allocated superclass id */ - private[core] var lastSuperId = -1 - - /** Allocate and return next free superclass id */ - private[core] def nextSuperId: Int = { - lastSuperId += 1 - if (lastSuperId >= classOfId.length) { - val tmp = new Array[ClassSymbol](classOfId.length * 2) - classOfId.copyToArray(tmp) - classOfId = tmp - } - lastSuperId - } - // Types state /** A table for hash consing unique types */ private[core] val uniques = new util.HashSet[Type](Config.initialUniquesCapacity) { @@ -682,9 +662,6 @@ object Contexts { def reset() = { for ((_, set) <- uniqueSets) set.clear() - for (i <- 0 until classOfId.length) classOfId(i) = null - superIdOfClass.clear() - lastSuperId = -1 } // Test that access is single threaded diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index e0c09baa5ea3..8b9f96781033 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -562,27 +562,6 @@ object Symbols { final def classDenot(implicit ctx: Context): ClassDenotation = denot.asInstanceOf[ClassDenotation] - private var superIdHint: Int = -1 - - override def superId(implicit ctx: Context): Int = { - val hint = superIdHint - if (hint >= 0 && hint <= ctx.lastSuperId && (ctx.classOfId(hint) eq this)) - hint - else { - val id = ctx.superIdOfClass get this match { - case Some(id) => - id - case None => - val id = ctx.nextSuperId - ctx.superIdOfClass(this) = id - ctx.classOfId(id) = this - id - } - superIdHint = id - id - } - } - override protected def prefixString = "ClassSymbol" } From 51aebce90b273d52f9733820f083840186a38f28 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 20:44:50 +0200 Subject: [PATCH 18/22] Fix handling of baseTypeRef caches. It's wow subject to the same invalidation as base data caches (except that there's no need for recursive invalidation of caches in subclasses) --- .../tools/dotc/core/SymDenotations.scala | 65 +++++++++++-------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index a792284c3f74..6f5eb2ca0e32 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1238,8 +1238,10 @@ object SymDenotations { private[this] var myMemberCache: LRUCache[Name, PreDenotation] = null private[this] var myMemberCachePeriod: Period = Nowhere - private[this] var baseTypeRefCache: java.util.HashMap[CachedType, Type] = null - private[this] var baseTypeRefValid: RunId = NoRunId + /** A cache from types T to baseTypeRef(T, C) */ + type BaseTypeRefMap = java.util.HashMap[CachedType, Type] + private[this] var myBaseTypeRefCache: BaseTypeRefMap = null + private[this] var myBaseTypeRefCachePeriod: Period = Nowhere private var baseDataCache: BaseData = BaseData.None private var memberNamesCache: MemberNames = MemberNames.None @@ -1252,6 +1254,16 @@ object SymDenotations { myMemberCache } + private def baseTypeRefCache(implicit ctx: Context): BaseTypeRefMap = { + if (myBaseTypeRefCachePeriod != ctx.period && + (myBaseTypeRefCachePeriod.runId != ctx.runId || + ctx.phases(myBaseTypeRefCachePeriod.phaseId).parentsGroup != ctx.phase.parentsGroup)) { + myBaseTypeRefCache = new BaseTypeRefMap + myBaseTypeRefCachePeriod = ctx.period + } + myBaseTypeRefCache + } + private def invalidateBaseDataCache() = { baseDataCache.invalidate() baseDataCache = BaseData.None @@ -1262,14 +1274,19 @@ object SymDenotations { memberNamesCache = MemberNames.None } - def invalidateBaseTypeRefCache() = - baseTypeRefCache = new java.util.HashMap[CachedType, Type] + def invalidateBaseTypeRefCache() = { + myBaseTypeRefCache = null + myBaseTypeRefCachePeriod = Nowhere + } override def copyCaches(from: SymDenotation, phase: Phase)(implicit ctx: Context): this.type = { from match { case from: ClassDenotation => if (from.memberNamesCache.isValidAt(phase)) memberNamesCache = from.memberNamesCache - if (from.baseDataCache.isValidAt(phase)) baseDataCache = from.baseDataCache + if (from.baseDataCache.isValidAt(phase)) { + baseDataCache = from.baseDataCache + myBaseTypeRefCache = from.baseTypeRefCache + } case _ => } this @@ -1381,13 +1398,6 @@ object SymDenotations { private def baseClassSet(implicit onBehalf: BaseData, ctx: Context): BaseClassSet = baseData._2 - /** Invalidate baseTypeRefCache, baseClasses and superClassBits on new run */ - private def checkBasesUpToDate()(implicit ctx: Context) = - if (baseTypeRefValid != ctx.runId) { - invalidateBaseTypeRefCache() - baseTypeRefValid = ctx.runId - } - def computeBaseData(implicit onBehalf: BaseData, ctx: Context): (List[ClassSymbol], BaseClassSet) = { val seen = mutable.SortedSet[Int]() def addBaseClasses(bcs: List[ClassSymbol], to: List[ClassSymbol]) @@ -1571,8 +1581,6 @@ object SymDenotations { case _ => bt } - def inCache(tp: Type) = baseTypeRefCache.containsKey(tp) - /** We cannot cache: * - type variables which are uninstantiated or whose instances can * change, depending on typerstate. @@ -1581,13 +1589,16 @@ object SymDenotations { * and this changes subtyping relations. As a shortcut, we do not * cache ErasedValueType at all. */ - def isCachable(tp: Type): Boolean = tp match { - case _: TypeErasure.ErasedValueType => false - case tp: TypeRef if tp.symbol.isClass => true - case tp: TypeVar => tp.inst.exists && inCache(tp.inst) - case tp: TypeProxy => inCache(tp.underlying) - case tp: AndOrType => inCache(tp.tp1) && inCache(tp.tp2) - case _ => true + def isCachable(tp: Type, btrCache: BaseTypeRefMap): Boolean = { + def inCache(tp: Type) = btrCache.containsKey(tp) + tp match { + case _: TypeErasure.ErasedValueType => false + case tp: TypeRef if tp.symbol.isClass => true + case tp: TypeVar => tp.inst.exists && inCache(tp.inst) + case tp: TypeProxy => inCache(tp.underlying) + case tp: AndOrType => inCache(tp.tp1) && inCache(tp.tp2) + case _ => true + } } def computeBaseTypeRefOf(tp: Type): Type = { @@ -1622,21 +1633,21 @@ object SymDenotations { /*>|>*/ ctx.debugTraceIndented(s"$tp.baseTypeRef($this)") /*<|<*/ { tp match { case tp: CachedType => + val btrCache = baseTypeRefCache try { - checkBasesUpToDate() - var basetp = baseTypeRefCache get tp + var basetp = btrCache get tp if (basetp == null) { - baseTypeRefCache.put(tp, NoPrefix) + btrCache.put(tp, NoPrefix) basetp = computeBaseTypeRefOf(tp) - if (isCachable(tp)) baseTypeRefCache.put(tp, basetp) - else baseTypeRefCache.remove(tp) + if (isCachable(tp, baseTypeRefCache)) btrCache.put(tp, basetp) + else btrCache.remove(tp) } else if (basetp == NoPrefix) throw CyclicReference(this) basetp } catch { case ex: Throwable => - baseTypeRefCache.put(tp, null) + btrCache.put(tp, null) throw ex } case _ => From 3f2ff0ce4e126fc7a7d59486d16523c2975e049b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 13 May 2017 21:21:25 +0200 Subject: [PATCH 19/22] Polish docs --- compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala | 2 +- compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala | 2 +- compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala b/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala index 3a3188ffff58..e5f34e60e2af 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala @@ -44,7 +44,7 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t // This phase moves methods around (in infotransform) so it may need to make other methods public override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[MoveStatics]) - override def changesMembers = true + override def changesMembers = true // the phase introduces new members with mangled names override def checkPostCondition(tree: Tree)(implicit ctx: Context): Unit = { tree match { diff --git a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala index 84b214a23ab3..02cf547b72e5 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala @@ -49,7 +49,7 @@ class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with Ful override def runsAfterGroupsOf = Set(classOf[FirstTransform]) // need companion objects to exist - override def changesMembers = true // the pahse adds extension methods + override def changesMembers = true // the phase adds extension methods override def transform(ref: SingleDenotation)(implicit ctx: Context): SingleDenotation = ref match { case moduleClassSym: ClassDenotation if moduleClassSym is ModuleClass => diff --git a/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala b/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala index 12aa18bd2734..8d2b8e25a61a 100644 --- a/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala +++ b/compiler/src/dotty/tools/dotc/transform/RestoreScopes.scala @@ -22,7 +22,7 @@ class RestoreScopes extends MiniPhaseTransform with IdentityDenotTransformer { t import ast.tpd._ override def phaseName = "restoreScopes" - override def changesMembers = true + override def changesMembers = true // the phase affects scopes, applying tree transformations of previous phases /* Note: We need to wait until we see a package definition because * DropEmptyConstructors changes template members when analyzing the From ed2e6763245280145138c303e69700da7c8debd4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 14 May 2017 11:34:08 +0200 Subject: [PATCH 20/22] Revert: Instrumentation to get basedata computation counts (reverted from commit c1254ed36287d461450d4345b54a2e3f1a853523) Last data over whole legacy test suite (running time: 337s): BaseData cache fills per phase: 0, 670675, 0, 0, 2648, 0, 12, 0, 2, 548, 1153, 1189, 190, 115, 0, 0, 0, 0, 28, 68, 0, 0, 129, 267, 0, 0, 0, 0, 0, 4, 0, 38, 0, 3, 13, 1, 6857, 809, 167147, 59961, 671, 246, 5417, 169, 114, 615, 17, 0, 2, 0, 1753, 0, 27, 0, 0, 8032, 26338, 0, 0, 736, 0, 0, 0, 4851, 69224, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 Cache invalidations: 80 --- compiler/src/dotty/tools/dotc/Run.scala | 2 -- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 7 +------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index a3de94454210..acd885dd3afe 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -92,8 +92,6 @@ class Run(comp: Compiler)(implicit ctx: Context) { for (unit <- units) Stats.record("retained typed trees at end", unit.tpdTree.treeSize) Stats.record("total trees at end", ast.Trees.ntrees) - println(s"bts counts: ${SymDenotations.btsCount.deep}") - println(s"invalidate count: ${SymDenotations.invalidateCount}") } private sealed trait PrintedTree diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 6f5eb2ca0e32..058c16f6297f 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1421,7 +1421,6 @@ object SymDenotations { } def emptyParentsExpected = is(Package) || (symbol == defn.AnyClass) || ctx.erasedTypes && (symbol == defn.ObjectClass) - btsCount(ctx.phaseId) += 1 if (classParents.isEmpty && !emptyParentsExpected) onBehalf.signalProvisional() (classSymbol :: addParentBaseClasses(classParents, Nil), @@ -2041,8 +2040,7 @@ object SymDenotations { final def isValid(implicit ctx: Context): Boolean = valid && isValidAt(ctx.phase) def invalidate(): Unit = - if (valid && !locked) { - invalidateCount += 1 + if (isValid && !locked) { cache = null valid = false invalidateDependents() @@ -2089,7 +2087,4 @@ object SymDenotations { } @sharable private var indent = 0 // for completions printing - - @sharable val btsCount = new Array[Int](Periods.MaxPossiblePhaseId + 1) - @sharable var invalidateCount = 0 } From 04bc040c8d6b407ba0d5ae3dd7380b2a7626e041 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 15 May 2017 15:58:20 +0200 Subject: [PATCH 21/22] Fix reviewers comments --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 058c16f6297f..70b35f68151a 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2040,7 +2040,7 @@ object SymDenotations { final def isValid(implicit ctx: Context): Boolean = valid && isValidAt(ctx.phase) def invalidate(): Unit = - if (isValid && !locked) { + if (valid && !locked) { cache = null valid = false invalidateDependents() From effaca17f465032e39f068ab069a5f6313e7c0a8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 15 May 2017 16:20:44 +0200 Subject: [PATCH 22/22] Address reviewers comments --- compiler/src/dotty/tools/dotc/core/Phases.scala | 15 +++++++++------ .../dotty/tools/dotc/core/SymDenotations.scala | 6 +++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Phases.scala b/compiler/src/dotty/tools/dotc/core/Phases.scala index 841fd9013227..bf33471cc98f 100644 --- a/compiler/src/dotty/tools/dotc/core/Phases.scala +++ b/compiler/src/dotty/tools/dotc/core/Phases.scala @@ -321,8 +321,8 @@ object Phases { private var mySymbolicRefs = false private var myLabelsReordered = false - private var myMembersGroup = 0 - private var myParentsGroup = 0 + private var mySameMembersStartId = NoPhaseId + private var mySameParentsStartId = NoPhaseId /** The sequence position of this phase in the given context where 0 * is reserved for NoPhase and the first real phase is at position 1. @@ -339,8 +339,11 @@ object Phases { final def refChecked = myRefChecked // Phase is after RefChecks final def symbolicRefs = mySymbolicRefs // Phase is after ResolveSuper, newly generated TermRefs should be symbolic final def labelsReordered = myLabelsReordered // Phase is after LabelDefs, labels are flattened and owner chains don't mirror this - final def membersGroup = myMembersGroup // group id for phases where all symbols have the same non-private members - final def parentsGroup = myParentsGroup // group id for phases where all symbols have the same base classes + + final def sameMembersStartId = mySameMembersStartId + // id of first phase where all symbols are guaranteed to have the same members as in this phase + final def sameParentsStartId = mySameParentsStartId + // id of first phase where all symbols are guaranteed to have the same parents as in this phase protected[Phases] def init(base: ContextBase, start: Int, end:Int): Unit = { if (start >= FirstPhaseId) @@ -352,8 +355,8 @@ object Phases { myRefChecked = prev.getClass == classOf[RefChecks] || prev.refChecked mySymbolicRefs = prev.getClass == classOf[ResolveSuper] || prev.symbolicRefs myLabelsReordered = prev.getClass == classOf[LabelDefs] || prev.labelsReordered - myMembersGroup = if (changesMembers) prev.membersGroup + 1 else prev.membersGroup - myParentsGroup = if (changesParents) prev.parentsGroup + 1 else prev.parentsGroup + mySameMembersStartId = if (changesMembers) id else prev.sameMembersStartId + mySameParentsStartId = if (changesParents) id else prev.sameMembersStartId } protected[Phases] def init(base: ContextBase, id: Int): Unit = init(base, id, id) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 70b35f68151a..8b2c34804e85 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1257,7 +1257,7 @@ object SymDenotations { private def baseTypeRefCache(implicit ctx: Context): BaseTypeRefMap = { if (myBaseTypeRefCachePeriod != ctx.period && (myBaseTypeRefCachePeriod.runId != ctx.runId || - ctx.phases(myBaseTypeRefCachePeriod.phaseId).parentsGroup != ctx.phase.parentsGroup)) { + ctx.phases(myBaseTypeRefCachePeriod.phaseId).sameParentsStartId != ctx.phase.sameParentsStartId)) { myBaseTypeRefCache = new BaseTypeRefMap myBaseTypeRefCachePeriod = ctx.period } @@ -2027,7 +2027,7 @@ object SymDenotations { finally addDependent(onBehalf) } - def sameGroup(p1: Phase, p2: Phase) = p1.membersGroup == p2.membersGroup + def sameGroup(p1: Phase, p2: Phase) = p1.sameMembersStartId == p2.sameMembersStartId } private class BaseDataImpl(createdAt: Period) extends InheritedCacheImpl(createdAt) with BaseData { @@ -2068,7 +2068,7 @@ object SymDenotations { finally addDependent(onBehalf) } - def sameGroup(p1: Phase, p2: Phase) = p1.parentsGroup == p2.parentsGroup + def sameGroup(p1: Phase, p2: Phase) = p1.sameParentsStartId == p2.sameParentsStartId } class BaseClassSet(val classIds: Array[Int]) extends AnyVal {