Skip to content

Fix caches of info that depends on inherited classes #2425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
May 29, 2017
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
23 changes: 0 additions & 23 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/core/DenotTransformers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
11 changes: 4 additions & 7 deletions compiler/src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,6 @@ object Flags {
/** A bridge method. Set by Erasure */
final val Bridge = termFlag(34, "<bridge>")

/** All class attributes are fully defined */
final val FullyCompleted = typeFlag(34, "<fully-completed>")

/** Symbol is a Java varargs bridge */ // (needed?)
final val VBridge = termFlag(35, "<vbridge>") // TODO remove

Expand All @@ -375,9 +372,6 @@ object Flags {
/** Denotation is in train of being loaded and completed, used to catch cyclic dependencies */
final val Touched = commonFlag(48, "<touched>")

/** Class is not allowed to accept new members because fingerprint of subclass has been taken */
final val Frozen = commonFlag(49, "<frozen>")

/** An error symbol */
final val Erroneous = commonFlag(50, "<is-error>")

Expand Down Expand Up @@ -450,7 +444,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
Expand Down Expand Up @@ -558,6 +552,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

Expand Down
12 changes: 12 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Copy link
Contributor

@DarkDimius DarkDimius May 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either the doc & implementation are out of sync~, or this is a bug.

The implementation counts how many times where parents changed, it does not use the actual phase-ids.
I've had a look through usages, this value seems to only be used for equality comparisons, so the "number of times members could have changed" would behave correctly, but I'd still propose to make it return a PhaseID to ease understanding.

Same with parentsGroup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll change to phase ids, and use a more descriptive name.

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)
Expand All @@ -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)
Expand Down
Loading