Skip to content

Commit

Permalink
Optimize lub algorithm (#20034)
Browse files Browse the repository at this point in the history 
Replace mergeIfSuper by a different algorithm that is more efficient.
We drop or-summands in both arguments of a lub that are subsumed by the
other.
This avoids expensive recursive calls to lub or expensive comparisons
with union types on the right.

I tested the previous performance regression #19907 with the new
algorithm, and without the changes in #19995 that avoid a slow lub.
Where previously it took minutes it now compiles fast.

Specifically, we get for i19907_slow_1000_3.scala: 2.9s with the
optimizations in #19995, 3.3s with just this PR. And for
i19907_slow_1000_4.scala: 3.9s with the optimizations in #19995, 4.5s
with just this PR. So the optimizations in #19995 are much less critical
now since lubs are much faster. Still, it's probably worthwhile to leave
them in in case there is a humongous program that stresses lubs even
more.
  • Loading branch information
@odersky
odersky authored Mar 29, 2024
2 parents fecfbda + b9430cb commit 1aba790
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 83 deletions.
133 changes: 68 additions & 65 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2352,8 +2352,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
}

/** The greatest lower bound of two types */
def glb(tp1: Type, tp2: Type): Type = /*>|>*/ trace(s"glb(${tp1.show}, ${tp2.show})", subtyping, show = true) /*<|<*/ {
if (tp1 eq tp2) tp1
def glb(tp1: Type, tp2: Type): Type = // trace(s"glb(${tp1.show}, ${tp2.show})", subtyping, show = true):
if tp1 eq tp2 then tp1
else if !tp1.exists || (tp1 eq WildcardType) then tp2
else if !tp2.exists || (tp2 eq WildcardType) then tp1
else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || isBottom(tp2) then tp2
Expand All @@ -2366,12 +2366,12 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
val tp2a = dropIfSuper(tp2, tp1)
if tp2a ne tp2 then glb(tp1, tp2a)
else tp2 match // normalize to disjunctive normal form if possible.
case tp2 @ OrType(tp21, tp22) =>
lub(tp1 & tp21, tp1 & tp22, isSoft = tp2.isSoft)
case tp2 @ OrType(tp2L, tp2R) =>
lub(tp1 & tp2L, tp1 & tp2R, isSoft = tp2.isSoft)
case _ =>
tp1 match
case tp1 @ OrType(tp11, tp12) =>
lub(tp11 & tp2, tp12 & tp2, isSoft = tp1.isSoft)
case tp1 @ OrType(tp1L, tp1R) =>
lub(tp1L & tp2, tp1R & tp2, isSoft = tp1.isSoft)
case tp1: ConstantType =>
tp2 match
case tp2: ConstantType =>
Expand All @@ -2386,8 +2386,10 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
NothingType
case _ => andType(tp1, tp2)
case _ => andType(tp1, tp2)
end mergedGlb

mergedGlb(dropExpr(tp1.stripLazyRef), dropExpr(tp2.stripLazyRef))
}
end glb

def widenInUnions(using Context): Boolean =
migrateTo3 || ctx.erasedTypes
Expand All @@ -2396,14 +2398,23 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
* @param canConstrain If true, new constraints might be added to simplify the lub.
* @param isSoft If the lub is a union, this determines whether it's a soft union.
*/
def lub(tp1: Type, tp2: Type, canConstrain: Boolean = false, isSoft: Boolean = true): Type = /*>|>*/ trace(s"lub(${tp1.show}, ${tp2.show}, canConstrain=$canConstrain, isSoft=$isSoft)", subtyping, show = true) /*<|<*/ {
if (tp1 eq tp2) tp1
def lub(tp1: Type, tp2: Type, canConstrain: Boolean = false, isSoft: Boolean = true): Type = // trace(s"lub(${tp1.show}, ${tp2.show}, canConstrain=$canConstrain, isSoft=$isSoft)", subtyping, show = true):
if tp1 eq tp2 then tp1
else if !tp1.exists || (tp2 eq WildcardType) then tp1
else if !tp2.exists || (tp1 eq WildcardType) then tp2
else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || isBottom(tp2) then tp1
else if tp2.isAny && !tp1.isLambdaSub || tp2.isAnyKind || isBottom(tp1) then tp2
else
def mergedLub(tp1: Type, tp2: Type): Type = {
def mergedLub(tp1: Type, tp2: Type): Type =
// First, if tp1 and tp2 are the same singleton type, return one of them.
if tp1.isSingleton && isSubType(tp1, tp2, whenFrozen = !canConstrain) then
return tp2
if tp2.isSingleton && isSubType(tp2, tp1, whenFrozen = !canConstrain) then
return tp1

// Second, handle special cases when tp1 and tp2 are disjunctions of
// singleton types. This saves time otherwise spent in
// costly subtype comparisons performed in dropIfSub below.
tp1.atoms match
case Atoms.Range(lo1, hi1) if !widenInUnions =>
tp2.atoms match
Expand All @@ -2413,20 +2424,24 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
if (hi1 & hi2).isEmpty then return orType(tp1, tp2, isSoft = isSoft)
case none =>
case none =>
val t1 = mergeIfSuper(tp1, tp2, canConstrain)
if (t1.exists) return t1

val t2 = mergeIfSuper(tp2, tp1, canConstrain)
if (t2.exists) return t2

def widen(tp: Type) = if (widenInUnions) tp.widen else tp.widenIfUnstable
// Third, try to simplify after widening as follows:
// 1. Drop all or-factors in tp2 that are subtypes of an or-factor
// in tp1, yielding tp2Final.
// 2. Drop all or-factors in tp1 that are subtypes of an or-factor
// in tp2Final, yielding tp1Final.
// 3. Combine the two final types in an OrType
def widen(tp: Type) =
if widenInUnions then tp.widen else tp.widenIfUnstable
val tp1w = widen(tp1)
val tp2w = widen(tp2)
if ((tp1 ne tp1w) || (tp2 ne tp2w)) lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft)
else orType(tp1w, tp2w, isSoft = isSoft) // no need to check subtypes again
}
val tp2Final = dropIfSub(tp2w, tp1w, canConstrain)
val tp1Final = dropIfSub(tp1w, tp2Final, canConstrain)
recombine(tp1Final, tp2Final, orType(_, _, isSoft = isSoft))
end mergedLub

