Consider eliminating static accessors for trait methods

[lrytz]

Preparing my scala world talk, I realized that the solution we currently have for emitting super calls to trait members is possibly not the best trade-off.

In RC1, for every non-private trait method (except $init$, which is a static method only), we emit a static method that forwards to the default method. This is to support arbitrary super calls. This was originally added in https://github.com/scala/scala/pull/5177.

At the time, we were still emitting mixin forwarders for every method that a class inherits from a trait (corresponding to -Xmixin-force-forwarders:true in RC1). This resulted in a large number of super calls to trait methods that were implemented using invokespecial, which required adding the interface class as a direct parent to the class. The new scheme (using the static accessors) significantly reduced the number of implemented interfaces, which (hopefully) had a good effect on performance: Jason observed that adding redundant interfaces increases the class loading time significantly (https://github.com/scala/scala-dev/issues/98#issuecomment-194602923).

However, three things have changed in the meantime:

1. Trait $init$ methods are always emitted static, so they can always be invoked without adding the interface as direct parent. This was different at the time (they were instance methods).
2. RC1 only emits mixin forwarder methods when required for Scala's semantics (https://github.com/scala/scala/pull/5085), which is very rare. The forwarders were the other common source of super calls. (Even if we re-introduce the forwarders, adding the interface as direct parent is often not needed thanks to the next point. Numbers on that below).
3. We realized that adding an interface as parent can be avoided in certain situations by using one of the class's existing interfaces as receiver in the invokespecial call.

Example for the last point:

trait T { def f = 1 }
trait U extends T
class C extends U { def t = super.f }

Here, emitting invokespecial U.f is correct, we don't need to add T as a direct parent of C.

For comparison, here's an example where adding an interface as parent is required:

trait T1 { def f = 1 }
trait T2 { self: T1 => override def f = 2 }
trait U extends T1 with T2
class C extends U

The generated mixin forwarder in C invokes T2.f. Using invokespecial U.f would not work, as U inherits two implementations for f from two interfaces where none is a subtype of the other. So we need to add T2 to Cs interfaces.

To get some numbers, I went ahead and eliminated the static accessors, and re-introduced the logic to add additional interfaces to classfiles. The numbers confirm my expectations (adding an interface is rare) for the compiler codebase, not so much for the library because of the collection library's extremely deep inheritance hierarchy. Some numbers:

	rewrite OK	add itf total	add itf	add itf (JLO)	add itf (minParents)	add itf (super acc)
library	35	392	121	156	6	109
reflect	460	14	2	4	0	8
compiler	123	31	10	15	0	6

Meanings:

• "rewrite OK": an existing direct parent interface could be used to implement the call (like in the example above)
• "add itf (JLO)": the method has a definition in java.lang.Object (hashCode, toString, equals). invokespecial U.hashCode where U is not the interface holding the definition of hashCode (but a parent interface of U) would resolve to Object.hashCode. So we have to add the actual interface as parent.
• "add itf (minParents)": the interface needed to be added to the class, but in fact the class was originally extending that interface (in source). The interface was eliminated by minimizeParents.
• "add itf (super acc)": the interface needed to be added. the method in which the super call occurs is a super accessor (this case is quite common in the collections library).

The data is here: https://1drv.ms/x/s!AgqXX83j_zSUcYDOIFgboc_L7G8

I also compiled everything with -Xmixin-force-forwarders:true, here's the data for this case:

	rewrite OK	add itf total	add itf	add itf (JLO)	add itf (minParents)	add itf (super acc)
library-force	2594	925	686	98	32	109
reflect-force	884	45	34	2	1	8
compiler-force	1244	69	44	15	4	6

Observations

• The majority of non-necessary forwarders (forced by -Xmixin-force-forwarders:true) can be implemented using a "rewrite" to an existing direct parent interface
• The collections library with its jungle of traits requires adding a significant number of interfaces

WIP branch here: https://github.com/scala/scala/compare/2.12.0...lrytz:traitSuperAccessors?expand=1. Note that it contains the printlns to get the statistics summarized above.

Another note: if we re-introduce to logic to add additional interfaces to classes, we can lift the current restriction on super calls to java-defined default methods.

[lrytz]

cc @retronym @adriaanm @DarkDimius - let me know what you think. I'll also prepare a comparison of .jar file size (the static methods are gone, but probably it doesn't make a significant difference).

[retronym]

What about cases like:

class C { def foo = "C" }
trait T extends C { override def foo = "T" }
class D extends C { override def foo = "D" }
class E extends D with T { super[T].foo }

AFAICT we'd need to outlaw that super[T].foo as invokespecial will always favour an implementation in a superclass.

[lrytz]

No, this works as expected:

$ cat A.scala
class C { def foo = "C" }
trait T extends C { override def foo = "T" }
class D extends C { override def foo = "D" }
class E extends D with T { def t = super[T].foo }

object Test {
  def main(args: Array[String]): Unit = {
    println(new E().t)
  }
}
$ qsc A.scala && qs Test
T

Method lookup for invokespecial only starts at the superclass if the receiver in the instruction is one of the superclasses of the current class. Here we get INVOKESPECIAL T.foo in class E, which resolves to the correct method.

[lrytz]

We decided to keep static accessors.

Still to do: run compiler benchmarks combinations of

• -Xmixin-force-forwarders:true / false
• static super accessors yes / no (https://github.com/scala/scala/pull/5415)

In particular, if we enable have both -Xmixin-force-forwarders:true and the static super accessors, every call to a trait method will go through two forwarders. We should make sure this doesn't hurt performance.

trait T { def f = 1 }
class C extends T { def g = f }

gives

public class C implements T {
    public int f() { return T.f$(this); }
    public int g() { return this.f(); }
}
public interface T {
    default public static /* synthetic */ int f$(T $this) { return $this.f(); }
    default public int f() { return 1; }
}

[adriaanm]

So, maybe it's better not to enable -Xmixin-force-forwarders:true? The small performance hit (10%) in cold performance may one day be resolved by the JVM, or it may not be as bad for many code bases? You could also argue our cold performance has improved thanks to indy lambda, so we have some margin to squander performance :-)