Skip to content

Commit b09fe7d

Browse files
dwijnanddwijnand
committed
Fixup List optimisation improvement
1 parent ca7bd7d commit b09fe7d

File tree

4 files changed

+88
-34
lines changed

4 files changed

+88
-34
lines changed

‎compiler/src/dotty/tools/dotc/core/Definitions.scala

+6-4
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,7 @@ class Definitions {
521521
def ListType: TypeRef = ListClass.typeRef
522522
@tu lazy val ListModule: Symbol = requiredModule("scala.collection.immutable.List")
523523
@tu lazy val ListModule_apply: Symbol = ListModule.requiredMethod(nme.apply)
524+
def ListModuleAlias: Symbol = ScalaPackageClass.requiredMethod(nme.List)
524525
@tu lazy val NilModule: Symbol = requiredModule("scala.collection.immutable.Nil")
525526
def NilType: TermRef = NilModule.termRef
526527
@tu lazy val ConsClass: Symbol = requiredClass("scala.collection.immutable.::")
@@ -535,17 +536,18 @@ class Definitions {
535536
List(AnyType), EmptyScope)
536537
@tu lazy val SingletonType: TypeRef = SingletonClass.typeRef
537538

538-
@tu lazy val CollectionSeqType: TypeRef = requiredClassRef("scala.collection.Seq")
539-
@tu lazy val SeqType: TypeRef = requiredClassRef("scala.collection.immutable.Seq")
539+
@tu lazy val CollectionSeqType: TypeRef = requiredClassRef("scala.collection.Seq")
540+
@tu lazy val SeqType: TypeRef = requiredClassRef("scala.collection.immutable.Seq")
541+
@tu lazy val SeqModule: Symbol = requiredModule("scala.collection.immutable.Seq")
542+
@tu lazy val SeqModule_apply: Symbol = SeqModule.requiredMethod(nme.apply)
543+
def SeqModuleAlias: Symbol = ScalaPackageClass.requiredMethod(nme.Seq)
540544
def SeqClass(using Context): ClassSymbol = SeqType.symbol.asClass
541545
@tu lazy val Seq_apply : Symbol = SeqClass.requiredMethod(nme.apply)
542546
@tu lazy val Seq_head : Symbol = SeqClass.requiredMethod(nme.head)
543547
@tu lazy val Seq_drop : Symbol = SeqClass.requiredMethod(nme.drop)
544548
@tu lazy val Seq_lengthCompare: Symbol = SeqClass.requiredMethod(nme.lengthCompare, List(IntType))
545549
@tu lazy val Seq_length : Symbol = SeqClass.requiredMethod(nme.length)
546550
@tu lazy val Seq_toSeq : Symbol = SeqClass.requiredMethod(nme.toSeq)
547-
@tu lazy val SeqModule : Symbol = requiredModule("scala.collection.immutable.Seq")
548-
@tu lazy val SeqModule_apply : Symbol = SeqModule.requiredMethod(nme.apply)
549551

550552

551553
@tu lazy val StringOps: Symbol = requiredClass("scala.collection.StringOps")

‎compiler/src/dotty/tools/dotc/transform/ArrayApply.scala

+39-26
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
1-
package dotty.tools.dotc
1+
package dotty.tools
2+
package dotc
23
package transform
34

4-
import core.*
5+
import ast.tpd
6+
import core.*, Contexts.*, Decorators.*, Symbols.*, Flags.*, StdNames.*
7+
import reporting.trace
58
import MegaPhase.*
6-
import Contexts.*
7-
import Symbols.*
8-
import Flags.*
9-
import StdNames.*
10-
import dotty.tools.dotc.ast.tpd
11-
12-
139

1410
/** This phase rewrites calls to `Array.apply` to a direct instantiation of the array in the bytecode.
1511
*
@@ -22,49 +18,66 @@ class ArrayApply extends MiniPhase {
2218

2319
override def description: String = ArrayApply.description
2420

25-
private var transformListApplyLimit = 8
21+
private val transformListApplyLimit = 8
2622

27-
private def reducingTransformListApply[A](depth: Int)(body: => A): A = {
28-
val saved = transformListApplyLimit
29-
transformListApplyLimit -= depth
30-
try body
31-
finally transformListApplyLimit = saved
32-
}
23+
override def transformTypeApply(tree: TypeApply)(using Context): Tree =
24+
stripCast(tree) match
25+
case app: Apply if isConsChain(app) => app
26+
case _ => tree
3327

34-
override def transformApply(tree: tpd.Apply)(using Context): tpd.Tree =
28+
override def transformApply(tree: Apply)(using Context): Tree =
3529
if isArrayModuleApply(tree.symbol) then
3630
tree.args match
37-
case StripAscription(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: ct :: Nil
31+
case StripAscription(Apply(wrapRefArrayMeth, (seqLit: JavaSeqLiteral) :: Nil)) :: ct :: Nil
3832
if defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) && elideClassTag(ct) =>
3933
seqLit
4034

41-
case elem0 :: StripAscription(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: Nil
35+
case elem0 :: StripAscription(Apply(wrapRefArrayMeth, (seqLit: JavaSeqLiteral) :: Nil)) :: Nil
4236
if defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) =>
43-
tpd.JavaSeqLiteral(elem0 :: seqLit.elems, seqLit.elemtpt)
37+
JavaSeqLiteral(elem0 :: seqLit.elems, seqLit.elemtpt)
4438

4539
case _ =>
4640
tree
4741

48-
else if isListOrSeqModuleApply(tree.symbol) then
42+
else if isSeqApply(tree) then
4943
tree.args match
5044
// <List or Seq>(a, b, c) ~> new ::(a, new ::(b, new ::(c, Nil))) but only for reference types
51-
case StripAscription(Apply(wrapArrayMeth, List(StripAscription(rest: tpd.JavaSeqLiteral)))) :: Nil
45+
case StripAscription(Apply(wrapArrayMeth, List(StripAscription(rest: JavaSeqLiteral)))) :: Nil
5246
if defn.WrapArrayMethods().contains(wrapArrayMeth.symbol) &&
5347
rest.elems.lengthIs < transformListApplyLimit =>
54-
rest.elems.foldRight(tpd.ref(defn.NilModule)): (elem, acc) =>
55-
tpd.New(defn.ConsType, List(elem.ensureConforms(defn.ObjectType), acc))
48+
rest.elems.foldRight(ref(defn.NilModule)): (elem, acc) =>
49+
New(defn.ConsType, List(elem.ensureConforms(defn.ObjectType), acc))
5650

5751
case _ =>
5852
tree
5953

6054
else tree
6155

56+
private def isConsChain(tree: Tree)(using Context): Boolean = tree match
57+
case Apply(Select(New(tt), nme.CONSTRUCTOR), List(_, arg)) =>
58+
tt.symbol == defn.ConsClass && (arg.symbol == defn.NilModule || isConsChain(arg))
59+
case _ => false
60+
6261
private def isArrayModuleApply(sym: Symbol)(using Context): Boolean =
6362
sym.name == nme.apply
6463
&& (sym.owner == defn.ArrayModuleClass || (sym.owner == defn.IArrayModuleClass && !sym.is(Extension)))
6564

66-
private def isListOrSeqModuleApply(sym: Symbol)(using Context): Boolean =
67-
sym == defn.ListModule_apply || sym == defn.SeqModule_apply
65+
private def isListApply(tree: Tree)(using Context): Boolean =
66+
(tree.symbol == defn.ListModule_apply || tree.symbol.name == nme.apply) && appliedCore(tree).match
67+
case Select(qual, _) =>
68+
val sym = qual.symbol
69+
sym == defn.ListModule
70+
|| sym == defn.ListModuleAlias
71+
case _ => false
72+
73+
private def isSeqApply(tree: Tree)(using Context): Boolean =
74+
isListApply(tree) || tree.symbol == defn.SeqModule_apply && appliedCore(tree).match
75+
case Select(qual, _) =>
76+
val sym = qual.symbol
77+
sym == defn.SeqModule
78+
|| sym == defn.SeqModuleAlias
79+
|| sym == defn.CollectionSeqType.symbol.companionModule
80+
case _ => false
6881

6982
/** Only optimize when classtag if it is one of
7083
* - `ClassTag.apply(classOf[XYZ])`

‎compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala

+40-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
package dotty.tools.backend.jvm
1+
package dotty.tools
2+
package backend.jvm
23

34
import org.junit.Test
45
import org.junit.Assert._
@@ -161,15 +162,50 @@ class ArrayApplyOptTest extends DottyBytecodeTest {
161162
}
162163

163164
@Test def testListApplyAvoidsIntermediateArray = {
164-
val source =
165+
checkApplyAvoidsIntermediateArray("List"):
165166
"""
166167
|class Foo {
167168
| def meth1: List[String] = List("1", "2", "3")
168169
| def meth2: List[String] =
169-
| new scala.collection.immutable.::("1", new scala.collection.immutable.::("2", new scala.collection.immutable.::("3", scala.collection.immutable.Nil))).asInstanceOf[List[String]]
170+
| new scala.collection.immutable.::("1", new scala.collection.immutable.::("2", new scala.collection.immutable.::("3", scala.collection.immutable.Nil))).asInstanceOf[List[String]]
171+
|}
172+
""".stripMargin
173+
}
174+
175+
@Test def testSeqApplyAvoidsIntermediateArray = {
176+
checkApplyAvoidsIntermediateArray("Seq"):
177+
"""
178+
|class Foo {
179+
| def meth1: Seq[String] = Seq("1", "2", "3")
180+
| def meth2: Seq[String] =
181+
| new scala.collection.immutable.::("1", new scala.collection.immutable.::("2", new scala.collection.immutable.::("3", scala.collection.immutable.Nil))).asInstanceOf[List[String]]
170182
|}
171183
""".stripMargin
184+
}
185+
186+
@Test def testSeqApplyAvoidsIntermediateArray2 = {
187+
checkApplyAvoidsIntermediateArray("scala.collection.immutable.Seq"):
188+
"""import scala.collection.immutable.Seq
189+
|class Foo {
190+
| def meth1: Seq[String] = Seq("1", "2", "3")
191+
| def meth2: Seq[String] =
192+
| new scala.collection.immutable.::("1", new scala.collection.immutable.::("2", new scala.collection.immutable.::("3", scala.collection.immutable.Nil))).asInstanceOf[List[String]]
193+
|}
194+
""".stripMargin
195+
}
196+
197+
@Test def testSeqApplyAvoidsIntermediateArray3 = {
198+
checkApplyAvoidsIntermediateArray("scala.collection.Seq"):
199+
"""import scala.collection.Seq
200+
|class Foo {
201+
| def meth1: Seq[String] = Seq("1", "2", "3")
202+
| def meth2: Seq[String] =
203+
| new scala.collection.immutable.::("1", new scala.collection.immutable.::("2", new scala.collection.immutable.::("3", scala.collection.immutable.Nil))).asInstanceOf[List[String]]
204+
|}
205+
""".stripMargin
206+
}
172207

208+
def checkApplyAvoidsIntermediateArray(name: String)(source: String) = {
173209
checkBCode(source) { dir =>
174210
val clsIn = dir.lookupName("Foo.class", directory = false).input
175211
val clsNode = loadClassNode(clsIn)
@@ -180,7 +216,7 @@ class ArrayApplyOptTest extends DottyBytecodeTest {
180216
val instructions2 = instructionsFromMethod(meth2)
181217

182218
assert(instructions1 == instructions2,
183-
"the List.apply method " +
219+
s"the $name.apply method\n" +
184220
diffInstructions(instructions1, instructions2))
185221
}
186222
}

‎tests/run/list-apply-eval.scala

+3
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,6 @@ object Test:
1919

2020
val emptyList = List[Int]()
2121
assert(emptyList == Nil)
22+
23+
// just assert it doesn't throw CCE to List
24+
val queue = scala.collection.mutable.Queue[String]()

0 commit comments

Comments
 (0)