mergedLub(dropExpr(tp1.stripLazyRef), dropExpr(tp2.stripLazyRef))
}
end lub

/** Try to produce joint arguments for a lub `A[T_1, ..., T_n] | A[T_1', ..., T_n']` using
* the following strategies:
Expand Down Expand Up @@ -2488,60 +2503,48 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
Nil
}

private def recombineAnd(tp: AndType, tp1: Type, tp2: Type) =
if (!tp1.exists) tp2
else if (!tp2.exists) tp1
else tp.derivedAndType(tp1, tp2)
private def recombine(tp1: Type, tp2: Type, rebuild: (Type, Type) => Type): Type =
if !tp1.exists then tp2
else if !tp2.exists then tp1
else rebuild(tp1, tp2)

private def recombine(tp1: Type, tp2: Type, tp: AndOrType): Type =
recombine(tp1, tp2, tp.derivedAndOrType)

/** If some (&-operand of) `tp` is a supertype of `sub` replace it with `NoType`.
*/
private def dropIfSuper(tp: Type, sub: Type): Type =
if (isSubTypeWhenFrozen(sub, tp)) NoType
else tp match {

def isSuperOf(sub: Type): Boolean = sub match
case AndType(sub1, sub2) => isSuperOf(sub1) || isSuperOf(sub2)
case sub: TypeVar if sub.isInstantiated => isSuperOf(sub.inst)
case _ => isSubTypeWhenFrozen(sub, tp)

tp match
case tp @ AndType(tp1, tp2) =>
recombineAnd(tp, dropIfSuper(tp1, sub), dropIfSuper(tp2, sub))
recombine(dropIfSuper(tp1, sub), dropIfSuper(tp2, sub), tp)
case tp: TypeVar if tp.isInstantiated =>
dropIfSuper(tp.inst, sub)
case _ =>
tp
}
if isSuperOf(sub) then NoType else tp
end dropIfSuper

/** Merge `t1` into `tp2` if t1 is a subtype of some &-summand of tp2.
*/
private def mergeIfSub(tp1: Type, tp2: Type): Type =
if (isSubTypeWhenFrozen(tp1, tp2)) tp1
else tp2 match {
case tp2 @ AndType(tp21, tp22) =>
val lower1 = mergeIfSub(tp1, tp21)
if (lower1 eq tp21) tp2
else if (lower1.exists) lower1 & tp22
else {
val lower2 = mergeIfSub(tp1, tp22)
if (lower2 eq tp22) tp2
else if (lower2.exists) tp21 & lower2
else NoType
}
case _ =>
NoType
}
/** If some (|-operand of) `tp` is a subtype of `sup` replace it with `NoType`. */
private def dropIfSub(tp: Type, sup: Type, canConstrain: Boolean): Type =

/** Merge `tp1` into `tp2` if tp1 is a supertype of some |-summand of tp2.
* @param canConstrain If true, new constraints might be added to make the merge possible.
*/
private def mergeIfSuper(tp1: Type, tp2: Type, canConstrain: Boolean): Type =
if (isSubType(tp2, tp1, whenFrozen = !canConstrain)) tp1
else tp2 match {
case tp2 @ OrType(tp21, tp22) =>
val higher1 = mergeIfSuper(tp1, tp21, canConstrain)
if (higher1 eq tp21) tp2
else if (higher1.exists) lub(higher1, tp22, isSoft = tp2.isSoft)
else {
val higher2 = mergeIfSuper(tp1, tp22, canConstrain)
if (higher2 eq tp22) tp2
else if (higher2.exists) lub(tp21, higher2, isSoft = tp2.isSoft)
else NoType
}
def isSubOf(sup: Type): Boolean = sup match
case OrType(sup1, sup2) => isSubOf(sup1) || isSubOf(sup2)
case sup: TypeVar if sup.isInstantiated => isSubOf(sup.inst)
case _ => isSubType(tp, sup, whenFrozen = !canConstrain)

tp match
case tp @ OrType(tp1, tp2) =>
recombine(dropIfSub(tp1, sup, canConstrain), dropIfSub(tp2, sup, canConstrain), tp)
case tp: TypeVar if tp.isInstantiated =>
dropIfSub(tp.inst, sup, canConstrain)
case _ =>
NoType
}
if isSubOf(sup) then NoType else tp
end dropIfSub

