Reduce frequency of checkcast in bytecode

[scabug]

When we use "def = new { ... }",
the generated bytecode appears to have too little type information, and too many casts. Same for "val", and for "new with { ... }"

Here's a minimal example:-

trait B

trait C

final class Test {
    def d1 = new B {} // return type java.lang.Object in bytecode
    def d3: B = new B {}

    val v1 = d1 // java.lang.Object in bytecode
    val v2: B = d1 // uses checkcast
    val v3: B = d3

    def f(x: B) {}

    f(v1) // uses checkcast
    f(v2)
    f(v3)
}

// If we change "B" to "B with C", the java.lang.Object and checkcast still occur

A real-world example is the trait Iterator, and the Iterator members

    val empty
    def single
    def range
    def append
    def ++
    def zip
    def zipWithIndex

None of these explicitly declare the return type, so java.lang.Object and checkcast occur.

Here's an informal benchmark that shows the impact this can have on performance:

object T7 {
    def f(i: Iterator[_]) = i.hasNext

    def main(args: Array[String]) {

        val empty = Iterator.empty // checkcast occurs within loop, program takes 26s
        // val empty: Iterator[_] = Iterator.empty // checkcast outside loop, program takes 22s

        for (i <- 1 to 1000000000) {
            f(empty)
        }
    }
}

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-1377?orig=1
Reporter: Eric Willigers (ewilligers)

[scabug]

@ijuma said:
It's generally a bad idea to write benchmarks that are whole contained in the main method because JITs can't optimise it properly (it requires on-stack replacement). Furthermore, that test causes a lot of allocation due to the usage of Range with large values (and subsequent GC). The casts themselves are a small part of the benchmark in the end.

Here's an improved version. Still, given how optimized casts are, it's perfectly possible that the code in the benchmark to prevent HotSpot from optimising the method call completely dominates the benchmark. Also, there's the risk that the f call could be optimised completely.

  def f(i: Iterator[_]) = i.hasNext

    def main(args: Array[String]) {
      test()
    }
    
    def test() {
      var i = 0
      while (i < 10) {
        val time = System.currentTimeMillis
        val result = inner()
        i += 1
        println("Time: " + (System.currentTimeMillis - time))
        println("Value: " + result)
      }
    }
    
    def inner() = {
      //val empty = Iterator.empty // checkcast occurs within loop
      val empty: Iterator[_] = Iterator.empty // checkcast outside loop
      var i = 0L
      while (i < 10000000000L) {
        f(empty)
        i += 1
      }
      i
    }
}

Using the version with the cast and without took the same time in my machine. Interestingly, both versions take about 4.8s after the initial JIT of the inner method, but from the 3rd iteration onwards (when the JIT recompiles the inner method) they both take 7.1s. Seems like HotSpot tries to optimise the code a bit more, but makes it worse. At any rate, both versions perform the same.

[scabug]

@ijuma said:
Having said the above, I think this bug has merit, not because of the removal of casts, but because the current system makes it very easy to break clients (yes, I know Scala isn't very good about binary compatibility, but it's still worth trying to improve it where possible). In fact something similar to this happened in the 2.7.1-rc1 release in Stream.apply:

http://www.nabble.com/FYI:-Scala-2.7.2.RC1-regression-td19080798.html

[scabug]

@ijuma said:
(Sorry for the noise, removed cc by mistake)

[scabug]

@lrytz said:
if somebody can show a significant performance gain, this might a reason to special case as proposed.

[scabug]

Ricky Clarkson (ricky_clarkson) said:
It would be good if def foo = new Bar { ... } was visible to Java as Bar, instead of java.lang.Object. I need to write def foo: Bar = new Bar { ... } to make that happen.

[scabug]

@ijuma said:
For reference, here's one way it could work according to Geoffrey:

http://www.nabble.com/Is-there-a-memory-leak-in-2.7.2-Actors--td20949656i20.html#a20985898

[scabug]

@odersky said:
I think this is fixed now with my recent changes to erasure.