Skip to content

Micro-optimization: Symbol#denot #3263

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.

Sign up for GitHub

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 2 commits into from
Oct 6, 2017
Merged

Micro-optimization: Symbol#denot #3263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f2cdce8
Micro-optimize Symbol#denot
odersky Oct 5, 2017
d0c377b
Further optimizations of denot
odersky Oct 5, 2017
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
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Denotations.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,8 +668,10 @@ object Denotations {
private[this] var myValidFor: Period = Nowhere

def validFor = myValidFor
def validFor_=(p: Period) =
def validFor_=(p: Period) = {
myValidFor = p
symbol.invalidateDenotCache()
}

/** The next SingleDenotation in this run, with wrap-around from last to first.
*
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1851,7 +1851,7 @@ object SymDenotations {
override def owner: Symbol = throw new AssertionError("NoDenotation.owner")
override def computeAsSeenFrom(pre: Type)(implicit ctx: Context): SingleDenotation = this
override def mapInfo(f: Type => Type)(implicit ctx: Context): SingleDenotation = this
validFor = Period.allInRun(NoRunId) // will be brought forward automatically
validFor = Period.allInRun(NoRunId)
}

@sharable val NoDenotation = new NoDenotation
Expand Down
31 changes: 23 additions & 8 deletions compiler/src/dotty/tools/dotc/core/Symbols.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,19 +398,34 @@ object Symbols {

/** The last denotation of this symbol */
private[this] var lastDenot: SymDenotation = _
private[this] var checkedPeriod: Period = Nowhere

private[core] def invalidateDenotCache() = { checkedPeriod = Nowhere }

/** Set the denotation of this symbol */
private[core] def denot_=(d: SymDenotation) =
private[core] def denot_=(d: SymDenotation) = {
lastDenot = d
checkedPeriod = Nowhere
}

/** The current denotation of this symbol */
final def denot(implicit ctx: Context): SymDenotation = {
var denot = lastDenot
if (!(denot.validFor contains ctx.period)) {
denot = denot.current.asInstanceOf[SymDenotation]
lastDenot = denot
}
denot
val lastd = lastDenot
if (checkedPeriod == ctx.period) lastd
else computeDenot(lastd)
}

private def computeDenot(lastd: SymDenotation)(implicit ctx: Context): SymDenotation = {
val now = ctx.period
checkedPeriod = now
if (lastd.validFor contains now) lastd else recomputeDenot(lastd)
}

/** Overridden in NoSymbol */
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it'll help performance, but have you tried doing if (this eq NoSymbol) NoDenotation else ... to avoid the override?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not tired it. I don't think we'd see a difference either way. Would be good to know more about tradeoffs like this, though.

protected def recomputeDenot(lastd: SymDenotation)(implicit ctx: Context) = {
val newd = lastd.current.asInstanceOf[SymDenotation]
lastDenot = newd
newd
}

/** The initial denotation of this symbol, without going through `current` */
Expand Down Expand Up @@ -631,8 +646,8 @@ object Symbols {

@sharable object NoSymbol extends Symbol(NoCoord, 0) {
denot = NoDenotation

override def associatedFile(implicit ctx: Context): AbstractFile = NoSource.file
override def recomputeDenot(lastd: SymDenotation)(implicit ctx: Context): SymDenotation = NoDenotation
}

implicit class Copier[N <: Name](sym: Symbol { type ThisName = N })(implicit ctx: Context) {
Expand Down