Skip to content
Browse files

[backport] Fix unsafe array opt. / opt. primitive Array(...)

SI-6611, SI-6247 (partial fix)

The original commits on master were a bit circuitous, this
is squashed to a neat little package.

I had to add type arguments to the Array.apply calls in the
test case, they are inferred on master.

commit 41ff05dfdbcf032157b3509ace633f2e7a12295c
Author: Jason Zaugg <jzaugg@gmail.com>
Date:   Sun Nov 4 14:44:59 2012 +0100

    Refactor guards checking for a particular overload of Array.apply.
    (cherry picked from commit 092345a)

commit 1e5c942deccaf64f8d57bd8891b912381d7f220a
Author: Jason Zaugg <jzaugg@gmail.com>
Date:   Sun Nov 4 14:17:25 2012 +0100

    Expand optimization of Array(e1, ..., en) to primitive arrays.
    (cherry picked from commit 8265175)

commit ab1bf77e39f2dfeacf3fc107ccb2907a1867f04c
Author: Jason Zaugg <jzaugg@gmail.com>
Date:   Sat Nov 3 13:34:20 2012 +0100

    SI-6611 Tighten up an unsafe array optimization

    The net was cast too wide and was unsafely optimizing away array
    copies.
    (cherry picked from commit dad8866)

And also:

Optimize primitive Array(e1, ..., en)

Expands an existing optimization for reference arrays to
apply to primitives, as well.

Fixes one aspect of SI-6247.
(cherry picked from commit cac5a08)

Conflicts:
	src/compiler/scala/tools/nsc/transform/CleanUp.scala

More principled tree copying.

Canonical > home-spun.

Conflicts:
	src/compiler/scala/tools/nsc/transform/CleanUp.scala
  • Loading branch information...
