Define TupleXXL and FunctionXXL using IArray #6929

nicolasstucki · 2019-07-24T17:00:05Z

This way we can avoid some unnecessary array copies.

liufengyun

Otherwise, LGTM

liufengyun · 2019-07-26T06:08:50Z

library/src/scala/Tuple.scala

+      case xs: IArray[Object] => xs
+      case xs =>
+        // TODO suport IArray.map
+        xs.asInstanceOf[Array[T]].map(_.asInstanceOf[Object]).asInstanceOf[IArray[Object]]


Will a bound on IArray work, like opaque type IArray[+T] <: Seq[T] = Array[_ <: T]?

Probably not. We need to do the same as array does and define a WrappedIArray for it.

liufengyun · 2019-07-26T06:11:20Z

library/src/scala/TupleXXL.scala

  assert(es.length > 22)

  def productElement(n: Int): Any = es(n)
  def productArity: Int = es.length

-  override def toString = elems.mkString("(", ",", ")")
-  override def hashCode = getClass.hashCode * 41 + deepHashCode(elems)
+  override def toString = elems.asInstanceOf[Array[Object]].mkString("(", ",", ")")


After the bound on IArray, this cast may be avoided.

liufengyun · 2019-07-26T06:14:04Z

library/src/scala/TupleXXL.scala


  def tailXXL: TupleXXL = {
    assert(es.length > 23)
-    new TupleXXL(es.tail)
+    new TupleXXL(es.asInstanceOf[Array[Object]].tail.asInstanceOf[IArray[Object]])


It would be nice to support IArray.tail.

Adding WrappedIArray would do it. Also imlementing all ArrayOps on IArray would help

liufengyun · 2019-07-26T06:19:11Z

library/src/scala/TupleXXL.scala

-  def apply(elems: Array[Object]) = new TupleXXL(elems.clone)
-  def apply(elems: Any*) = new TupleXXL(elems.asInstanceOf[Seq[Object]].toArray)
-  def unapplySeq(x: TupleXXL): Option[Seq[Any]] = Some(x.elems.toSeq)
+  def fromIterator(elems: Iterator[Any]) = new TupleXXL(elems.map(_.asInstanceOf[Object]).toArray.asInstanceOf[IArray[Object]])


It's better to avoid .asInstanceOf[IArray[Object]], as the Array instance might be aliased, thus break the protocol. Create IArray through its constructors is recommended.

IArray would copy the array at least twice, this is the best we can do for now. We need Iterator.toIArray.

liufengyun · 2019-07-26T06:19:58Z

library/src/scala/TupleXXL.scala

-  def unapplySeq(x: TupleXXL): Option[Seq[Any]] = Some(x.elems.toSeq)
+  def fromIterator(elems: Iterator[Any]) = new TupleXXL(elems.map(_.asInstanceOf[Object]).toArray.asInstanceOf[IArray[Object]])
+  def fromIArray(elems: IArray[Object]) = new TupleXXL(elems)
+  def apply(elems: Any*) = new TupleXXL(elems.asInstanceOf[Seq[Object]].toArray.asInstanceOf[IArray[Object]])


It's better to create IArray through its constructors instead of cast.

liufengyun · 2019-07-26T06:20:31Z

library/src/scala/runtime/DynamicTuple.scala

@@ -52,9 +52,13 @@ object DynamicTuple {
    case 20 => Tuple20(xs(0), xs(1), xs(2), xs(3), xs(4), xs(5), xs(6), xs(7), xs(8), xs(9), xs(10), xs(11), xs(12), xs(13), xs(14), xs(15), xs(16), xs(17), xs(18), xs(19)).asInstanceOf[T]
    case 21 => Tuple21(xs(0), xs(1), xs(2), xs(3), xs(4), xs(5), xs(6), xs(7), xs(8), xs(9), xs(10), xs(11), xs(12), xs(13), xs(14), xs(15), xs(16), xs(17), xs(18), xs(19), xs(20)).asInstanceOf[T]
    case 22 => Tuple22(xs(0), xs(1), xs(2), xs(3), xs(4), xs(5), xs(6), xs(7), xs(8), xs(9), xs(10), xs(11), xs(12), xs(13), xs(14), xs(15), xs(16), xs(17), xs(18), xs(19), xs(20), xs(21)).asInstanceOf[T]
-    case _ => TupleXXL(xs).asInstanceOf[T]
+    case _ => TupleXXL.fromIArray(xs.clone().asInstanceOf[IArray[Object]]).asInstanceOf[T]


It's better to create IArray through its constructors instead of cast.

Good point. It might also be good to have a toIArray on collections

liufengyun · 2019-07-26T06:23:39Z

library/src/scala/runtime/DynamicTuple.scala

@@ -169,7 +173,7 @@ object DynamicTuple {
    case _ =>
      xs match {
        case xs: TupleXXL => xs
-        case xs => TupleXXL(xs.productIterator.map(_.asInstanceOf[Object]).toArray)
+        case xs => TupleXXL.fromIArray(xs.productIterator.map(_.asInstanceOf[Object]).toArray.asInstanceOf[IArray[Object]])


It's better to create IArray through its constructors instead of cast.

Same as before, need Iterator.toIArray

liufengyun · 2019-07-26T06:23:55Z

library/src/scala/runtime/DynamicTuple.scala

+  def dynamicToIArray(self: Tuple): IArray[Object] = (self: Any) match {
+    case self: Unit => Array.emptyObjectArray.asInstanceOf[IArray[Object]]
+    case self: TupleXXL => self.elems
+    case self: Product => productToArray(self).asInstanceOf[IArray[Object]]


It's better to create IArray through its constructors instead of cast.

The array constructor would create a new empty array each time

nicolasstucki · 2019-07-26T06:45:51Z

All this shows the need to implement all collection operations on IArray. I will open an issue describing the detail.

nicolasstucki · 2019-07-26T07:57:42Z

On another note, IArray is defined in library/src-bootstrapped and it's operation are not accessible in library/src. I will try to move IArray to library/src after this PR, the current reference compiler should support it.

liufengyun

LGTM

nicolasstucki self-assigned this Jul 24, 2019

nicolasstucki force-pushed the define-TupleXXL-using-IArray branch from 57eec5b to 490a66c Compare July 25, 2019 06:08

Define TupleXXL and FunctionXXL using IArray

3d827b4

This way we can avoid some unnecessary array copies.

nicolasstucki force-pushed the define-TupleXXL-using-IArray branch from 490a66c to 3d827b4 Compare July 25, 2019 08:15

nicolasstucki changed the title ~~Define TupleXXL using IArray~~ Define TupleXXL and FunctionXXL using IArray Jul 25, 2019

nicolasstucki marked this pull request as ready for review July 25, 2019 08:58

nicolasstucki requested a review from liufengyun July 25, 2019 08:59

nicolasstucki assigned liufengyun and unassigned nicolasstucki Jul 25, 2019

liufengyun approved these changes Jul 26, 2019

View reviewed changes

nicolasstucki added 2 commits July 26, 2019 09:39

Use IArray.apply and state where operations are missing on IArray

14bbae8

Add empty IArrays

0c320fa

nicolasstucki requested a review from liufengyun July 26, 2019 10:15

liufengyun approved these changes Jul 26, 2019

View reviewed changes

nicolasstucki merged commit 232b3fa into scala:master Jul 26, 2019

nicolasstucki deleted the define-TupleXXL-using-IArray branch July 26, 2019 14:50

Define TupleXXL and FunctionXXL using IArray #6929

Define TupleXXL and FunctionXXL using IArray #6929

Uh oh!

Conversation

nicolasstucki commented Jul 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liufengyun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki commented Jul 26, 2019

Uh oh!

nicolasstucki commented Jul 26, 2019

Uh oh!

liufengyun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicolasstucki commented Jul 24, 2019 •

edited

Loading