/** There's a window of vulnerability between ElimByName and Erasure where some
* ExprTypes `=> T` that appear as parameters of function types are not yet converted
Expand Down
11 changes: 2 additions & 9 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,8 @@ object TypeOps:
tp.derivedAlias(simplify(tp.alias, theMap))
case AndType(l, r) if !ctx.mode.is(Mode.Type) =>
simplify(l, theMap) & simplify(r, theMap)
case tp @ OrType(l, r)
if !ctx.mode.is(Mode.Type)
&& (tp.isSoft || l.isBottomType || r.isBottomType) =>
// Normalize A | Null and Null | A to A even if the union is hard (i.e.
// explicitly declared), but not if -Yexplicit-nulls is set. The reason is
// that in this case the normal asSeenFrom machinery is not prepared to deal
// with Nulls (which have no base classes). Under -Yexplicit-nulls, we take
// corrective steps, so no widening is wanted.
simplify(l, theMap) | simplify(r, theMap)
case tp @ OrType(l, r) if !ctx.mode.is(Mode.Type) =>
TypeComparer.lub(simplify(l, theMap), simplify(r, theMap), isSoft = tp.isSoft)
case tp @ CapturingType(parent, refs) =>
if !ctx.mode.is(Mode.Type)
&& refs.subCaptures(parent.captureSet, frozen = true).isOK
Expand Down
11 changes: 5 additions & 6 deletions compiler/src/dotty/tools/dotc/core/Types.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3606,12 +3606,11 @@ object Types extends TypeUtils {

override def widenUnionWithoutNull(using Context): Type =
if myUnionPeriod != ctx.period then
myUnion =
if isSoft then
TypeComparer.lub(tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, canConstrain = true, isSoft = isSoft) match
case union: OrType => union.join
case res => res
else derivedOrType(tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, soft = isSoft)
val union = TypeComparer.lub(
tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, canConstrain = isSoft, isSoft = isSoft)
myUnion = union match
case union: OrType if isSoft => union.join
case _ => union
if !isProvisional then myUnionPeriod = ctx.period
myUnion

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Namer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1961,7 +1961,8 @@ class Namer { typer: Typer =>
else
// don't strip @uncheckedVariance annot for default getters
TypeOps.simplify(tp.widenTermRefExpr,
if defaultTp.exists then TypeOps.SimplifyKeepUnchecked() else null) match
if defaultTp.exists then TypeOps.SimplifyKeepUnchecked() else null)
match
case ctp: ConstantType if sym.isInlineVal => ctp
case tp => TypeComparer.widenInferred(tp, pt, widenUnions = true)

Expand Down
8 changes: 8 additions & 0 deletions tests/pos/i10693.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
def test[A, B](a: A, b: B): A | B = a
val v0 = test("string", 1)
val v1 = test(1, "string")
val v2 = test(v0, v1)
val v3 = test(v1, v0)
val v4 = test(v2, v3)
val v5 = test(v3, v2)
val v6 = test(v4, v5)
4 changes: 2 additions & 2 deletions tests/semanticdb/metac.expect
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2020,7 +2020,7 @@ Symbols:
example/InstrumentTyper# => class InstrumentTyper extends Object { self: AnyRef & InstrumentTyper => +5 decls }
example/InstrumentTyper#AnnotatedType# => type AnnotatedType = Int @param
example/InstrumentTyper#`<init>`(). => primary ctor <init> (): InstrumentTyper
example/InstrumentTyper#all(). => method all => List[Float | Double | List[Nothing] | Boolean | Unit | Char | String | LinkOption | Int | Long | Class[Option[Int]]]
example/InstrumentTyper#all(). => method all => List[Char | String | LinkOption | Int | Long | Class[Option[Int]] | Float | Double | Boolean | Unit | List[Nothing]]
example/InstrumentTyper#clazzOf. => final val method clazzOf Option[Int]
example/InstrumentTyper#singletonType(). => method singletonType (param x: Predef.type): Nothing
example/InstrumentTyper#singletonType().(x) => param x: Predef.type
Expand Down Expand Up @@ -2082,7 +2082,7 @@ Occurrences:
[24:37..24:40): Int -> scala/Int#

Synthetics:
[8:12..8:16):List => *.apply[Float | Double | List[Nothing] | Boolean | Unit | Char | String | LinkOption | Int | Long | Class[Option[Int]]]
[8:12..8:16):List => *.apply[Char | String | LinkOption | Int | Long | Class[Option[Int]] | Float | Double | Boolean | Unit | List[Nothing]]
[20:4..20:8):List => *.apply[Nothing]

expect/InventedNames.scala
Expand Down

0 comments on commit 1aba790

Please sign in to comment.