Reduce construction of ArrayOps.ofX

[rorygraves]

We have lots of calls to

traced.csv:972349,scala.collection.mutable.ArrayOps$ofRef and ArrayOps.ofChar

This allow for extension methods such as .length .isDefinedAt

Each usage could be removed, but it would be better if there was a rewrite rule or similar that turns.

foo.length ( becomes ArrayOps.ofRef(foo).length Into
ArrayOpsStatic.lengthOfRef(foo)
avoiding object creation Where/how this might best be implemented is an open question.

[mkeskells]

972349 allocations?

[rorygraves]

I think thats calla - I see 5.6M calls to WrappedArray constructor: 5650115,scala.collection.mutable.WrappedArray$ofRef::()V

[rorygraves]

The numbers don't quite tie up for me mentally, but this is definitely a target.

[mkeskells]

empty ones are cached I think - although that may be lost in the reverted code

[retronym]

This is a limitation of value classes: invocations of methods inherited from universal traits still incurs boxing.

scala> trait T extends Any { def tee = "t" }; class C(val self: Any) extends AnyVal with T { def cee = "c"};
defined trait T
defined class C

scala> :paste -raw
// Entering paste mode (ctrl-D to finish)

trait T extends Any { def tee = "t" }; class C(val self: Any) extends AnyVal with T { def cee = "c"};
class ValueClassTest { def test(c: C) = { c.cee; c.tee } }

// Exiting paste mode, now interpreting.

scala> :javap -c ValueClassTest
Compiled from "<pastie>"
public class ValueClassTest {
  public java.lang.String test(java.lang.Object);
    Code:
       0: getstatic     #17                 // Field C$.MODULE$:LC$;
       3: aload_1
       4: invokevirtual #20                 // Method C$.cee$extension:(Ljava/lang/Object;)Ljava/lang/String;
       7: pop
       8: new           #22                 // class C
      11: dup
      12: aload_1
      13: invokespecial #26                 // Method C."<init>":(Ljava/lang/Object;)V
      16: invokevirtual #30                 // Method C.tee:()Ljava/lang/String;
      19: areturn

  public ValueClassTest();
    Code:
       0: aload_0
       1: invokespecial #36                 // Method java/lang/Object."<init>":()V
       4: return
}

It would be worth checking if you actually see ArrayOps$ofRef in the allocation stats recorded by flight recorder, as by instrumenting its constructor you will be disabling escape analysis. I'm not sure if that typically kicks in in this case or not.

[retronym]

If you want to have a dense, mutable collection that you keep around, another option is to convert once to a WrappedArray, and perform operations on that, rather than always going through ArrayOps.

[mkeskells]

Looks like 2.13 will avoid some/all of these conversions - need to verify though

[ackratos]

Verified, this has been avoided in 2.13:

For same piece of code:

package scala.collection.immutable

class Foo(val id: String) {}

object Test extends App {
  val foo1 = new Foo("1")
  val foo2 = new Foo("2")
  val foo3 = new Foo("3")
  val foos = Array(foo1, foo2, foo3)
  val newSlice = foos.slice(0,2)
  println(newSlice)
}

In scala 2.12: The call to foos.slice would involve constructor of ofRef class:

71: putfield      #83                 // Field foos:[Lscala/collection/immutable/Foo;
        74: aload_0
        75: new           #14                 // class scala/collection/mutable/ArrayOps$ofRef
        78: dup
        79: getstatic     #111                // Field scala/Predef$.MODULE$:Lscala/Predef$;
        82: aload_0
        83: invokevirtual #113                // Method foos:()[Lscala/collection/immutable/Foo;
        86: checkcast     #105                // class "[Ljava/lang/Object;"
        89: invokevirtual #117                // Method scala/Predef$.refArrayOps:([Ljava/lang/Object;)[Ljava/lang/Object;
        92: invokespecial #120                // Method scala/collection/mutable/ArrayOps$ofRef."<init>":([Ljava/lang/Object;)V
        95: iconst_0
        96: iconst_1
        97: invokevirtual #124                // Method scala/collection/mutable/ArrayOps$ofRef.slice:(II)Ljava/lang/Object;

But in 2.13:

75: getstatic     #106                // Field scala/collection/ArrayOps$.MODULE$:Lscala/collection/ArrayOps$;
        78: getstatic     #111                // Field scala/Predef$.MODULE$:Lscala/Predef$;
        81: aload_0
        82: invokevirtual #113                // Method foos:()[Lscala/collection/immutable/Foo;
        85: checkcast     #100                // class "[Ljava/lang/Object;"
        88: invokevirtual #117                // Method scala/Predef$.refArrayOps:([Ljava/lang/Object;)Ljava/lang/Object;
        91: iconst_0
        92: iconst_2
        93: invokevirtual #121                // Method scala/collection/ArrayOps$.slice$extension:(Ljava/lang/Object;II)Ljava/lang/Object;
        96: checkcast     #101                // class "[Lscala/collection/immutable/Foo;"
        99: putfield      #80                 // Field newSlice:[Lscala/collection/immutable/Foo;
       102: getstatic     #111                // Field scala/Predef$.MODULE$:Lscala/Predef$;
       105: aload_0
       106: invokevirtual #123                // Method newSlice:()[Lscala/collection/immutable/Foo;

We can see that in 2.13 ArrayOps$.slice$extension is invoked directly without construction of an ArrayOps class. This is exact semantic (benefit) of value class.

@mkeskells I think we can close this issue:)

[mkeskells]

agreed