1 parent 884737c commit ba411c4c2cb4b400481ed3dffed30b6975c000c1 @retronym retronym committed
View
19 src/compiler/scala/tools/nsc/transform/CleanUp.scala
@@ -15,6 +15,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
import global._
import definitions._
import CODE._
+ import treeInfo.StripCast
/** the following two members override abstract members in Transform */
val phaseName: String = "cleanup"
@@ -606,14 +607,16 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
}
transformApply
- // This transform replaces Array(Predef.wrapArray(Array(...)), <tag>)
- // with just Array(...)
- case Apply(appMeth, List(Apply(wrapRefArrayMeth, List(array)), _))
- if (wrapRefArrayMeth.symbol == Predef_wrapRefArray &&
- appMeth.symbol == ArrayModule_overloadedApply.suchThat {
- _.tpe.resultType.dealias.typeSymbol == ObjectClass
- }) =>
- super.transform(array)
+ // Replaces `Array(Predef.wrapArray(ArrayValue(...).$asInstanceOf[...]), <tag>)`
+ // with just `ArrayValue(...).$asInstanceOf[...]`
+ //
+ // See SI-6611; we must *only* do this for literal vararg arrays.
+ case Apply(appMeth, List(Apply(wrapRefArrayMeth, List(arg @ StripCast(ArrayValue(_, _)))), _))
+ if wrapRefArrayMeth.symbol == Predef_wrapRefArray && appMeth.symbol == ArrayModule_genericApply =>
+ super.transform(arg)
+ case Apply(appMeth, List(elem0, Apply(wrapArrayMeth, List(rest @ ArrayValue(elemtpt, _)))))
+ if wrapArrayMeth.symbol == Predef_wrapArray(elemtpt.tpe) && appMeth.symbol == ArrayModule_apply(elemtpt.tpe) =>
+ super.transform(treeCopy.ArrayValue(rest, rest.elemtpt, elem0 :: rest.elems))
case _ =>
super.transform(tree)
View
10 src/library/scala/Array.scala
@@ -115,6 +115,8 @@ object Array extends FallbackArrayBuilding {
* @param xs the elements to put in the array
* @return an array containing all elements from xs.
*/
+ // Subject to a compiler optimization in Cleanup.
+ // Array(e0, ..., en) is translated to { val a = new Array(3); a(i) = ei; a }
def apply[T: ClassTag](xs: T*): Array[T] = {
val array = new Array[T](xs.length)
var i = 0
@@ -123,6 +125,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Boolean` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Boolean, xs: Boolean*): Array[Boolean] = {
val array = new Array[Boolean](xs.length + 1)
array(0) = x
@@ -132,6 +135,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Byte` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Byte, xs: Byte*): Array[Byte] = {
val array = new Array[Byte](xs.length + 1)
array(0) = x
@@ -141,6 +145,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Short` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Short, xs: Short*): Array[Short] = {
val array = new Array[Short](xs.length + 1)
array(0) = x
@@ -150,6 +155,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Char` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Char, xs: Char*): Array[Char] = {
val array = new Array[Char](xs.length + 1)
array(0) = x
@@ -159,6 +165,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Int` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Int, xs: Int*): Array[Int] = {
val array = new Array[Int](xs.length + 1)
array(0) = x
@@ -168,6 +175,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Long` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Long, xs: Long*): Array[Long] = {
val array = new Array[Long](xs.length + 1)
array(0) = x
@@ -177,6 +185,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Float` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Float, xs: Float*): Array[Float] = {
val array = new Array[Float](xs.length + 1)
array(0) = x
@@ -186,6 +195,7 @@ object Array extends FallbackArrayBuilding {
}
/** Creates an array of `Double` objects */
+ // Subject to a compiler optimization in Cleanup, see above.
def apply(x: Double, xs: Double*): Array[Double] = {
val array = new Array[Double](xs.length + 1)
array(0) = x
View
15 src/reflect/scala/reflect/internal/Definitions.scala
@@ -337,12 +337,13 @@ trait Definitions extends api.StandardDefinitions {
lazy val PredefModule = requiredModule[scala.Predef.type]
lazy val PredefModuleClass = PredefModule.moduleClass
- def Predef_classOf = getMemberMethod(PredefModule, nme.classOf)
- def Predef_identity = getMemberMethod(PredefModule, nme.identity)
- def Predef_conforms = getMemberMethod(PredefModule, nme.conforms)
- def Predef_wrapRefArray = getMemberMethod(PredefModule, nme.wrapRefArray)
- def Predef_??? = getMemberMethod(PredefModule, nme.???)
- def Predef_implicitly = getMemberMethod(PredefModule, nme.implicitly)
+ def Predef_classOf = getMemberMethod(PredefModule, nme.classOf)
+ def Predef_identity = getMemberMethod(PredefModule, nme.identity)
+ def Predef_conforms = getMemberMethod(PredefModule, nme.conforms)
+ def Predef_wrapRefArray = getMemberMethod(PredefModule, nme.wrapRefArray)
+ def Predef_wrapArray(tp: Type) = getMemberMethod(PredefModule, wrapArrayMethodName(tp))
+ def Predef_??? = getMemberMethod(PredefModule, nme.???)
+ def Predef_implicitly = getMemberMethod(PredefModule, nme.implicitly)
/** Is `sym` a member of Predef with the given name?
* Note: DON't replace this by sym == Predef_conforms/etc, as Predef_conforms is a `def`
@@ -466,6 +467,8 @@ trait Definitions extends api.StandardDefinitions {
// arrays and their members
lazy val ArrayModule = requiredModule[scala.Array.type]
lazy val ArrayModule_overloadedApply = getMemberMethod(ArrayModule, nme.apply)
+ def ArrayModule_genericApply = ArrayModule_overloadedApply.suchThat(_.paramss.flatten.last.tpe.typeSymbol == ClassTagClass) // [T: ClassTag](xs: T*): Array[T]
+ def ArrayModule_apply(tp: Type) = ArrayModule_overloadedApply.suchThat(_.tpe.resultType =:= arrayType(tp)) // (p1: AnyVal1, ps: AnyVal1*): Array[AnyVal1]
lazy val ArrayClass = getRequiredClass("scala.Array") // requiredClass[scala.Array[_]]
lazy val Array_apply = getMemberMethod(ArrayClass, nme.apply)
lazy val Array_update = getMemberMethod(ArrayClass, nme.update)
View
14 src/reflect/scala/reflect/internal/TreeInfo.scala
@@ -234,6 +234,20 @@ abstract class TreeInfo {
tree
}
+ /** Strips layers of `.asInstanceOf[T]` / `_.$asInstanceOf[T]()` from an expression */
+ def stripCast(tree: Tree): Tree = tree match {
+ case TypeApply(sel @ Select(inner, _), _) if isCastSymbol(sel.symbol) =>
+ stripCast(inner)
+ case Apply(TypeApply(sel @ Select(inner, _), _), Nil) if isCastSymbol(sel.symbol) =>
+ stripCast(inner)
+ case t =>
+ t
+ }
+
+ object StripCast {
+ def unapply(tree: Tree): Some[Tree] = Some(stripCast(tree))
+ }
+
/** Is tree a self or super constructor call? */
def isSelfOrSuperConstrCall(tree: Tree) = {
// stripNamedApply for SI-3584: adaptToImplicitMethod in Typers creates a special context
View
1 test/files/instrumented/t6611.check
@@ -0,0 +1 @@
+Method call statistics:
View
35 test/files/instrumented/t6611.scala
@@ -0,0 +1,35 @@
+import scala.tools.partest.instrumented.Instrumentation._
+
+object Test {
+ def main(args: Array[String]) {
+ startProfiling()
+
+ // tests optimization in Cleanup for varargs reference arrays
+ Array("")
+
+
+ Array(true)
+ Array(true, false)
+ Array(1: Byte)
+ Array(1: Byte, 2: Byte)
+ Array(1: Short)
+ Array(1: Short, 2: Short)
+ Array(1)
+ Array(1, 2)
+ Array(1L)
+ Array(1L, 2L)
+ Array(1d)
+ Array(1d, 2d)
+ Array(1f)
+ Array(1f, 2f)
+
+ /* Not currently optimized:
+ Array[Int](1, 2) etc
+ Array(())
+ Array((), ())
+ */
+
+ stopProfiling()
+ printStatistics()
+ }
+}
View
61 test/files/run/t6611.scala
@@ -0,0 +1,61 @@
+object Test extends App {
+ locally {
+ val a = Array("1")
+ val a2 = Array(a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array("1": Object)
+ val a2 = Array[Object](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(true)
+ val a2 = Array[Boolean](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(1: Short)
+ val a2 = Array[Short](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(1: Byte)
+ val a2 = Array[Byte](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(1)
+ val a2 = Array[Int](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(1L)
+ val a2 = Array[Long](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(1f)
+ val a2 = Array[Float](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(1d)
+ val a2 = Array[Double](a: _*)
+ assert(a ne a2)
+ }
+
+ locally {
+ val a = Array(())
+ val a2 = Array[Unit](a: _*)
+ assert(a ne a2)
+ }
+}

0 comments on commit ba411c4

Please sign in to comment.
Something went wrong with that request. Please try again.