Skip to content

Optimize interpolateTypeVars #6331

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 7 commits into from
Apr 19, 2019
Merged

Optimize interpolateTypeVars #6331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7fe7954
Optimize interpolateTypeVars
odersky Apr 17, 2019
35e6318
Another optimization
odersky Apr 17, 2019
f72522c
Avoid constructing intermediate sets in interpolateTypeVars
odersky Apr 18, 2019
85f79cb
Optimize SimpleIdentitySet some more
odersky Apr 18, 2019
ad2776c
More optimizations
odersky Apr 18, 2019
792714b
Drop concat as separate method in SimpleIdentitySet
odersky Apr 19, 2019
4819f27
Address review comments
odersky Apr 19, 2019
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
103 changes: 53 additions & 50 deletions compiler/src/dotty/tools/dotc/typer/Inferencing.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,57 +410,60 @@ trait Inferencing { this: Typer =>
// anymore if they've been garbage-collected, so we can't use
// `state.ownedVars.size > locked.size` as an early check to avoid computing
// `qualifying`.
val qualifying = state.ownedVars -- locked

if (!qualifying.isEmpty) {
typr.println(i"interpolate $tree: ${tree.tpe.widen} in $state, owned vars = ${state.ownedVars.toList}%, %, previous = ${locked.toList}%, % / ${state.constraint}")
val resultAlreadyConstrained =
tree.isInstanceOf[Apply] || tree.tpe.isInstanceOf[MethodOrPoly]
if (!resultAlreadyConstrained)
constrainResult(tree.symbol, tree.tpe, pt)
// This is needed because it could establish singleton type upper bounds. See i2998.scala.

val tp = tree.tpe.widen
val vs = variances(tp)

// Avoid interpolating variables occurring in tree's type if typerstate has unreported errors.
// Reason: The errors might reflect unsatisfiable constraints. In that
// case interpolating without taking account the constraints risks producing
// nonsensical types that then in turn produce incomprehensible errors.
// An example is in neg/i1240.scala. Without the condition in the next code line
// we get for
//
// val y: List[List[String]] = List(List(1))
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: Nothing
// val y: List[List[String]] = List(List(1))
// ^
// With the condition, we get the much more sensical:
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: String
// val y: List[List[String]] = List(List(1))
val hasUnreportedErrors = state.reporter.hasUnreportedErrors
def constraint = state.constraint
for (tvar <- qualifying)
if (!tvar.isInstantiated && state.constraint.contains(tvar)) {
// Needs to be checked again, since previous interpolations could already have
// instantiated `tvar` through unification.
val v = vs(tvar)
if (v == null) {
typr.println(i"interpolate non-occurring $tvar in $state in $tree: $tp, fromBelow = ${tvar.hasLowerBound}, $constraint")
tvar.instantiate(fromBelow = tvar.hasLowerBound)
}
else if (!hasUnreportedErrors)
if (v.intValue != 0) {
typr.println(i"interpolate $tvar in $state in $tree: $tp, fromBelow = ${v.intValue == 1}, $constraint")
tvar.instantiate(fromBelow = v.intValue == 1)

val ownedVars = state.ownedVars
if ((ownedVars ne locked) && !ownedVars.isEmpty) {
val qualifying = ownedVars -- locked
if (!qualifying.isEmpty) {
typr.println(i"interpolate $tree: ${tree.tpe.widen} in $state, owned vars = ${state.ownedVars.toList}%, %, previous = ${locked.toList}%, % / ${state.constraint}")
val resultAlreadyConstrained =
tree.isInstanceOf[Apply] || tree.tpe.isInstanceOf[MethodOrPoly]
if (!resultAlreadyConstrained)
constrainResult(tree.symbol, tree.tpe, pt)
// This is needed because it could establish singleton type upper bounds. See i2998.scala.

val tp = tree.tpe.widen
val vs = variances(tp)

// Avoid interpolating variables occurring in tree's type if typerstate has unreported errors.
// Reason: The errors might reflect unsatisfiable constraints. In that
// case interpolating without taking account the constraints risks producing
// nonsensical types that then in turn produce incomprehensible errors.
// An example is in neg/i1240.scala. Without the condition in the next code line
// we get for
//
// val y: List[List[String]] = List(List(1))
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: Nothing
// val y: List[List[String]] = List(List(1))
// ^
// With the condition, we get the much more sensical:
//
// i1430.scala:5: error: type mismatch:
// found : Int(1)
// required: String
// val y: List[List[String]] = List(List(1))
val hasUnreportedErrors = state.reporter.hasUnreportedErrors
def constraint = state.constraint
for (tvar <- qualifying)
if (!tvar.isInstantiated && state.constraint.contains(tvar)) {
// Needs to be checked again, since previous interpolations could already have
// instantiated `tvar` through unification.
val v = vs(tvar)
if (v == null) {
typr.println(i"interpolate non-occurring $tvar in $state in $tree: $tp, fromBelow = ${tvar.hasLowerBound}, $constraint")
tvar.instantiate(fromBelow = tvar.hasLowerBound)
}
else typr.println(i"no interpolation for nonvariant $tvar in $state")
}
else if (!hasUnreportedErrors)
if (v.intValue != 0) {
typr.println(i"interpolate $tvar in $state in $tree: $tp, fromBelow = ${v.intValue == 1}, $constraint")
tvar.instantiate(fromBelow = v.intValue == 1)
}
else typr.println(i"no interpolation for nonvariant $tvar in $state")
}
}
}
tree
}
Expand Down
118 changes: 107 additions & 11 deletions compiler/src/dotty/tools/dotc/util/SimpleIdentitySet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,30 @@
package dotty.tools.dotc.util

import collection.mutable.ListBuffer
import collection.mutable

/** A simple linked set with `eq` as the comparison, optimized for small sets.
* It has linear complexity for `contains`, `+`, and `-`.
*/
abstract class SimpleIdentitySet[+Elem <: AnyRef] {
def size: Int
def isEmpty: Boolean = size == 0
final def isEmpty: Boolean = size == 0
def + [E >: Elem <: AnyRef](x: E): SimpleIdentitySet[E]
def - [E >: Elem <: AnyRef](x: E): SimpleIdentitySet[Elem]
def contains[E >: Elem <: AnyRef](x: E): Boolean
def foreach(f: Elem => Unit): Unit
def toList: List[Elem] = {
val buf = new ListBuffer[Elem]
foreach(buf += _)
buf.toList
}
def exists[E >: Elem <: AnyRef](p: E => Boolean): Boolean
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A
def toList: List[Elem]
def ++ [E >: Elem <: AnyRef](that: SimpleIdentitySet[E]): SimpleIdentitySet[E] =
((this: SimpleIdentitySet[E]) /: that.toList)(_ + _)
def -- [E >: Elem <: AnyRef](that: SimpleIdentitySet[E]): SimpleIdentitySet[Elem] =
(this /: that.toList)(_ - _)
if (this.size == 0) that
else if (that.size == 0) this
else ((this: SimpleIdentitySet[E]) /: that)(_ + _)
def -- [E >: Elem <: AnyRef](that: SimpleIdentitySet[E]): SimpleIdentitySet[E] =
if (that.size == 0) this
else
((SimpleIdentitySet.empty: SimpleIdentitySet[E]) /: this) { (s, x) =>
if (that.contains(x)) s else s + x
}
override def toString: String = toList.mkString("(", ", ", ")")
}

Expand All @@ -33,6 +37,9 @@ object SimpleIdentitySet {
this
def contains[E <: AnyRef](x: E): Boolean = false
def foreach(f: Nothing => Unit): Unit = ()
def exists[E <: AnyRef](p: E => Boolean): Boolean = false
def /: [A, E <: AnyRef](z: A)(f: (A, E) => A): A = z
def toList = Nil
}

private class Set1[+Elem <: AnyRef](x0: AnyRef) extends SimpleIdentitySet[Elem] {
Expand All @@ -43,6 +50,11 @@ object SimpleIdentitySet {
if (x `eq` x0) empty else this
def contains[E >: Elem <: AnyRef](x: E): Boolean = x `eq` x0
def foreach(f: Elem => Unit): Unit = f(x0.asInstanceOf[Elem])
def exists[E >: Elem <: AnyRef](p: E => Boolean): Boolean =
p(x0.asInstanceOf[E])
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
f(z, x0.asInstanceOf[E])
def toList = x0.asInstanceOf[Elem] :: Nil
}

private class Set2[+Elem <: AnyRef](x0: AnyRef, x1: AnyRef) extends SimpleIdentitySet[Elem] {
Expand All @@ -55,6 +67,11 @@ object SimpleIdentitySet {
else this
def contains[E >: Elem <: AnyRef](x: E): Boolean = (x `eq` x0) || (x `eq` x1)
def foreach(f: Elem => Unit): Unit = { f(x0.asInstanceOf[Elem]); f(x1.asInstanceOf[Elem]) }
def exists[E >: Elem <: AnyRef](p: E => Boolean): Boolean =
p(x0.asInstanceOf[E]) || p(x1.asInstanceOf[E])
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
f(f(z, x0.asInstanceOf[E]), x1.asInstanceOf[E])
def toList = x0.asInstanceOf[Elem] :: x1.asInstanceOf[Elem] :: Nil
}

private class Set3[+Elem <: AnyRef](x0: AnyRef, x1: AnyRef, x2: AnyRef) extends SimpleIdentitySet[Elem] {
Expand All @@ -78,9 +95,14 @@ object SimpleIdentitySet {
def foreach(f: Elem => Unit): Unit = {
f(x0.asInstanceOf[Elem]); f(x1.asInstanceOf[Elem]); f(x2.asInstanceOf[Elem])
}
def exists[E >: Elem <: AnyRef](p: E => Boolean): Boolean =
p(x0.asInstanceOf[E]) || p(x1.asInstanceOf[E]) || p(x2.asInstanceOf[E])
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
f(f(f(z, x0.asInstanceOf[E]), x1.asInstanceOf[E]), x2.asInstanceOf[E])
def toList = x0.asInstanceOf[Elem] :: x1.asInstanceOf[Elem] :: x2.asInstanceOf[Elem] :: Nil
}

private class SetN[+Elem <: AnyRef](xs: Array[AnyRef]) extends SimpleIdentitySet[Elem] {
private class SetN[+Elem <: AnyRef](val xs: Array[AnyRef]) extends SimpleIdentitySet[Elem] {
def size = xs.length
def + [E >: Elem <: AnyRef](x: E): SimpleIdentitySet[E] =
if (contains(x)) this
Expand Down Expand Up @@ -115,5 +137,79 @@ object SimpleIdentitySet {
var i = 0
while (i < size) { f(xs(i).asInstanceOf[Elem]); i += 1 }
}
def exists[E >: Elem <: AnyRef](p: E => Boolean): Boolean =
xs.asInstanceOf[Array[E]].exists(p)
def /: [A, E >: Elem <: AnyRef](z: A)(f: (A, E) => A): A =
(z /: xs.asInstanceOf[Array[E]])(f)
def toList: List[Elem] = {
val buf = new mutable.ListBuffer[Elem]
foreach(buf += _)
buf.toList
}
override def ++ [E >: Elem <: AnyRef](that: SimpleIdentitySet[E]): SimpleIdentitySet[E] =
that match {
case that: SetN[_] =>
var toAdd: mutable.ArrayBuffer[AnyRef] = null
var i = 0
val limit = that.xs.length
while (i < limit) {
val elem = that.xs(i)
if (!contains(elem)) {
if (toAdd == null) toAdd = new mutable.ArrayBuffer
toAdd += elem
}
i += 1
}
if (toAdd == null) this
else {
val numAdded = toAdd.size
val xs1 = new Array[AnyRef](size + numAdded)
System.arraycopy(xs, 0, xs1, 0, size)
var i = 0
while (i < numAdded) {
xs1(i + size) = toAdd(i)
i += 1
}
new SetN[E](xs1)
}
case _ => super.++(that)
}
override def -- [E >: Elem <: AnyRef](that: SimpleIdentitySet[E]): SimpleIdentitySet[E] =
that match {
case that: SetN[_] =>
// both sets are large, optimize assuming they are similar
// by starting from empty set and adding elements
var toAdd: mutable.ArrayBuffer[AnyRef] = null
val thisSize = this.size
val thatSize = that.size
val thatElems = that.xs
var i = 0
var searchStart = 0
while (i < thisSize) {
val elem = this.xs(i)
var j = searchStart // search thatElems in round robin fashion, starting one after latest hit
var missing = false
while (!missing && (elem ne thatElems(j))) {
j += 1
if (j == thatSize) j = 0
missing = j == searchStart
}
if (missing) {
if (toAdd == null) toAdd = new mutable.ArrayBuffer
toAdd += elem
}
else searchStart = (j + 1) % thatSize
i += 1
}
if (toAdd == null) empty
else toAdd.size match {
case 1 => new Set1[E](toAdd(0))
case 2 => new Set2[E](toAdd(0), toAdd(1))
case 3 => new Set3[E](toAdd(0), toAdd(1), toAdd(2))
case _ => new SetN[E](toAdd.toArray)
}
case _ => // this set is large, that set is small: reduce from above using `-`
((this: SimpleIdentitySet[E]) /: that)(_ - _)
}
}
}