From 17b659443606ea91a2718ed350fa448be9cb8f61 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 23 Mar 2023 10:43:00 -0700 Subject: [PATCH 1/2] More tailrec in handling long seq --- .../scala/reflect/internal/Printers.scala | 3 ++- .../scala/reflect/internal/tpe/GlbLubs.scala | 27 +++++++++++-------- test/files/run/t12757.scala | 16 +++++++++++ 3 files changed, 34 insertions(+), 12 deletions(-) create mode 100644 test/files/run/t12757.scala diff --git a/src/reflect/scala/reflect/internal/Printers.scala b/src/reflect/scala/reflect/internal/Printers.scala index 8d62aea85931..63a4485c93aa 100644 --- a/src/reflect/scala/reflect/internal/Printers.scala +++ b/src/reflect/scala/reflect/internal/Printers.scala @@ -109,7 +109,8 @@ trait Printers extends api.Printers { self: SymbolTable => out.write(indentString, 0, indentMargin) } - def printSeq[a](ls: List[a])(printelem: a => Unit)(printsep: => Unit): Unit = + @tailrec + final def printSeq[A](ls: List[A])(printelem: A => Unit)(printsep: => Unit): Unit = ls match { case List() => case List(x) => printelem(x) diff --git a/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala b/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala index ffb24459fce0..e0d9622dbb86 100644 --- a/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala +++ b/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala @@ -198,29 +198,34 @@ private[internal] trait GlbLubs { /** From a list of types, retain only maximal types as determined by the partial order `po`. */ private def maxTypes(ts: List[Type])(po: (Type, Type) => Boolean): List[Type] = { - def loop(ts: List[Type]): List[Type] = ts match { - case t :: ts1 => - val ts2 = loop(ts1.filterNot(po(_, t))) - if (ts2.exists(po(t, _))) ts2 else t :: ts2 - case Nil => Nil + @tailrec + def loop(remaining: List[Type], hs: List[Type]): List[Type] = remaining match { + case h :: rest => + loop(rest.filterNot(po(_, h)), h :: hs) + case _ => + def sieve(res: List[Type], todo: List[Type]): List[Type] = todo match { + case h :: tail => + val res1 = if (res.exists(po(h, _))) res else h :: res + sieve(res1, tail) + case _ => res + } + sieve(Nil, hs) } // The order here matters because type variables and // wildcards can act both as subtypes and supertypes. - val (ts2, ts1) = partitionConserve(ts) { tp => - isWildCardOrNonGroundTypeVarCollector.collect(tp).isDefined - } + val (wilds, ts1) = partitionConserve(ts)(isWildCardOrNonGroundTypeVarCollector.collect(_).isDefined) - loop(ts1 ::: ts2) + loop(ts1 ::: wilds, Nil) } /** Eliminate from list of types all elements which are a supertype - * of some other element of the list. */ + * of some other element of the list. */ private def elimSuper(ts: List[Type]): List[Type] = maxTypes(ts)((t1, t2) => t2 <:< t1) /** Eliminate from list of types all elements which are a subtype - * of some other element of the list. */ + * of some other element of the list. */ @tailrec private def elimSub(ts: List[Type], depth: Depth): List[Type] = { val ts1 = maxTypes(ts)(isSubType(_, _, depth.decr)) if (ts1.lengthCompare(1) <= 0) ts1 else { diff --git a/test/files/run/t12757.scala b/test/files/run/t12757.scala new file mode 100644 index 000000000000..5d24922b0668 --- /dev/null +++ b/test/files/run/t12757.scala @@ -0,0 +1,16 @@ + +import scala.tools.partest.DirectTest + +object Test extends DirectTest { + def header = """|object Test extends App { + | val myStrings: List[String] = List(""".stripMargin.linesIterator + def footer = """| ) + | println(myStrings.mkString(",")) + |}""".stripMargin.linesIterator + def values = Iterator.tabulate(4000)(i => s" \"$i\",") + def code = (header ++ values ++ footer).mkString("\n") + + override def extraSettings: String = "-usejavacp -J-Xms256k" + + def show() = assert(compile()) +} From b9a45183038a51fc292be8ca0b64e74a5c59b158 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 25 Mar 2023 23:39:27 -0700 Subject: [PATCH 2/2] Strings unify, fast path short lists of types --- .../scala/tools/partest/nest/Runner.scala | 3 +- .../scala/reflect/internal/tpe/GlbLubs.scala | 77 +++++++++++-------- .../internal/tpe/TypeConstraints.scala | 32 ++++---- test/files/run/t12757b.scala | 16 ++++ test/files/run/t12757c.scala | 19 +++++ 5 files changed, 102 insertions(+), 45 deletions(-) create mode 100644 test/files/run/t12757b.scala create mode 100644 test/files/run/t12757c.scala diff --git a/src/partest/scala/tools/partest/nest/Runner.scala b/src/partest/scala/tools/partest/nest/Runner.scala index 7905ae55ff69..1f63d1d75197 100644 --- a/src/partest/scala/tools/partest/nest/Runner.scala +++ b/src/partest/scala/tools/partest/nest/Runner.scala @@ -249,7 +249,8 @@ class Runner(val testInfo: TestInfo, val suiteRunner: AbstractRunner) { // We'll let the checkfile diffing report this failure Files.write(log.toPath, stackTraceString(t).getBytes(Charset.defaultCharset()), CREATE, APPEND) case t: Throwable => - Files.write(log.toPath, t.getMessage.getBytes(Charset.defaultCharset()), CREATE, APPEND) + val data = (if (t.getMessage != null) t.getMessage else t.getClass.getName).getBytes(Charset.defaultCharset()) + Files.write(log.toPath, data, CREATE, APPEND) throw t } } diff --git a/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala b/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala index e0d9622dbb86..92b652b10396 100644 --- a/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala +++ b/src/reflect/scala/reflect/internal/tpe/GlbLubs.scala @@ -15,7 +15,7 @@ package reflect package internal package tpe -import scala.collection.mutable +import scala.collection.mutable, mutable.ListBuffer import scala.annotation.tailrec import Variance._ @@ -101,20 +101,20 @@ private[internal] trait GlbLubs { def headOf(ix: Int) = baseTypeSeqs(ix).rawElem(ices(ix)) - val pretypes: mutable.ListBuffer[Type] = mutable.ListBuffer.empty[Type] + val pretypes: ListBuffer[Type] = ListBuffer.empty[Type] var isFinished = false - while (! isFinished && ices(0) < baseTypeSeqs(0).length){ + while (!isFinished && ices(0) < baseTypeSeqs(0).length) { lubListDepth = lubListDepth.incr // Step 1: run through the List with these variables: // 1) Is there any empty list? Are they equal or are we taking the smallest? // isFinished: tsBts.exists(typeListIsEmpty) // Is the frontier made up of types with the same symbol? - var isUniformFrontier = true + var isUniformFrontier = true var sym = headOf(0).typeSymbol // var tsYs = tsBts var ix = 0 - while (! isFinished && ix < baseTypeSeqs.length){ + while (!isFinished && ix < baseTypeSeqs.length) { if (ices(ix) == baseTypeSeqs(ix).length) isFinished = true else { @@ -130,7 +130,7 @@ private[internal] trait GlbLubs { // the invariant holds, i.e., the one that conveys most information regarding subtyping. Before // merging, strip targs that refer to bound tparams (when we're computing the lub of type // constructors.) Also filter out all types that are a subtype of some other type. - if (! isFinished){ + if (!isFinished) { // ts0 is the 1-dimensional frontier of symbols cutting through 2-dimensional tsBts. // Invariant: all symbols "under" (closer to the first row) the frontier // are smaller (according to _.isLess) than the ones "on and beyond" the frontier @@ -145,7 +145,7 @@ private[internal] trait GlbLubs { } if (isUniformFrontier) { - val ts1 = elimSub(ts0, depth) map elimHigherOrderTypeParam + val ts1 = elimSub(ts0, depth).map(elimHigherOrderTypeParam) mergePrefixAndArgs(ts1, Covariant, depth) match { case NoType => case tp => pretypes += tp @@ -165,11 +165,12 @@ private[internal] trait GlbLubs { jx += 1 } if (printLubs) { - val str = baseTypeSeqs.zipWithIndex.map({ case (tps, idx) => - tps.toList.drop(ices(idx)).map(" " + _ + "\n").mkString(" (" + idx + ")\n", "", "\n") - }).mkString("") - - println("Frontier(\n" + str + ")") + println { + baseTypeSeqs.zipWithIndex.map { case (tps, idx) => + tps.toList.drop(ices(idx)).map(" " + _).mkString(" (" + idx + ")\n", "\n", "\n") + } + .mkString("Frontier(\n", "", ")") + } printLubMatrixAux(lubListDepth) } } @@ -198,41 +199,57 @@ private[internal] trait GlbLubs { /** From a list of types, retain only maximal types as determined by the partial order `po`. */ private def maxTypes(ts: List[Type])(po: (Type, Type) => Boolean): List[Type] = { + def stacked(ts: List[Type]): List[Type] = ts match { + case t :: ts1 => + val ts2 = stacked(ts1.filterNot(po(_, t))) + if (ts2.exists(po(t, _))) ts2 else t :: ts2 + case Nil => Nil + } + + // loop thru tails, filtering for survivors of po test with the current element, which is saved for later culling @tailrec - def loop(remaining: List[Type], hs: List[Type]): List[Type] = remaining match { + def loop(survivors: List[Type], toCull: List[Type]): List[Type] = survivors match { case h :: rest => - loop(rest.filterNot(po(_, h)), h :: hs) + loop(rest.filterNot(po(_, h)), h :: toCull) case _ => - def sieve(res: List[Type], todo: List[Type]): List[Type] = todo match { + // unwind the stack of saved elements, accumulating a result containing elements surviving po (in swapped order) + def sieve(res: List[Type], remaining: List[Type]): List[Type] = remaining match { case h :: tail => val res1 = if (res.exists(po(h, _))) res else h :: res sieve(res1, tail) case _ => res } - sieve(Nil, hs) + toCull match { + case _ :: Nil => toCull + case _ => sieve(Nil, toCull) + } } - // The order here matters because type variables and - // wildcards can act both as subtypes and supertypes. - val (wilds, ts1) = partitionConserve(ts)(isWildCardOrNonGroundTypeVarCollector.collect(_).isDefined) - - loop(ts1 ::: wilds, Nil) + // The order here matters because type variables and wildcards can act both as subtypes and supertypes. + val sorted = { + val (wilds, ts1) = partitionConserve(ts)(isWildCardOrNonGroundTypeVarCollector.collect(_).isDefined) + ts1 ::: wilds + } + if (sorted.lengthCompare(5) > 0) loop(sorted, Nil) + else stacked(sorted) } /** Eliminate from list of types all elements which are a supertype * of some other element of the list. */ private def elimSuper(ts: List[Type]): List[Type] = - maxTypes(ts)((t1, t2) => t2 <:< t1) + if (ts.lengthCompare(1) <= 0) ts + else maxTypes(ts)((t1, t2) => t2 <:< t1) /** Eliminate from list of types all elements which are a subtype * of some other element of the list. */ - @tailrec private def elimSub(ts: List[Type], depth: Depth): List[Type] = { - val ts1 = maxTypes(ts)(isSubType(_, _, depth.decr)) - if (ts1.lengthCompare(1) <= 0) ts1 else { - val ts2 = ts1.mapConserve(t => elimAnonymousClass(t.dealiasWiden)) - if (ts1 eq ts2) ts1 else elimSub(ts2, depth) + @tailrec private def elimSub(ts: List[Type], depth: Depth): List[Type] = + if (ts.lengthCompare(1) <= 0) ts else { + val ts1 = maxTypes(ts)(isSubType(_, _, depth.decr)) + if (ts1.lengthCompare(1) <= 0) ts1 else { + val ts2 = ts1.mapConserve(t => elimAnonymousClass(t.dealiasWiden)) + if (ts1 eq ts2) ts1 else elimSub(ts2, depth) + } } - } /** Does this set of types have the same weak lub as * it does regular lub? This is exposed so lub callers @@ -496,7 +513,7 @@ private[internal] trait GlbLubs { val (ts, tparams) = stripExistentialsAndTypeVars(ts0) val glbOwner = commonOwner(ts) val ts1 = { - val res = mutable.ListBuffer.empty[Type] + val res = ListBuffer.empty[Type] def loop(ty: Type): Unit = ty match { case RefinedType(ps, _) => ps.foreach(loop) case _ => res += ty @@ -513,7 +530,7 @@ private[internal] trait GlbLubs { def glbsym(proto: Symbol): Symbol = { val prototp = glbThisType.memberInfo(proto) val symtypes: List[Type] = { - val res = mutable.ListBuffer.empty[Type] + val res = ListBuffer.empty[Type] ts foreach { t => t.nonPrivateMember(proto.name).alternatives foreach { alt => val mi = glbThisType.memberInfo(alt) diff --git a/src/reflect/scala/reflect/internal/tpe/TypeConstraints.scala b/src/reflect/scala/reflect/internal/tpe/TypeConstraints.scala index 9376640a5d17..bd61cb3bf0b0 100644 --- a/src/reflect/scala/reflect/internal/tpe/TypeConstraints.scala +++ b/src/reflect/scala/reflect/internal/tpe/TypeConstraints.scala @@ -24,8 +24,8 @@ private[internal] trait TypeConstraints { import definitions._ /** A log of type variable with their original constraints. Used in order - * to undo constraints in the case of isSubType/isSameType failure. - */ + * to undo constraints in the case of isSubType/isSameType failure. + */ private lazy val _undoLog = new UndoLog def undoLog = _undoLog @@ -54,9 +54,9 @@ private[internal] trait TypeConstraints { } /** No sync necessary, because record should only - * be called from within an undo or undoUnless block, - * which is already synchronized. - */ + * be called from within an undo or undoUnless block, + * which is already synchronized. + */ private[reflect] def record(tv: TypeVar) = { log ::= UndoPair(tv, tv.constr.cloneInternal) } @@ -96,11 +96,11 @@ private[internal] trait TypeConstraints { */ /** Guard these lists against AnyClass and NothingClass appearing, - * else loBounds.isEmpty will have different results for an empty - * constraint and one with Nothing as a lower bound. [Actually - * guarding addLoBound/addHiBound somehow broke raw types so it - * only guards against being created with them.] - */ + * else loBounds.isEmpty will have different results for an empty + * constraint and one with Nothing as a lower bound. [Actually + * guarding addLoBound/addHiBound somehow broke raw types so it + * only guards against being created with them.] + */ private[this] var lobounds = lo0 filterNot (_.isNothing) private[this] var hibounds = hi0 filterNot (_.isAny) private[this] var numlo = numlo0 @@ -124,15 +124,21 @@ private[internal] trait TypeConstraints { // See pos/t6367 and pos/t6499 for the competing test cases. val mustConsider = tp.typeSymbol match { case NothingClass => true - case _ => !(lobounds contains tp) + case _ => !lobounds.contains(tp) } if (mustConsider) { + def justTwoStrings: Boolean = ( + tp.typeSymbol == StringClass && tp.isInstanceOf[ConstantType] && + lobounds.lengthCompare(1) == 0 && lobounds.head.typeSymbol == StringClass + ) if (isNumericBound && isNumericValueType(tp)) { if (numlo == NoType || isNumericSubType(numlo, tp)) numlo = tp else if (!isNumericSubType(tp, numlo)) numlo = numericLoBound } + else if (justTwoStrings) + lobounds = tp.widen :: Nil // don't accumulate strings; we know they are not exactly the same bc mustConsider else lobounds ::= tp } } @@ -222,7 +228,7 @@ private[internal] trait TypeConstraints { @inline def toBound(hi: Boolean, tparam: Symbol) = if (hi) tparam.info.upperBound else tparam.info.lowerBound - def solveOne(tvar: TypeVar, isContravariant: Boolean): Unit = { + def solveOne(tvar: TypeVar, isContravariant: Boolean): Unit = if (tvar.constr.inst == NoType) { tvar.constr.inst = null // mark tvar as being solved @@ -252,7 +258,6 @@ private[internal] trait TypeConstraints { } } - if (!(otherTypeVarBeingSolved || containsSymbol(bound, tparam))) { val boundSym = bound.typeSymbol if (up) { @@ -284,7 +289,6 @@ private[internal] trait TypeConstraints { // debuglog(s"$tvar setInst $newInst") tvar setInst newInst } - } // println("solving "+tvars+"/"+tparams+"/"+(tparams map (_.info))) foreachWithIndex(tvars)((tvar, i) => solveOne(tvar, areContravariant(i))) diff --git a/test/files/run/t12757b.scala b/test/files/run/t12757b.scala new file mode 100644 index 000000000000..c83558853a50 --- /dev/null +++ b/test/files/run/t12757b.scala @@ -0,0 +1,16 @@ + +import scala.tools.partest.DirectTest + +object Test extends DirectTest { + def header = """|object Test extends App { + | val myInts: List[Int] = List(""".stripMargin.linesIterator + def footer = """| ) + | println(myInts.mkString(",")) + |}""".stripMargin.linesIterator + def values = Iterator.tabulate(4000)(i => s" $i,") + def code = (header ++ values ++ footer).mkString("\n") + + override def extraSettings: String = "-usejavacp -J-Xms256k" + + def show() = assert(compile()) +} diff --git a/test/files/run/t12757c.scala b/test/files/run/t12757c.scala new file mode 100644 index 000000000000..e387f7697e9e --- /dev/null +++ b/test/files/run/t12757c.scala @@ -0,0 +1,19 @@ + +import scala.tools.partest.DirectTest + +object Test extends DirectTest { + def header = """|object Test extends App { + | val myStrings = List( + | 42, + | Test, + |""".stripMargin.linesIterator + def footer = """| ) + | println(myStrings.mkString(",")) + |}""".stripMargin.linesIterator + def values = Iterator.tabulate(4000)(i => s" \"$i\",") + def code = (header ++ values ++ footer).mkString("\n") + + override def extraSettings: String = "-usejavacp -J-Xms256k" + + def show() = assert(compile()) +}