Skip to content

Various small optimizations #9605

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 8 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 5 additions & 5 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1379,11 +1379,11 @@ object Trees {
// Ties the knot of the traversal: call `foldOver(x, tree))` to dive in the `tree` node.
def apply(x: X, tree: Tree)(using Context): X

def apply(x: X, trees: List[Tree])(using Context): X = trees match
case tree :: rest =>
apply(apply(x, tree), rest)
case Nil =>
x
def apply(x: X, trees: List[Tree])(using Context): X =
def fold(x: X, trees: List[Tree]): X = trees match
case tree :: rest => fold(apply(x, tree), rest)
case Nil => x
fold(x, trees)
Copy link
Contributor

Choose a reason for hiding this comment

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

In ee4b125, I'm trying:

        var acc = x
        var list = trees
        while (!list.isEmpty) do
          acc = apply(acc, list.head)
          list = list.tail
        acc

After tail-call optimization, the two versions are almost the same. Let's see if there is difference in benchmarks.

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 don't think there will be. If anything, the tail recursive version should be faster since it does a single type test per iteration instead of one each in isEmpty, head, and tail.

Copy link
Contributor

Choose a reason for hiding this comment

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

In #9565, we get a slight speedup for Dotty.


def foldOver(x: X, tree: Tree)(using Context): X =
if (tree.source != ctx.source && tree.source.exists)
Expand Down
18 changes: 6 additions & 12 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ object Contexts {
private val (profilerLoc, store7) = store6.newLocation[Profiler]()
private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]()
private val (importInfoLoc, store9) = store8.newLocation[ImportInfo]()
private val (typeAssignerLoc, store10) = store9.newLocation[TypeAssigner](TypeAssigner)

private val initialStore = store9
private val initialStore = store10

/** The current context */
inline def ctx(using ctx: Context): Context = ctx
Expand Down Expand Up @@ -157,11 +158,6 @@ object Contexts {
protected def typerState_=(typerState: TyperState): Unit = _typerState = typerState
final def typerState: TyperState = _typerState

/** The current type assigner or typer */
private var _typeAssigner: TypeAssigner = _
protected def typeAssigner_=(typeAssigner: TypeAssigner): Unit = _typeAssigner = typeAssigner
final def typeAssigner: TypeAssigner = _typeAssigner

/** The current bounds in force for type parameters appearing in a GADT */
private var _gadt: GadtConstraint = _
protected def gadt_=(gadt: GadtConstraint): Unit = _gadt = gadt
Expand Down Expand Up @@ -228,6 +224,9 @@ object Contexts {
/** The currently active import info */
def importInfo = store(importInfoLoc)

/** The current type assigner or typer */
def typeAssigner: TypeAssigner = store(typeAssignerLoc)

/** The new implicit references that are introduced by this scope */
protected var implicitsCache: ContextualImplicits = null
def implicits: ContextualImplicits = {
Expand Down Expand Up @@ -483,7 +482,6 @@ object Contexts {
_owner = origin.owner
_tree = origin.tree
_scope = origin.scope
_typeAssigner = origin.typeAssigner
_gadt = origin.gadt
_searchHistory = origin.searchHistory
_source = origin.source
Expand Down Expand Up @@ -574,10 +572,6 @@ object Contexts {
def setNewTyperState(): this.type = setTyperState(typerState.fresh().setCommittable(true))
def setExploreTyperState(): this.type = setTyperState(typerState.fresh().setCommittable(false))
def setReporter(reporter: Reporter): this.type = setTyperState(typerState.fresh().setReporter(reporter))
def setTypeAssigner(typeAssigner: TypeAssigner): this.type =
util.Stats.record("Context.setTypeAssigner")
this.typeAssigner = typeAssigner
this
def setTyper(typer: Typer): this.type = { this.scope = typer.scope; setTypeAssigner(typer) }
def setGadt(gadt: GadtConstraint): this.type =
util.Stats.record("Context.setGadt")
Expand Down Expand Up @@ -615,6 +609,7 @@ object Contexts {
def setProfiler(profiler: Profiler): this.type = updateStore(profilerLoc, profiler)
def setNotNullInfos(notNullInfos: List[NotNullInfo]): this.type = updateStore(notNullInfosLoc, notNullInfos)
def setImportInfo(importInfo: ImportInfo): this.type = updateStore(importInfoLoc, importInfo)
def setTypeAssigner(typeAssigner: TypeAssigner): this.type = updateStore(typeAssignerLoc, typeAssigner)

def setProperty[T](key: Key[T], value: T): this.type =
setMoreProperties(moreProperties.updated(key, value))
Expand Down Expand Up @@ -742,7 +737,6 @@ object Contexts {
typerState = TyperState.initialState()
owner = NoSymbol
tree = untpd.EmptyTree
typeAssigner = TypeAssigner
moreProperties = Map(MessageLimiter -> DefaultMessageLimiter())
source = NoSource
store = initialStore
Expand Down
14 changes: 9 additions & 5 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1050,11 +1050,15 @@ class Definitions {
def scalaClassName(ref: Type)(using Context): TypeName = scalaClassName(ref.classSymbol)

private def isVarArityClass(cls: Symbol, prefix: String) =
cls.isClass && cls.owner.eq(ScalaPackageClass) &&
cls.name.testSimple(name =>
name.startsWith(prefix) &&
name.length > prefix.length &&
name.drop(prefix.length).forall(_.isDigit))
cls.isClass
&& cls.owner.eq(ScalaPackageClass)
&& cls.name.testSimple(name =>
name.startsWith(prefix)
&& name.length > prefix.length
&& digitsOnlyAfter(name, prefix.length))

private def digitsOnlyAfter(name: SimpleName, idx: Int): Boolean =
idx == name.length || name(idx).isDigit && digitsOnlyAfter(name, idx + 1)

def isBottomClass(cls: Symbol): Boolean =
if (ctx.explicitNulls && !ctx.phase.erasedTypes) cls == NothingClass
Expand Down
8 changes: 5 additions & 3 deletions compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -567,11 +567,13 @@ class OrderingConstraint(private val boundsMap: ParamBounds,

def foreachTypeVar(op: TypeVar => Unit): Unit =
boundsMap.foreachBinding { (poly, entries) =>
for (i <- 0 until paramCount(entries))
typeVar(entries, i) match {
var i = 0
val limit = paramCount(entries)
while i < limit do
typeVar(entries, i) match
case tv: TypeVar if !tv.inst.exists => op(tv)
case _ =>
}
i += 1
}

private var myUninstVars: mutable.ArrayBuffer[TypeVar] = _
Expand Down
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Scopes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ object Scopes {
def next(): Symbol = { val r = e.sym; e = lookupNextEntry(e); r }
}

/** Does this scope contain a reference to `sym` when looking up `name`? */
final def contains(name: Name, sym: Symbol)(using Context): Boolean =
var e = lookupEntry(name)
while e != null && e.sym != sym do e = lookupNextEntry(e)
e != null

/** The denotation set of all the symbols with given name in this scope
* Symbols occur in the result in reverse order relative to their occurrence
* in `this.toList`.
Expand Down
12 changes: 5 additions & 7 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2361,19 +2361,17 @@ object SymDenotations {
stillValidInOwner(denot)
}

private[SymDenotations] def stillValidInOwner(denot: SymDenotation)(using Context): Boolean = try {
private[SymDenotations] def stillValidInOwner(denot: SymDenotation)(using Context): Boolean = try
val owner = denot.owner.denot
stillValid(owner) && (
stillValid(owner)
&& (
!owner.isClass
|| owner.isRefinementClass
|| owner.is(Scala2x)
|| (owner.unforcedDecls.lookupAll(denot.name) contains denot.symbol)
|| owner.unforcedDecls.contains(denot.name, denot.symbol)
|| denot.isSelfSym
|| denot.isLocalDummy)
}
catch {
case ex: StaleSymbol => false
}
catch case ex: StaleSymbol => false

/** Explain why symbol is invalid; used for debugging only */
def traceInvalid(denot: Denotation)(using Context): Boolean = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Instrumentation extends MiniPhase { thisPhase =>
ctx.settings.YinstrumentAllocations.value

private val namesOfInterest = List(
"::", "+=", "toString", "newArray", "box",
"::", "+=", "toString", "newArray", "box", "toCharArray",
"map", "flatMap", "filter", "withFilter", "collect", "foldLeft", "foldRight", "take",
"reverse", "mapConserve", "mapconserve", "filterConserve", "zip")
private var namesToRecord: Set[Name] = _
Expand Down
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2596,7 +2596,7 @@ class Typer extends Namer

def typedStats(stats: List[untpd.Tree], exprOwner: Symbol)(using Context): (List[Tree], Context) = {
val buf = new mutable.ListBuffer[Tree]
val enumContexts = new mutable.HashMap[Symbol, Context]
var enumContexts: SimpleIdentityMap[Symbol, Context] = SimpleIdentityMap.Empty
val initialNotNullInfos = ctx.notNullInfos
// A map from `enum` symbols to the contexts enclosing their definitions
@tailrec def traverse(stats: List[untpd.Tree])(using Context): (List[Tree], Context) = stats match {
Expand All @@ -2618,7 +2618,7 @@ class Typer extends Namer
// replace body with expansion, because it will be used as inlined body
// from separately compiled files - the original BodyAnnotation is not kept.
case mdef1: TypeDef if mdef1.symbol.is(Enum, butNot = Case) =>
enumContexts(mdef1.symbol) = ctx
enumContexts = enumContexts.updated(mdef1.symbol, ctx)
buf += mdef1
case EmptyTree =>
// clashing synthetic case methods are converted to empty trees, drop them here
Expand Down Expand Up @@ -2650,7 +2650,8 @@ class Typer extends Namer
}
def finalize(stat: Tree)(using Context): Tree = stat match {
case stat: TypeDef if stat.symbol.is(Module) =>
for (enumContext <- enumContexts.get(stat.symbol.linkedClass))
val enumContext = enumContexts(stat.symbol.linkedClass)
if enumContext != null then
checkEnumCaseRefsLegal(stat, enumContext)
stat.removeAttachment(Deriver) match {
case Some(deriver) => deriver.finalize(stat)
Expand Down
18 changes: 9 additions & 9 deletions compiler/src/dotty/tools/dotc/util/SourceFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,18 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends
def positionInUltimateSource(position: SourcePosition): SourcePosition =
SourcePosition(underlying, position.span shift start)

private def isLineBreak(idx: Int) =
if (idx >= length) false else {
val ch = content()(idx)
// don't identify the CR in CR LF as a line break, since LF will do.
if (ch == CR) (idx + 1 == length) || (content()(idx + 1) != LF)
else isLineBreakChar(ch)
}

private def calculateLineIndices(cs: Array[Char]) = {
val buf = new ArrayBuffer[Int]
buf += 0
for (i <- 0 until cs.length) if (isLineBreak(i)) buf += i + 1
var i = 0
while i < cs.length do
val isLineBreak =
val ch = cs(i)
// don't identify the CR in CR LF as a line break, since LF will do.
if ch == CR then i + 1 == cs.length || cs(i + 1) != LF
else isLineBreakChar(ch)
if isLineBreak then buf += i + 1
i += 1
buf += cs.length // sentinel, so that findLine below works smoother
buf.toArray
}
Expand Down