Skip to content

Some more optimizations #3217

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 3 commits into from
Sep 30, 2017
Merged

Some more optimizations #3217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e73508e
Some polishings
odersky Sep 30, 2017
ed5eb24
Avoid useless computations in LinkAll
odersky Sep 30, 2017
f27cd43
More extensive stats
odersky Sep 30, 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: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,8 @@ object Trees {
abstract class TreeMap(val cpy: TreeCopier = inst.cpy) {

def transform(tree: Tree)(implicit ctx: Context): Tree = {
Stats.record(s"TreeMap.transform $getClass")
Stats.record("TreeMap.transform total")
def localCtx =
if (tree.hasType && tree.symbol.exists) ctx.withOwner(tree.symbol) else ctx

Expand Down Expand Up @@ -1228,6 +1230,8 @@ object Trees {

def apply(x: X, trees: Traversable[Tree])(implicit ctx: Context): X = (x /: trees)(apply)
def foldOver(x: X, tree: Tree)(implicit ctx: Context): X = {
Stats.record(s"TreeAccumulator.foldOver $getClass")
Stats.record("TreeAccumulator.foldOver total")
def localCtx =
if (tree.hasType && tree.symbol.exists) ctx.withOwner(tree.symbol) else ctx
tree match {
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/dotty/tools/dotc/core/TypeErasure.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,6 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
// constructor method should not be semi-erased.
else if (isConstructor && isDerivedValueClass(sym)) eraseNormalClassRef(tp)
else this(tp)
case RefinedType(parent, _, _) if !(parent isRef defn.ArrayClass) => // @!!!
eraseResult(parent)
case AppliedType(tycon, _) if !(tycon isRef defn.ArrayClass) =>
eraseResult(tycon)
case _ =>
Expand Down
11 changes: 8 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Types.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ object Types {
/** The full parent types, including all type arguments */
def parents(implicit ctx: Context): List[Type] = this match {
case tp @ AppliedType(tycon, args) if tycon.typeSymbol.isClass =>
tycon.parents.map(_.subst(tycon.typeSymbol.typeParams, args)) // @!!! cache?
tycon.parents.map(_.subst(tycon.typeSymbol.typeParams, args))
case tp: TypeRef =>
if (tp.info.isInstanceOf[TempClassInfo]) {
tp.reloadDenot()
Expand Down Expand Up @@ -3720,6 +3720,8 @@ object Types {

/** Map this function over given type */
def mapOver(tp: Type): Type = {
record(s"mapOver ${getClass}")
record("mapOver total")
implicit val ctx = this.ctx
tp match {
case tp: NamedType =>
Expand Down Expand Up @@ -4095,7 +4097,10 @@ object Types {
protected final def applyToPrefix(x: T, tp: NamedType) =
atVariance(variance max 0)(this(x, tp.prefix)) // see remark on NamedType case in TypeMap

def foldOver(x: T, tp: Type): T = tp match {
def foldOver(x: T, tp: Type): T = {
record(s"foldOver $getClass")
record(s"foldOver total")
tp match {
case tp: TypeRef =>
if (stopAtStatic && tp.symbol.isStatic) x
else {
Expand Down Expand Up @@ -4184,7 +4189,7 @@ object Types {
tp.fold(x, this)

case _ => x
}
}}

@tailrec final def foldOver(x: T, ts: List[Type]): T = ts match {
case t :: ts1 => foldOver(apply(x, t), ts1)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/LinkAll.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class LinkAll extends MiniPhaseTransform {
}

if (ctx.settings.XlinkOptimise.value) super.runOn(allUnits(Set.empty, units.toSet, Set.empty))
Copy link
Contributor

Choose a reason for hiding this comment

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

This came from the design that @DarkDimius showed me to add new compilation units. I mostly looked at it from a functional point of view, not a performance one. I will have a look and see if it can be done in a better way.

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 see. In essence, MiniPhase.runOn calls the indexing framework that figures out the method call plan. I thought that would be done before, but it seems not (maybe because we don't have the right context before?). The problem with that approach is that it is done afresh for each compilation unit, even if it
is always the same computation.

Nobody except LinkAll seems to call runOn, however.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably cleaner #3219, there was not a good reason for it to be a miniphase. That PR should further improve performance on LinkAll.

Copy link
Contributor

Choose a reason for hiding this comment

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

If I remember correctly runOn was designed specifically for the purpose of adding new compilation units. Until LinkAll this use case was only used in the linker fork.

else super.runOn(units)
else units
}

/** Collects all class denotations that may need to be loaded. */
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/transform/TreeTransform.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,7 @@ object TreeTransforms {
def transform(tree: Tree, info: TransformerInfo, cur: Int)(implicit ctx: Context): Tree = ctx.traceIndented(s"transforming ${tree.show} at ${ctx.phase}", transforms, show = true) {
try
if (cur < info.transformers.length) {
util.Stats.record("TreeTransform.transform")
// if cur > 0 then some of the symbols can be created by already performed transformations
// this means that their denotations could not exists in previous period
val pctx = ctx.withPhase(info.transformers(cur).treeTransformPhase)
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
}

def typed(tree: untpd.Tree, pt: Type = WildcardType)(implicit ctx: Context): Tree = /*>|>*/ ctx.traceIndented (i"typing $tree", typr, show = true) /*<|<*/ {
record(s"typed $getClass")
record("typed total")
assertPositioned(tree)
try adapt(typedUnadapted(tree, pt), pt)
catch {
Expand Down