From 2165c4f543f14febd1c3ee2d1462c63ebbda7aba Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 30 Jun 2022 11:46:34 +0200 Subject: [PATCH 1/2] Optimize same type test for invariant applied types with the same structure Two deeply applied types with the same structure uses recursive isSameType tests. Each of these translated to two isSubType tests which can lead to exponential blowup relative to the type's nesting depth. This problem does not occur if the two types are `eq`. But two types might still be structurally equal modulo dealiasing. We now cache isSameType successes after a certain nesting level to avoid recomputation. Fixes #15525. Reclassifies #15365 as a pos test --- .../dotty/tools/dotc/core/TypeComparer.scala | 49 ++++++++++++++--- tests/neg/i15525.scala | 53 ++++++++++++++++++ .../allow-deep-subtypes => pos}/i15365.scala | 2 +- tests/pos/i15525.scala | 55 +++++++++++++++++++ 4 files changed, 150 insertions(+), 9 deletions(-) create mode 100644 tests/neg/i15525.scala rename tests/{neg-custom-args/allow-deep-subtypes => pos}/i15365.scala (95%) create mode 100644 tests/pos/i15525.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 6500c166c1a4..bcb1ee06708e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -8,8 +8,7 @@ import Phases.{gettersPhase, elimByNamePhase} import StdNames.nme import TypeOps.refineUsingParent import collection.mutable -import util.Stats -import util.NoSourcePosition +import util.{Stats, NoSourcePosition, EqHashMap} import config.Config import config.Feature.migrateTo3 import config.Printers.{subtyping, gadts, matchTypes, noPrinter} @@ -163,6 +162,20 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling /** A flag to prevent recursive joins when comparing AndTypes on the left */ private var joined = false + /** A variable to keep track of number of outstanding isSameType tests */ + private var sameLevel = 0 + + /** A map that records successful isSameType comparisons. + * Used together with `sameLevel` to avoid exponential blowUp of isSameType + * comparisons for deeply nested invariant applied types. + */ + private var sames: util.EqHashMap[Type, Type] | Null = null + + /** The `sameLevel` nesting depth from which on we want to keep track + * of isSameTypes suucesses using `sames` + */ + val startSameTypeTrackingLevel = 3 + private inline def inFrozenGadtIf[T](cond: Boolean)(inline op: T): T = { val savedFrozenGadt = frozenGadt frozenGadt ||= cond @@ -1553,8 +1566,9 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling && defn.isByNameFunction(arg2.dealias) => isSubArg(arg1res, arg2.argInfos.head) case _ => - (v > 0 || isSubType(arg2, arg1)) && - (v < 0 || isSubType(arg1, arg2)) + if v > 0 then isSubType(arg1, arg2) + else if v < 0 then isSubType(arg2, arg1) + else isSameType(arg1, arg2) isSubArg(args1.head, args2.head) } && recurArgs(args1.tail, args2.tail, tparams2.tail) @@ -2012,11 +2026,30 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // Type equality =:= - /** Two types are the same if are mutual subtypes of each other */ + /** Two types are the same if they are mutual subtypes of each other. + * To avoid exponential blowup for deeply nested invariant applied types, + * we cache successes once the stack of outstanding isSameTypes reaches + * depth `startSameTypeTrackingLevel`. See pos/i15525.scala, where this matters. + */ def isSameType(tp1: Type, tp2: Type): Boolean = - if (tp1 eq NoType) false - else if (tp1 eq tp2) true - else isSubType(tp1, tp2) && isSubType(tp2, tp1) + if tp1 eq NoType then false + else if tp1 eq tp2 then true + else + sames != null && (sames.nn.lookup(tp1) eq tp2) + || { + val savedSames = sames + sameLevel += 1 + if sameLevel >= startSameTypeTrackingLevel then + Stats.record("cache same type") + sames = new util.EqHashMap() + val res = + try isSubType(tp1, tp2) && isSubType(tp2, tp1) + finally + sameLevel -= 1 + sames = savedSames + if res && sames != null then sames.nn(tp2) = tp1 + res + } override protected def isSame(tp1: Type, tp2: Type)(using Context): Boolean = isSameType(tp1, tp2) diff --git a/tests/neg/i15525.scala b/tests/neg/i15525.scala new file mode 100644 index 000000000000..0813d7c82435 --- /dev/null +++ b/tests/neg/i15525.scala @@ -0,0 +1,53 @@ +class /[D, T] +class Delegating[D] + +type Aux[E] = Container { type Elements = E } + +class Container: + type Elements = Delegating[Delegates] + type Delegates + +class Resolution[E](value: Aux[E]): + type Type = Aux[E] + +def element22( + transmittable0: Resolution[?], transmittable1: Resolution[?], + transmittable2: Resolution[?], transmittable3: Resolution[?], + transmittable4: Resolution[?], transmittable5: Resolution[?], + transmittable6: Resolution[?], transmittable7: Resolution[?], + transmittable8: Resolution[?], transmittable9: Resolution[?], + transmittable10: Resolution[?], transmittable11: Resolution[?], + transmittable12: Resolution[?], transmittable13: Resolution[?], + transmittable14: Resolution[?], transmittable15: Resolution[?], + transmittable16: Resolution[?], transmittable17: Resolution[?], + transmittable18: Resolution[?], transmittable19: Resolution[?], + transmittable20: Resolution[?], transmittable21: Resolution[?]) +: Container { + type Delegates = + transmittable0.Type / transmittable1.Type / + transmittable2.Type / transmittable3.Type / + transmittable4.Type / transmittable5.Type / + transmittable6.Type / transmittable7.Type / + transmittable8.Type / transmittable9.Type / + transmittable10.Type / transmittable11.Type / + transmittable12.Type / transmittable13.Type / + transmittable14.Type / transmittable15.Type / + transmittable16.Type / transmittable17.Type / + transmittable18.Type / transmittable19.Type / + transmittable20.Type / transmittable21.Type + } = ??? + +def test22 = + Resolution( + element22( + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0), // error // error + Resolution(element0), Resolution(element0)))// error // error diff --git a/tests/neg-custom-args/allow-deep-subtypes/i15365.scala b/tests/pos/i15365.scala similarity index 95% rename from tests/neg-custom-args/allow-deep-subtypes/i15365.scala rename to tests/pos/i15365.scala index f7d368b5223a..04ada7ca56aa 100644 --- a/tests/neg-custom-args/allow-deep-subtypes/i15365.scala +++ b/tests/pos/i15365.scala @@ -14,4 +14,4 @@ class OptionInputType[TO](ofType: InputType[TO]) extends InputType[Option[TO]] type Argument[TA] def argument[Ta](argumentType: InputType[Ta])(implicit fromInput: FromInput[Ta], res: WithoutInputTypeTags[Ta]): Argument[Option[Ta]] = ??? -def test = argument(OptionInputType(??? : InputType[WithTag[Boolean, Int]])) :: Nil // error +def test = argument(OptionInputType(??? : InputType[WithTag[Boolean, Int]])) :: Nil diff --git a/tests/pos/i15525.scala b/tests/pos/i15525.scala new file mode 100644 index 000000000000..89d4bb38d94b --- /dev/null +++ b/tests/pos/i15525.scala @@ -0,0 +1,55 @@ +class /[D, T] +class Delegating[D] + +type Aux[E] = Container { type Elements = E } + +class Container: + type Elements = Delegating[Delegates] + type Delegates + +class Resolution[E](value: Aux[E]): + type Type = Aux[E] + +def element0: Container { type Delegates = Unit } = ??? + +def element22( + transmittable0: Resolution[?], transmittable1: Resolution[?], + transmittable2: Resolution[?], transmittable3: Resolution[?], + transmittable4: Resolution[?], transmittable5: Resolution[?], + transmittable6: Resolution[?], transmittable7: Resolution[?], + transmittable8: Resolution[?], transmittable9: Resolution[?], + transmittable10: Resolution[?], transmittable11: Resolution[?], + transmittable12: Resolution[?], transmittable13: Resolution[?], + transmittable14: Resolution[?], transmittable15: Resolution[?], + transmittable16: Resolution[?], transmittable17: Resolution[?], + transmittable18: Resolution[?], transmittable19: Resolution[?], + transmittable20: Resolution[?], transmittable21: Resolution[?]) +: Container { + type Delegates = + transmittable0.Type / transmittable1.Type / + transmittable2.Type / transmittable3.Type / + transmittable4.Type / transmittable5.Type / + transmittable6.Type / transmittable7.Type / + transmittable8.Type / transmittable9.Type / + transmittable10.Type / transmittable11.Type / + transmittable12.Type / transmittable13.Type / + transmittable14.Type / transmittable15.Type / + transmittable16.Type / transmittable17.Type / + transmittable18.Type / transmittable19.Type / + transmittable20.Type / transmittable21.Type + } = ??? + +def test22 = + Resolution( + element22( + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0), + Resolution(element0), Resolution(element0))) From a54fac40455c2eda16f27333cd8eb5abea61a6fe Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 30 Jun 2022 14:34:23 +0200 Subject: [PATCH 2/2] Perform subtype comparisons in same order as before. Interestingly, swapping the order in which argument subtypes were tested in `isSubArg` caused - one exhaustivity failure (patmat/i9631.scala) - one failed compile in the community build (libretto). - one reclassification of a neg test to a pos test (i15365.scala) We should find out at some point why, when we have the time. --- .../dotty/tools/dotc/core/TypeComparer.scala | 34 +++++++++---------- .../allow-deep-subtypes}/i15365.scala | 2 +- 2 files changed, 17 insertions(+), 19 deletions(-) rename tests/{pos => neg-custom-args/allow-deep-subtypes}/i15365.scala (95%) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index bcb1ee06708e..6d79f377c84e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1566,9 +1566,9 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling && defn.isByNameFunction(arg2.dealias) => isSubArg(arg1res, arg2.argInfos.head) case _ => - if v > 0 then isSubType(arg1, arg2) - else if v < 0 then isSubType(arg2, arg1) - else isSameType(arg1, arg2) + if v < 0 then isSubType(arg2, arg1) + else if v > 0 then isSubType(arg1, arg2) + else isSameType(arg2, arg1) isSubArg(args1.head, args2.head) } && recurArgs(args1.tail, args2.tail, tparams2.tail) @@ -2034,22 +2034,20 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling def isSameType(tp1: Type, tp2: Type): Boolean = if tp1 eq NoType then false else if tp1 eq tp2 then true + else if sames != null && (sames.nn.lookup(tp1) eq tp2) then true else - sames != null && (sames.nn.lookup(tp1) eq tp2) - || { - val savedSames = sames - sameLevel += 1 - if sameLevel >= startSameTypeTrackingLevel then - Stats.record("cache same type") - sames = new util.EqHashMap() - val res = - try isSubType(tp1, tp2) && isSubType(tp2, tp1) - finally - sameLevel -= 1 - sames = savedSames - if res && sames != null then sames.nn(tp2) = tp1 - res - } + val savedSames = sames + sameLevel += 1 + if sameLevel >= startSameTypeTrackingLevel then + Stats.record("cache same type") + sames = new util.EqHashMap() + val res = + try isSubType(tp1, tp2) && isSubType(tp2, tp1) + finally + sameLevel -= 1 + sames = savedSames + if res && sames != null then sames.nn(tp2) = tp1 + res override protected def isSame(tp1: Type, tp2: Type)(using Context): Boolean = isSameType(tp1, tp2) diff --git a/tests/pos/i15365.scala b/tests/neg-custom-args/allow-deep-subtypes/i15365.scala similarity index 95% rename from tests/pos/i15365.scala rename to tests/neg-custom-args/allow-deep-subtypes/i15365.scala index 04ada7ca56aa..f7d368b5223a 100644 --- a/tests/pos/i15365.scala +++ b/tests/neg-custom-args/allow-deep-subtypes/i15365.scala @@ -14,4 +14,4 @@ class OptionInputType[TO](ofType: InputType[TO]) extends InputType[Option[TO]] type Argument[TA] def argument[Ta](argumentType: InputType[Ta])(implicit fromInput: FromInput[Ta], res: WithoutInputTypeTags[Ta]): Argument[Option[Ta]] = ??? -def test = argument(OptionInputType(??? : InputType[WithTag[Boolean, Int]])) :: Nil +def test = argument(OptionInputType(??? : InputType[WithTag[Boolean, Int]])) :: Nil // error