Emit mixin forwarders by default?

[smarter]

This is what Scala 2.12 does, for startup performance reasons (http://www.scala-lang.org/blog/2016/07/08/trait-method-performance.html). Currently we're only doing it for JUnit-annotated methods (#2557).
/cc @DarkDimius

[DarkDimius]

@smarter, I still believe that the results of this blog-post are not true for jit-ed code and we should perform a more close study.

[DarkDimius]

While this may help startup time, but I feel this would only worsen peak performance

[smarter]

scala/scala#5429 says:

• Startup is slower without forwarders
• For a large program with a short running time (compiling a small file) the difference can be significant (> 40%)
• Hot performance is not affected
• There are probably multiple underlying causes (CHA, populating vtables), we don't have a clear picture

And also:

We could not identify any performance issues due to having two forwarders. Note that the JVM will unconditionally inline small methods (< 35 bytes) without querying any heuristics, the forwarders fall into this category.

[DarkDimius]

Citing your citation:

we don't have a clear picture

I don't think we should make a decision here before having a clear picture. The picture that is desciben in the blogpost i believe is not what happens in JVM.

And, to cite the update to this blogpost:

The micro-benchmark analyzed in this blog post is not the root cause of the performance regression observed when running the Scala compiler. It just shows one example where the use of default methods can have unexpected consequences on performance.

the 20% slowdown was due to different reason.

[lrytz]

I totally agree that the discussion in the blog post cannot be related to what happens to the compiler performance. However, the decision to merge scala/scala#5429 was not based on the blog post, but on the benchmarks shown in the PR discussion: scala/scala#5429 (comment) and later comments

[DarkDimius]

@lrytz, thanks for the link.

I think that we should NOT do this, as this can hide correctness bugs. We should make sure that our predicates on when to emmit forwarders for correctess is exercised.
When we'll have a stable release, we should reconsider. By that time jdk9 is likely to ship and a lot of fixes for performance of default methods will be in.

[smarter]

When we'll have a stable release, we should reconsider

Well, hopefully before we have a stable release because switching -Xmixin-force-forwarders on/off is not serialization-compatible (cf discussion in scala/scala#5429).

I still believe that the results of this blog-post are not true for jit-ed code and we should perform a more close study.

What kind of "close study" do you have in mind? If we run http://github.com/dotty-staging/compiler-benchmark and see that mixin forwarders improve cold performance without impacting hot performance, would that be enough?

I think that we should NOT do this, as this can hide correctness bugs. We should make sure that our predicates on when to emmit forwarders for correctess is exercised.

One easy way to work around this is to run the tests twice, with mixing forwarders on or off.

[DarkDimius]

Also, our startup time won't be affected much.
Afaik the milestone release is going to use scala.collections.* compiled by scalac.
When we'll be preparing a bootstrapped stable release we should reconsider.

What kind of "close study" do you have in mind?

I think we should understand how it works, not try to guess it. @lrytz and scalac-team tried to do this in the linked PR, but studies of that kind are always hard to perform, due to debug versions of JVM behaving very differently performance-wise. The right discussion seems to be here: scala/scala-dev#243, we should instrument the optimized build of hotspot to see if our assumptions are correct.

I think that we should NOT do this, as this can hide correctness bugs. We should make sure that our predicates on when to emmit forwarders for correctess is exercised.

One easy way to work around this is to run the tests twice, with mixing forwarders on or off.

While I agree, I don't recall triggering bugs by emitting more forwarders rather then less.