New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default -Xmixin-force-forwarders to true #5429

Merged
merged 5 commits into from Oct 12, 2016

Conversation

Projects
None yet
4 participants
@lrytz
Member

lrytz commented Sep 28, 2016

Also eliminates the warning when a mixin forwarder cannot be implemented because the target method is a java-defined default method in an interface that is not a direct parent of the class.

Adds some test cases, and fixes scala/scala-dev#224.

Discussion Summary

Since #5085, classes inheriting default methods no longer get a forwarder method that delegates to the default method (except in a few corner cases, like trait methods overriding class methods). The JVMs method resolution will pick the correct method.

This PR reverts that change.

We realized that missing forwarders cause a performance regression in startup time (www.scala-lang.org/blog/2016/07/08/trait-method-performance.html). Further benchmark numbers are available in comments in this PR. In summary:

  • 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

Two Forwarders

When trait forwarders are added, invoking a trait method involves indirection through two forwarders:

trait T { def f = 1 }
class A extends T   // generates `def f = super[T].f`

generates

class A implements T {
    public int f() { return T.f$(this); }
}
interface T {
    default public static int f$(T $this) { return $this.f(); }

    default public int f() { return 1; }
}

The reason for having a static accessor method (f$) and using it for the super calls is summarized in scala/scala-dev#228.

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.

Serialization Stability

If a class does not have an explicit @SerialVersionUID, it is computed by the VM. The computed value is sensitive to changes in the classfile that don't affect deserialization, for example adding a public method.

This means that we cannot change our strategy for mixin forwarders in a 2.12.x minor release, as it would affect serialization compatibility. So even if the performance issue with missing forwarders is fixed in a future JVM update, we need to keep the forwarders. We can re-consider for 2.13.

See also scala/scala-dev#237.

@scala-jenkins scala-jenkins added this to the 2.12.0-RC2 milestone Sep 28, 2016

lrytz added some commits Sep 28, 2016

Error message for super calls to indirect java parent interfaces
Super calls to indirect java parent interfaces cannot be emitted, an
error message is emitted during SuperAccessors.

The error message was missing if the super call was non-qualified,
resulting in an assertion failure in the backend.
@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Sep 30, 2016

Member

Test failure due to a changed serialVersionUID of scala.reflect.ClassTag$GenericClassTag (there's no explicit value, the generated one changes due to the addition of mixin forwarders). The test was added in #3711 for SI-8549 to ensure serialization stability.

Looking at the class hierarchy around ClassTag and Manifest, it seems the only class that has a serialVersionUID is AnyValManifest, where the hierarchy is something like:

trait ClassTag                      // extends Serializable
|- class GenericClassTag
|- trait Manifest
   |- class ClassTypeManifest
   |- class SingletonTypeManifest
   |- ...
   |- abstract class AnyValManifest // has @SerialVersionUID
      |- class DoubleManifest
      |- ...

A comment in #3711 says

the annotation [@SerialVersionUID] can be defined up the class hierarchy

I think this comment is not correct (or it means something else than I think it means):

$ cat Test.scala
@SerialVersionUID(1L)
class A extends Serializable
class B extends A

$ scalac Test.scala && serialver -classpath .:/Users/luc/scala/scala-2.12.0-RC1/lib/scala-library.jar B
B:    private static final long serialVersionUID = -4759527637665705469L;

$ cat Test.scala
@SerialVersionUID(1L)
class A extends Serializable
class B extends A { def f = 1 }

$ scalac Test.scala && serialver -classpath .:/Users/luc/scala/scala-2.12.0-RC1/lib/scala-library.jar B
B:    private static final long serialVersionUID = -6380942902981420110L;

The @SerialVersionUID annotation on AnyValManifest was added in the same PR (#3711 / e7ab41e), probably with the intention to have a stable value for all subclasses.

I think the right solution here is to add explicit @SerialVersionUID annotations to all concrete Manifest / ClassTag subclasses.

Member

lrytz commented Sep 30, 2016

Test failure due to a changed serialVersionUID of scala.reflect.ClassTag$GenericClassTag (there's no explicit value, the generated one changes due to the addition of mixin forwarders). The test was added in #3711 for SI-8549 to ensure serialization stability.

Looking at the class hierarchy around ClassTag and Manifest, it seems the only class that has a serialVersionUID is AnyValManifest, where the hierarchy is something like:

trait ClassTag                      // extends Serializable
|- class GenericClassTag
|- trait Manifest
   |- class ClassTypeManifest
   |- class SingletonTypeManifest
   |- ...
   |- abstract class AnyValManifest // has @SerialVersionUID
      |- class DoubleManifest
      |- ...

A comment in #3711 says

the annotation [@SerialVersionUID] can be defined up the class hierarchy

I think this comment is not correct (or it means something else than I think it means):

$ cat Test.scala
@SerialVersionUID(1L)
class A extends Serializable
class B extends A

$ scalac Test.scala && serialver -classpath .:/Users/luc/scala/scala-2.12.0-RC1/lib/scala-library.jar B
B:    private static final long serialVersionUID = -4759527637665705469L;

$ cat Test.scala
@SerialVersionUID(1L)
class A extends Serializable
class B extends A { def f = 1 }

$ scalac Test.scala && serialver -classpath .:/Users/luc/scala/scala-2.12.0-RC1/lib/scala-library.jar B
B:    private static final long serialVersionUID = -6380942902981420110L;

The @SerialVersionUID annotation on AnyValManifest was added in the same PR (#3711 / e7ab41e), probably with the intention to have a stable value for all subclasses.

I think the right solution here is to add explicit @SerialVersionUID annotations to all concrete Manifest / ClassTag subclasses.

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Sep 30, 2016

Member

LGTM

Member

adriaanm commented Sep 30, 2016

LGTM

Explicit SerialVersionUID for all ClassTags / Manifests
Looking at the class hierarchy around ClassTag and Manifest, the only
class that had a serialVersionUID is AnyValManifest, where the hierarchy
is something like:

trait ClassTag                      // extends Serializable
|- class GenericClassTag
|- trait Manifest
   |- class ClassTypeManifest
   |- class SingletonTypeManifest
   |- ...
   |- abstract class AnyValManifest // has SerialVersionUID
      |- class DoubleManifest
      |- ...

Note that AnyValManifest is an abstract class, so the SerialVersionUID
annotation does not help there.

This commit adds explicit SerialVersionUID annotations to (hopefully)
all subclasses of ClassTag, to make sure they are stable under
compatible changes (such as changing -Xmixin-force-forwarders).
@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Sep 30, 2016

Member

The SerialVersionUID test failed for Set$EmptySet$ now. I patched it up, but the way we handle this annotation in the std lib is really ad-hoc..

Member

lrytz commented Sep 30, 2016

The SerialVersionUID test failed for Set$EmptySet$ now. I patched it up, but the way we handle this annotation in the std lib is really ad-hoc..

lrytz added some commits Sep 30, 2016

Default -Xmixin-force-forwarders to true
Also eliminates the warning when a mixin forwarder cannot be implemented
because the target method is a java-defined default method in an
interface that is not a direct parent of the class.

The test t5148 is moved to neg, as expected: It was moved to pos when
disabling mixin forwarders in 33e7106. Same for the changed error
message in t4749.
Test cases for super calls
Recovered and adapted some test cases for super calls from #5415
@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Sep 30, 2016

Member

rebased with update to t8549 (git diff ffc8e3e..0e0614c):

diff --git a/test/files/run/t8549.scala b/test/files/run/t8549.scala
index 2bf648f..7ec3635 100644
--- a/test/files/run/t8549.scala
+++ b/test/files/run/t8549.scala
@@ -79,7 +79,7 @@ object Test extends App {
     }
   }

-  // Generated on 20160930-19:54:01 with Scala version 2.12.0-local-d86377e)
+  // Generated on 20160930-16:09:23 with Scala version 2.12.0-local-ffc8e3e)
   overwrite.foreach(updateComment)

   check(Some(1))("rO0ABXNyAApzY2FsYS5Tb21lESLyaV6hi3QCAAFMAAV2YWx1ZXQAEkxqYXZhL2xhbmcvT2JqZWN0O3hyAAxzY2FsYS5PcHRpb27+aTf92w5mdAIAAHhwc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAE=")
@@ -163,7 +163,7 @@ object Test extends App {
   // TODO SI-8576 Uninitialized field: IndexedSeqLike.scala: 56
   // check(immutable.Stream(1, 2, 3))( "rO0ABXNyACZzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5TdHJlYW0kQ29uc/ekjBXM3TlFAgADTAACaGR0ABJMamF2YS9sYW5nL09iamVjdDtMAAV0bEdlbnQAEUxzY2FsYS9GdW5jdGlvbjA7TAAFdGxWYWx0ACNMc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvU3RyZWFtO3hyACFzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5TdHJlYW0552RDntM42gIAAHhwc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFzcgAtc2NhbGEuY29sbGVjdGlvbi5JdGVyYXRvciQkYW5vbmZ1biR0b1N0cmVhbSQxRWR4We0SX0UCAAFMAAYkb3V0ZXJ0ABtMc2NhbGEvY29sbGVjdGlvbi9JdGVyYXRvcjt4cHNyAChzY2FsYS5jb2xsZWN0aW9uLkluZGV4ZWRTZXFMaWtlJEVsZW1lbnRzGF+1cBwmcx0CAANJAANlbmRJAAVpbmRleEwABiRvdXRlcnQAIUxzY2FsYS9jb2xsZWN0aW9uL0luZGV4ZWRTZXFMaWtlO3hwAAAAAwAAAAFzcgArc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLldyYXBwZWRBcnJheSRvZkludMmRLBcI15VjAgABWwAFYXJyYXl0AAJbSXhwdXIAAltJTbpgJnbqsqUCAAB4cAAAAAMAAAABAAAAAgAAAANw")

-  check(immutable.TreeSet[Int]())(   "rO0ABXNyACJzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5UcmVlU2V0sRdVIDjbWAsCAAJMAAhvcmRlcmluZ3QAFUxzY2FsYS9tYXRoL09yZGVyaW5nO0wABHRyZWV0AC5Mc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHA=")
+  check(immutable.TreeSet[Int]())(   "rO0ABXNyACJzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5UcmVlU2V0sRdVIDjbWAsCAAJMAAhvcmRlcmluZ3QAFUxzY2FsYS9tYXRoL09yZGVyaW5nO0wABHRyZWV0AC5Mc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHA=")

   // TODO SI-8576 unstable under -Xcheckinit
   // check(immutable.TreeSet(1, 2, 3))( "rO0ABXNyACJzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5UcmVlU2V0sRdVIDjbWAsCAAJMAAhvcmRlcmluZ3QAFUxzY2FsYS9tYXRoL09yZGVyaW5nO0wABHRyZWV0AC5Mc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkC4BMdr1Z51wCAAB4cHNyADFzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5SZWRCbGFja1RyZWUkQmxhY2tUcmVlzRxnCKenVAECAAB4cgAsc2NhbGEuY29sbGVjdGlvbi5pbW11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWVrqCSyHJbsMgIABUkABWNvdW50TAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgACTAAFcmlnaHRxAH4AAkwABXZhbHVlcQB+AAh4cAAAAANzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAnNxAH4ABgAAAAFzcQB+AAoAAAABcHBzcgAXc2NhbGEucnVudGltZS5Cb3hlZFVuaXR0pn1HHezLmgIAAHhwc3EAfgAGAAAAAXNxAH4ACgAAAANwcHEAfgAQcQB+ABA=")
@@ -179,12 +179,12 @@ object Test extends App {
   check(mutable.HashMap())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGFzaE1hcAAAAAAAAAABAwAAeHB3DQAAAu4AAAAAAAAABAB4")
   check(mutable.HashMap(1 -> 1))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGFzaE1hcAAAAAAAAAABAwAAeHB3DQAAAu4AAAABAAAABABzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAXEAfgAEeA==")
   check(mutable.HashSet(1, 2, 3))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGFzaFNldAAAAAAAAAABAwAAeHB3DQAAAcIAAAADAAAABQBzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAXNxAH4AAgAAAAJzcQB+AAIAAAADeA==")
-  check(mutable.TreeMap[Int, Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
-  check(mutable.TreeMap(1 -> 1, 3 -> 6))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHNxAH4ADAAAAAZxAH4ADg==")
-  check(mutable.TreeMap(1 -> 1, 3 -> 6).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcCRUcmVlTWFwVmlldx7MCZxLhVQ8AgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlTWFwO0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVNYXDcfKgttvWb8AIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVNYXAkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JCk2+Jz+mgKqAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBzcQB+AA8AAAAGcQB+ABFzcQB+AANxAH4ACHEAfgALc3IACnNjYWxhLlNvbWURIvJpXqGLdAIAAUwABXZhbHVlcQB+AA14cgAMc2NhbGEuT3B0aW9u/mk3/dsOZnQCAAB4cHEAfgARc3EAfgAWc3EAfgAPAAAAAg==")
-  check(mutable.TreeSet[Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
-  check(mutable.TreeSet(1, 3))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHBw")
-  check(mutable.TreeSet(1, 3).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldCRUcmVlU2V0Vmlld2JdAzqy0DpGAgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlU2V0O0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVTZXTNdJ8RUA6beAIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVTZXQkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JCk2+Jz+mgKqAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBwcHNxAH4AA3EAfgAIcQB+AAtzcgAKc2NhbGEuU29tZREi8mleoYt0AgABTAAFdmFsdWVxAH4ADXhyAAxzY2FsYS5PcHRpb27+aTf92w5mdAIAAHhwcQB+ABFzcQB+ABVzcQB+AA8AAAAC")
+  check(mutable.TreeMap[Int, Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
+  check(mutable.TreeMap(1 -> 1, 3 -> 6))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHNxAH4ADAAAAAZxAH4ADg==")
+  check(mutable.TreeMap(1 -> 1, 3 -> 6).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcCRUcmVlTWFwVmlldx7MCZxLhVQ8AgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlTWFwO0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVNYXDcfKgttvWb8AIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVNYXAkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JPLu3IK7lc7nAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBzcQB+AA8AAAAGcQB+ABFzcQB+AANxAH4ACHEAfgALc3IACnNjYWxhLlNvbWURIvJpXqGLdAIAAUwABXZhbHVlcQB+AA14cgAMc2NhbGEuT3B0aW9u/mk3/dsOZnQCAAB4cHEAfgARc3EAfgAWc3EAfgAPAAAAAg==")
+  check(mutable.TreeSet[Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
+  check(mutable.TreeSet(1, 3))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHBw")
+  check(mutable.TreeSet(1, 3).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldCRUcmVlU2V0Vmlld2JdAzqy0DpGAgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlU2V0O0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVTZXTNdJ8RUA6beAIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVTZXQkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JPLu3IK7lc7nAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBwcHNxAH4AA3EAfgAIcQB+AAtzcgAKc2NhbGEuU29tZREi8mleoYt0AgABTAAFdmFsdWVxAH4ADXhyAAxzY2FsYS5PcHRpb27+aTf92w5mdAIAAHhwcQB+ABFzcQB+ABVzcQB+AA8AAAAC")
   // TODO SI-8576 Uninitialized field under -Xcheckinit
   // check(new mutable.History())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGlzdG9yeUhuXxDIFJrsAgACSQAKbWF4SGlzdG9yeUwAA2xvZ3QAIExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUXVldWU7eHAAAAPoc3IAHnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5RdWV1ZbjMURVfOuHHAgAAeHIAJHNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5NdXRhYmxlTGlzdFJpnjJ+gFbAAgADSQADbGVuTAAGZmlyc3QwdAAlTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9MaW5rZWRMaXN0O0wABWxhc3QwcQB+AAV4cAAAAABzcgAjc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLkxpbmtlZExpc3Sak+nGCZHaUQIAAkwABGVsZW10ABJMamF2YS9sYW5nL09iamVjdDtMAARuZXh0dAAeTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9TZXE7eHBwcQB+AApxAH4ACg==")
   check(mutable.LinkedHashMap(1 -> 2))( "rO0ABXNyACZzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuTGlua2VkSGFzaE1hcAAAAAAAAAABAwAAeHB3DQAAAu4AAAABAAAABABzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAXNxAH4AAgAAAAJ4")
Member

adriaanm commented Sep 30, 2016

rebased with update to t8549 (git diff ffc8e3e..0e0614c):

diff --git a/test/files/run/t8549.scala b/test/files/run/t8549.scala
index 2bf648f..7ec3635 100644
--- a/test/files/run/t8549.scala
+++ b/test/files/run/t8549.scala
@@ -79,7 +79,7 @@ object Test extends App {
     }
   }

-  // Generated on 20160930-19:54:01 with Scala version 2.12.0-local-d86377e)
+  // Generated on 20160930-16:09:23 with Scala version 2.12.0-local-ffc8e3e)
   overwrite.foreach(updateComment)

   check(Some(1))("rO0ABXNyAApzY2FsYS5Tb21lESLyaV6hi3QCAAFMAAV2YWx1ZXQAEkxqYXZhL2xhbmcvT2JqZWN0O3hyAAxzY2FsYS5PcHRpb27+aTf92w5mdAIAAHhwc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAE=")
@@ -163,7 +163,7 @@ object Test extends App {
   // TODO SI-8576 Uninitialized field: IndexedSeqLike.scala: 56
   // check(immutable.Stream(1, 2, 3))( "rO0ABXNyACZzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5TdHJlYW0kQ29uc/ekjBXM3TlFAgADTAACaGR0ABJMamF2YS9sYW5nL09iamVjdDtMAAV0bEdlbnQAEUxzY2FsYS9GdW5jdGlvbjA7TAAFdGxWYWx0ACNMc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvU3RyZWFtO3hyACFzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5TdHJlYW0552RDntM42gIAAHhwc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFzcgAtc2NhbGEuY29sbGVjdGlvbi5JdGVyYXRvciQkYW5vbmZ1biR0b1N0cmVhbSQxRWR4We0SX0UCAAFMAAYkb3V0ZXJ0ABtMc2NhbGEvY29sbGVjdGlvbi9JdGVyYXRvcjt4cHNyAChzY2FsYS5jb2xsZWN0aW9uLkluZGV4ZWRTZXFMaWtlJEVsZW1lbnRzGF+1cBwmcx0CAANJAANlbmRJAAVpbmRleEwABiRvdXRlcnQAIUxzY2FsYS9jb2xsZWN0aW9uL0luZGV4ZWRTZXFMaWtlO3hwAAAAAwAAAAFzcgArc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLldyYXBwZWRBcnJheSRvZkludMmRLBcI15VjAgABWwAFYXJyYXl0AAJbSXhwdXIAAltJTbpgJnbqsqUCAAB4cAAAAAMAAAABAAAAAgAAAANw")

-  check(immutable.TreeSet[Int]())(   "rO0ABXNyACJzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5UcmVlU2V0sRdVIDjbWAsCAAJMAAhvcmRlcmluZ3QAFUxzY2FsYS9tYXRoL09yZGVyaW5nO0wABHRyZWV0AC5Mc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHA=")
+  check(immutable.TreeSet[Int]())(   "rO0ABXNyACJzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5UcmVlU2V0sRdVIDjbWAsCAAJMAAhvcmRlcmluZ3QAFUxzY2FsYS9tYXRoL09yZGVyaW5nO0wABHRyZWV0AC5Mc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHA=")

   // TODO SI-8576 unstable under -Xcheckinit
   // check(immutable.TreeSet(1, 2, 3))( "rO0ABXNyACJzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5UcmVlU2V0sRdVIDjbWAsCAAJMAAhvcmRlcmluZ3QAFUxzY2FsYS9tYXRoL09yZGVyaW5nO0wABHRyZWV0AC5Mc2NhbGEvY29sbGVjdGlvbi9pbW11dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkC4BMdr1Z51wCAAB4cHNyADFzY2FsYS5jb2xsZWN0aW9uLmltbXV0YWJsZS5SZWRCbGFja1RyZWUkQmxhY2tUcmVlzRxnCKenVAECAAB4cgAsc2NhbGEuY29sbGVjdGlvbi5pbW11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWVrqCSyHJbsMgIABUkABWNvdW50TAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgACTAAFcmlnaHRxAH4AAkwABXZhbHVlcQB+AAh4cAAAAANzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAnNxAH4ABgAAAAFzcQB+AAoAAAABcHBzcgAXc2NhbGEucnVudGltZS5Cb3hlZFVuaXR0pn1HHezLmgIAAHhwc3EAfgAGAAAAAXNxAH4ACgAAAANwcHEAfgAQcQB+ABA=")
@@ -179,12 +179,12 @@ object Test extends App {
   check(mutable.HashMap())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGFzaE1hcAAAAAAAAAABAwAAeHB3DQAAAu4AAAAAAAAABAB4")
   check(mutable.HashMap(1 -> 1))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGFzaE1hcAAAAAAAAAABAwAAeHB3DQAAAu4AAAABAAAABABzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAXEAfgAEeA==")
   check(mutable.HashSet(1, 2, 3))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGFzaFNldAAAAAAAAAABAwAAeHB3DQAAAcIAAAADAAAABQBzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAXNxAH4AAgAAAAJzcQB+AAIAAAADeA==")
-  check(mutable.TreeMap[Int, Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
-  check(mutable.TreeMap(1 -> 1, 3 -> 6))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHNxAH4ADAAAAAZxAH4ADg==")
-  check(mutable.TreeMap(1 -> 1, 3 -> 6).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcCRUcmVlTWFwVmlldx7MCZxLhVQ8AgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlTWFwO0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVNYXDcfKgttvWb8AIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVNYXAkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JCk2+Jz+mgKqAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBzcQB+AA8AAAAGcQB+ABFzcQB+AANxAH4ACHEAfgALc3IACnNjYWxhLlNvbWURIvJpXqGLdAIAAUwABXZhbHVlcQB+AA14cgAMc2NhbGEuT3B0aW9u/mk3/dsOZnQCAAB4cHEAfgARc3EAfgAWc3EAfgAPAAAAAg==")
-  check(mutable.TreeSet[Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
-  check(mutable.TreeSet(1, 3))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQkKTb4nP6aAqoCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHBw")
-  check(mutable.TreeSet(1, 3).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldCRUcmVlU2V0Vmlld2JdAzqy0DpGAgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlU2V0O0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVTZXTNdJ8RUA6beAIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVTZXQkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JCk2+Jz+mgKqAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBwcHNxAH4AA3EAfgAIcQB+AAtzcgAKc2NhbGEuU29tZREi8mleoYt0AgABTAAFdmFsdWVxAH4ADXhyAAxzY2FsYS5PcHRpb27+aTf92w5mdAIAAHhwcQB+ABFzcQB+ABVzcQB+AA8AAAAC")
+  check(mutable.TreeMap[Int, Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
+  check(mutable.TreeMap(1 -> 1, 3 -> 6))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcNx8qC229ZvwAgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZU1hcCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHNxAH4ADAAAAAZxAH4ADg==")
+  check(mutable.TreeMap(1 -> 1, 3 -> 6).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZU1hcCRUcmVlTWFwVmlldx7MCZxLhVQ8AgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlTWFwO0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVNYXDcfKgttvWb8AIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVNYXAkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JPLu3IK7lc7nAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBzcQB+AA8AAAAGcQB+ABFzcQB+AANxAH4ACHEAfgALc3IACnNjYWxhLlNvbWURIvJpXqGLdAIAAUwABXZhbHVlcQB+AA14cgAMc2NhbGEuT3B0aW9u/mk3/dsOZnQCAAB4cHEAfgARc3EAfgAWc3EAfgAPAAAAAg==")
+  check(mutable.TreeSet[Int]())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAABw")
+  check(mutable.TreeSet(1, 3))( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldM10nxFQDpt4AgACTAAIb3JkZXJpbmd0ABVMc2NhbGEvbWF0aC9PcmRlcmluZztMACZzY2FsYSRjb2xsZWN0aW9uJG11dGFibGUkVHJlZVNldCQkdHJlZXQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJFRyZWU7eHBzcgAYc2NhbGEubWF0aC5PcmRlcmluZyRJbnQk8u7cgruVzucCAAB4cHNyACpzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuUmVkQmxhY2tUcmVlJFRyZWUATKc08DWmFQIAAkkABHNpemVMAARyb290dAAsTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9SZWRCbGFja1RyZWUkTm9kZTt4cAAAAAJzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSROb2RlGxHsFtValgACAAZaAANyZWRMAANrZXl0ABJMamF2YS9sYW5nL09iamVjdDtMAARsZWZ0cQB+AAdMAAZwYXJlbnRxAH4AB0wABXJpZ2h0cQB+AAdMAAV2YWx1ZXEAfgAKeHAAc3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAFwcHNxAH4ACQFzcQB+AAwAAAADcHEAfgALcHBw")
+  check(mutable.TreeSet(1, 3).range(1, 2))( "rO0ABXNyACxzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuVHJlZVNldCRUcmVlU2V0Vmlld2JdAzqy0DpGAgADTAAGJG91dGVydAAiTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9UcmVlU2V0O0wABGZyb210AA5Mc2NhbGEvT3B0aW9uO0wABXVudGlscQB+AAJ4cgAgc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlRyZWVTZXTNdJ8RUA6beAIAAkwACG9yZGVyaW5ndAAVTHNjYWxhL21hdGgvT3JkZXJpbmc7TAAmc2NhbGEkY29sbGVjdGlvbiRtdXRhYmxlJFRyZWVTZXQkJHRyZWV0ACxMc2NhbGEvY29sbGVjdGlvbi9tdXRhYmxlL1JlZEJsYWNrVHJlZSRUcmVlO3hwc3IAGHNjYWxhLm1hdGguT3JkZXJpbmckSW50JPLu3IK7lc7nAgAAeHBzcgAqc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLlJlZEJsYWNrVHJlZSRUcmVlAEynNPA1phUCAAJJAARzaXplTAAEcm9vdHQALExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUmVkQmxhY2tUcmVlJE5vZGU7eHAAAAACc3IAKnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5SZWRCbGFja1RyZWUkTm9kZRsR7BbVWpYAAgAGWgADcmVkTAADa2V5dAASTGphdmEvbGFuZy9PYmplY3Q7TAAEbGVmdHEAfgAKTAAGcGFyZW50cQB+AApMAAVyaWdodHEAfgAKTAAFdmFsdWVxAH4ADXhwAHNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAABcHBzcQB+AAwBc3EAfgAPAAAAA3BxAH4ADnBwcHNxAH4AA3EAfgAIcQB+AAtzcgAKc2NhbGEuU29tZREi8mleoYt0AgABTAAFdmFsdWVxAH4ADXhyAAxzY2FsYS5PcHRpb27+aTf92w5mdAIAAHhwcQB+ABFzcQB+ABVzcQB+AA8AAAAC")
   // TODO SI-8576 Uninitialized field under -Xcheckinit
   // check(new mutable.History())( "rO0ABXNyACBzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuSGlzdG9yeUhuXxDIFJrsAgACSQAKbWF4SGlzdG9yeUwAA2xvZ3QAIExzY2FsYS9jb2xsZWN0aW9uL211dGFibGUvUXVldWU7eHAAAAPoc3IAHnNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5RdWV1ZbjMURVfOuHHAgAAeHIAJHNjYWxhLmNvbGxlY3Rpb24ubXV0YWJsZS5NdXRhYmxlTGlzdFJpnjJ+gFbAAgADSQADbGVuTAAGZmlyc3QwdAAlTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9MaW5rZWRMaXN0O0wABWxhc3QwcQB+AAV4cAAAAABzcgAjc2NhbGEuY29sbGVjdGlvbi5tdXRhYmxlLkxpbmtlZExpc3Sak+nGCZHaUQIAAkwABGVsZW10ABJMamF2YS9sYW5nL09iamVjdDtMAARuZXh0dAAeTHNjYWxhL2NvbGxlY3Rpb24vbXV0YWJsZS9TZXE7eHBwcQB+AApxAH4ACg==")
   check(mutable.LinkedHashMap(1 -> 2))( "rO0ABXNyACZzY2FsYS5jb2xsZWN0aW9uLm11dGFibGUuTGlua2VkSGFzaE1hcAAAAAAAAAABAwAAeHB3DQAAAu4AAAABAAAABABzcgARamF2YS5sYW5nLkludGVnZXIS4qCk94GHOAIAAUkABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwAAAAAXNxAH4AAgAAAAJ4")
@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 1, 2016

Member

Green now, i guess you just re-generated the test?

Member

lrytz commented Oct 1, 2016

Green now, i guess you just re-generated the test?

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 1, 2016

Member

Wait, you had to update the test in the "Default -Xmixin-force-forwarders to true" commit, which means that there's still some classes being tested by t8549 that change SVUID under -Xmixin-force-forwarders:ture. Instead, these classes should get the annotation.

Member

lrytz commented Oct 1, 2016

Wait, you had to update the test in the "Default -Xmixin-force-forwarders to true" commit, which means that there's still some classes being tested by t8549 that change SVUID under -Xmixin-force-forwarders:ture. Instead, these classes should get the annotation.

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 1, 2016

Member

So this time it would be Ordering$Int$ that should get the annotation it seems. Strangely I couldn't reproduce the failure locally (i did a local bootstrap). So yeah, not sure if we should go through more of the std lib and add the annotations everywhere..

This is a reminder that changing -Xmixin-force-forwarders, even though binary compatible, is not serialization compatible.

Member

lrytz commented Oct 1, 2016

So this time it would be Ordering$Int$ that should get the annotation it seems. Strangely I couldn't reproduce the failure locally (i did a local bootstrap). So yeah, not sure if we should go through more of the std lib and add the annotations everywhere..

This is a reminder that changing -Xmixin-force-forwarders, even though binary compatible, is not serialization compatible.

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Oct 1, 2016

Member

This is a reminder that changing -Xmixin-force-forwarders, even though binary compatible, is not serialization compatible.

Yep :-/ Do we still commit to having it on, knowing we can't turn it off?

Member

adriaanm commented Oct 1, 2016

This is a reminder that changing -Xmixin-force-forwarders, even though binary compatible, is not serialization compatible.

Yep :-/ Do we still commit to having it on, knowing we can't turn it off?

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 3, 2016

Member

I'm still in favor of the change. Do we have any guarantees in serialization compatibility? We have t8549, but it covers only a slice of the serializable classes in the sdt lib. For users, anyone relying on serialization compatibility across binary versions should be using @SVUID anyway.

Member

lrytz commented Oct 3, 2016

I'm still in favor of the change. Do we have any guarantees in serialization compatibility? We have t8549, but it covers only a slice of the serializable classes in the sdt lib. For users, anyone relying on serialization compatibility across binary versions should be using @SVUID anyway.

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Oct 3, 2016

Member

We don't explicitly promise it because we can't mechanically check, but we
have tended to shy away from making changes that could impact
serializability.

If the performance benefit is significant, I'm on board but I don't think
we can revert it later knowing the impact on serialization.
On Mon, Oct 3, 2016 at 04:52 Lukas Rytz notifications@github.com wrote:

I'm still in favor of the change. Do we have any guarantees in
serialization compatibility? We have t8549, but it covers only a slice of
the serializable classes in the sdt lib. For users, anyone relying on
serialization compatibility across binary versions should be using
@SVUID anyway.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#5429 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAFjy21nsCJf1x2n2wauaJdMKfdQ1ELAks5qwNBRgaJpZM4KI4At
.

Member

adriaanm commented Oct 3, 2016

We don't explicitly promise it because we can't mechanically check, but we
have tended to shy away from making changes that could impact
serializability.

If the performance benefit is significant, I'm on board but I don't think
we can revert it later knowing the impact on serialization.
On Mon, Oct 3, 2016 at 04:52 Lukas Rytz notifications@github.com wrote:

I'm still in favor of the change. Do we have any guarantees in
serialization compatibility? We have t8549, but it covers only a slice of
the serializable classes in the sdt lib. For users, anyone relying on
serialization compatibility across binary versions should be using
@SVUID anyway.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#5429 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAFjy21nsCJf1x2n2wauaJdMKfdQ1ELAks5qwNBRgaJpZM4KI4At
.

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 4, 2016

Member

I'm going a bit further into the off-topic..

Maybe we should have an -Xlint warning for subclasses (abstract and concrete, not traits) of Serializable that don't have the @SVUID annotation?

The problem with explicit @SVUID is that it not only allows benign changes such as adding a public method to a class, but also lets problematic changes pass. For example:

@SerialVersionUID(1L)
class ArrayBuffer extends Serializable {
  private var elems = new Array[AnyRef](8)
  private def ensureCapacity(n: Int): Unit = {
    // var newSize = elems.length * 2
    // while (n >= newSize) newSize *= 2
  }
  def clear(): Unit = { elems = new Array[AnyRef](8) }
}

Now assume there's a refactoring:

@SerialVersionUID(1L)
class ArrayBuffer(initSize: Int) extends Serializable {
  assert(initSize > 1)
  def this() = this(8)
  private var elems = new Array[AnyRef](initSize)
  private def ensureCapacity(n: Int): Unit = {
    // ...
    // var newSize = elems.length * 2
    // while (n >= newSize) newSize *= 2
    // 
  }
  def clear(): Unit = { elems = new Array[AnyRef](initSize) }
}

Now if you de-serialize a stream from the first version into the second version, the initSize field will be 0. After a call to clear(), ensureCapacity will loop forever.

This is of course not a Scala-specific problem, but we don't have a way of tracking actual breaking changes in the std lib.

Member

lrytz commented Oct 4, 2016

I'm going a bit further into the off-topic..

Maybe we should have an -Xlint warning for subclasses (abstract and concrete, not traits) of Serializable that don't have the @SVUID annotation?

The problem with explicit @SVUID is that it not only allows benign changes such as adding a public method to a class, but also lets problematic changes pass. For example:

@SerialVersionUID(1L)
class ArrayBuffer extends Serializable {
  private var elems = new Array[AnyRef](8)
  private def ensureCapacity(n: Int): Unit = {
    // var newSize = elems.length * 2
    // while (n >= newSize) newSize *= 2
  }
  def clear(): Unit = { elems = new Array[AnyRef](8) }
}

Now assume there's a refactoring:

@SerialVersionUID(1L)
class ArrayBuffer(initSize: Int) extends Serializable {
  assert(initSize > 1)
  def this() = this(8)
  private var elems = new Array[AnyRef](initSize)
  private def ensureCapacity(n: Int): Unit = {
    // ...
    // var newSize = elems.length * 2
    // while (n >= newSize) newSize *= 2
    // 
  }
  def clear(): Unit = { elems = new Array[AnyRef](initSize) }
}

Now if you de-serialize a stream from the first version into the second version, the initSize field will be 0. After a call to clear(), ensureCapacity will loop forever.

This is of course not a Scala-specific problem, but we don't have a way of tracking actual breaking changes in the std lib.

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Oct 4, 2016

Member

Let's spin the serialization topic of to its own scala-dev issue. Maybe we could consider assigning a SVUID that's 1:1 with the Scala binary version for all classes in the stdlib? Together with MiMa this would ensure serialization "compatibility" (somewhat trivially).

Member

adriaanm commented Oct 4, 2016

Let's spin the serialization topic of to its own scala-dev issue. Maybe we could consider assigning a SVUID that's 1:1 with the Scala binary version for all classes in the stdlib? Together with MiMa this would ensure serialization "compatibility" (somewhat trivially).

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 4, 2016

Member

Sounds good to me (discussing scala-dev). I think this PR could be merged, though it has a bit of an ad-hoc mix now for the @SVUID question: for some classes the annotation is added to make t8549 pass (the ClassTag subclasses), for others we adjusted the serialized value in the test itself (Ordering$Int$). But that's no different than what we've been doing so far.

@retronym could you take another look at the PR and the various points we discussed here? Could you also run your benchmarks before/after this commit? We can also merge it beforehand if that makes benchmarking easier.

I've been talking to Adriaan today, our (current) conclusion is to merge this PR if the performance benefit is significant, knowing we cannot change it during the 2.12.x cycle because of serialization compatibility.

Member

lrytz commented Oct 4, 2016

Sounds good to me (discussing scala-dev). I think this PR could be merged, though it has a bit of an ad-hoc mix now for the @SVUID question: for some classes the annotation is added to make t8549 pass (the ClassTag subclasses), for others we adjusted the serialized value in the test itself (Ordering$Int$). But that's no different than what we've been doing so far.

@retronym could you take another look at the PR and the various points we discussed here? Could you also run your benchmarks before/after this commit? We can also merge it beforehand if that makes benchmarking easier.

I've been talking to Adriaan today, our (current) conclusion is to merge this PR if the performance benefit is significant, knowing we cannot change it during the 2.12.x cycle because of serialization compatibility.

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 4, 2016

Member

t8549 is designed to check that we don't break serialization within minor releases.

I think we should use the serizaliazation proxy pattern more pervasively in the collections, rather than directly serializing the internal representation. But this would also expose us more to the seemingly intractable problem of https://issues.scala-lang.org/browse/SI-9237. sigh..

Member

retronym commented Oct 4, 2016

t8549 is designed to check that we don't break serialization within minor releases.

I think we should use the serizaliazation proxy pattern more pervasively in the collections, rather than directly serializing the internal representation. But this would also expose us more to the seemingly intractable problem of https://issues.scala-lang.org/browse/SI-9237. sigh..

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 4, 2016

Member

I'll benchmark again. I feel a bit uncomfortable going ahead with this change based purely on the result of the cold benchmark improvement without actually having understood why it makes the difference. It might not be possible to produce such an answer in time, but we can try some variations (e.g. like turning this on for library- or compiler-only).

Member

retronym commented Oct 4, 2016

I'll benchmark again. I feel a bit uncomfortable going ahead with this change based purely on the result of the cold benchmark improvement without actually having understood why it makes the difference. It might not be possible to produce such an answer in time, but we can try some variations (e.g. like turning this on for library- or compiler-only).

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Oct 4, 2016

Member

Let's prioritize finding the best answer to the performance question within
the time available (by end of week).
On Tue, Oct 4, 2016 at 16:55 Jason Zaugg notifications@github.com wrote:

I'll benchmark again. I feel a bit uncomfortable going ahead with this
change based purely on the result of the cold benchmark improvement without
actually having understood why it makes the difference. It might not be
possible to produce such an answer in time, but we can try some variations
(e.g. like turning this on for library- or compiler-only).


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#5429 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAFjy1U38JuJUXtGcjHWP47bEqW1s_S0ks5qwstMgaJpZM4KI4At
.

Member

adriaanm commented Oct 4, 2016

Let's prioritize finding the best answer to the performance question within
the time available (by end of week).
On Tue, Oct 4, 2016 at 16:55 Jason Zaugg notifications@github.com wrote:

I'll benchmark again. I feel a bit uncomfortable going ahead with this
change based purely on the result of the cold benchmark improvement without
actually having understood why it makes the difference. It might not be
possible to produce such an answer in time, but we can try some variations
(e.g. like turning this on for library- or compiler-only).


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#5429 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAFjy1U38JuJUXtGcjHWP47bEqW1s_S0ks5qwstMgaJpZM4KI4At
.

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 9, 2016

Member

OK, so hot performance seems to be the same. (n = no forwarders, y = with forwarders, unit = ms/op).

better-files

  • n: 821.011 ± 9.608
  • y: 818.349 ± 3.649

hello (a very simple class)

  • n: 45.312 ± 0.196
  • y: 44.975 ± 0.324

For cold performance, having the forwarders helps. The longer the compiler runs, the smaller the difference. For a minimal class, the difference is 41%.

scalap (2117 loc): 9%

  • n: 12240.296 ± 177.272
  • y: 11223.632 ± 313.381

better-files (924 loc): 12%

  • no forwarders: 8742.618 ± 142.837
  • with forwarders: 7757.120 ± 138.085

vector (935 loc): 17%

  • n: 7223.134 ± 191.534
  • y: 6145.266 ± 174.945

hello (3 loc): 41%

  • n: 3969.111 ± 24.397
  • y: 2804.122 ± 29.709

Wild speculation: the relation between slowdown and running time could confirm the hypothesis "due to no CHA for default methods". CHA is used for inlining decisions in C1, so it has a big impact in the beginning. C2 uses type profiling information for inlining decisions, so the lack of CHA does not matter anymore.

Member

lrytz commented Oct 9, 2016

OK, so hot performance seems to be the same. (n = no forwarders, y = with forwarders, unit = ms/op).

better-files

  • n: 821.011 ± 9.608
  • y: 818.349 ± 3.649

hello (a very simple class)

  • n: 45.312 ± 0.196
  • y: 44.975 ± 0.324

For cold performance, having the forwarders helps. The longer the compiler runs, the smaller the difference. For a minimal class, the difference is 41%.

scalap (2117 loc): 9%

  • n: 12240.296 ± 177.272
  • y: 11223.632 ± 313.381

better-files (924 loc): 12%

  • no forwarders: 8742.618 ± 142.837
  • with forwarders: 7757.120 ± 138.085

vector (935 loc): 17%

  • n: 7223.134 ± 191.534
  • y: 6145.266 ± 174.945

hello (3 loc): 41%

  • n: 3969.111 ± 24.397
  • y: 2804.122 ± 29.709

Wild speculation: the relation between slowdown and running time could confirm the hypothesis "due to no CHA for default methods". CHA is used for inlining decisions in C1, so it has a big impact in the beginning. C2 uses type profiling information for inlining decisions, so the lack of CHA does not matter anymore.

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 9, 2016

Member

will do more benchmarks, but a bit later :) ideas (mostly by jason):

  • effect of having forwarders only in the compiler, only in the library
  • check the effect of -XX:-UseCHA in 2.11.8, and in 2.12.x with / without forwarders
  • check the effect of -XX:TieredStopAtLevel=1 (disable C2) for various compilers
  • compare the compiler phase timings
  • compare -XX:+PrintCompilation logs
Member

lrytz commented Oct 9, 2016

will do more benchmarks, but a bit later :) ideas (mostly by jason):

  • effect of having forwarders only in the compiler, only in the library
  • check the effect of -XX:-UseCHA in 2.11.8, and in 2.12.x with / without forwarders
  • check the effect of -XX:TieredStopAtLevel=1 (disable C2) for various compilers
  • compare the compiler phase timings
  • compare -XX:+PrintCompilation logs
@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 9, 2016

Member

I'm also interested in the cold performance using JDK9-ea, rather than JDK8.

For 2.11.8, I've measured an improvement of 1.6s -> 1.4s from 1.8_102 to 9-ea-128.

I haven't quite been able to measure on 2.12.0-SNAPSHOT or this PR yet as I discovered another bug in the new optimizer (scala/scala-dev#242).

Member

retronym commented Oct 9, 2016

I'm also interested in the cold performance using JDK9-ea, rather than JDK8.

For 2.11.8, I've measured an improvement of 1.6s -> 1.4s from 1.8_102 to 9-ea-128.

I haven't quite been able to measure on 2.12.0-SNAPSHOT or this PR yet as I discovered another bug in the new optimizer (scala/scala-dev#242).

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 9, 2016

Member

I'm also interested in the cold performance using JDK9-ea, rather than JDK8.

I didn't see a difference in the cold performance between these JDK version in 2.12.0-SNAPSHOT.

Following on in the direction of your "compile hello world" benchmark, I tried benchmarking scalac -version. This takes 0.2s in 2.11.8, and 1s (!!!) on 2.12.0-SNAPSHOT.

Zooming in with the XCode profiler:

% instruments -t "Time Profiler" java ...

I don't see an obvious hotspot, but I do see an old enemy, generate_default_methods spread through the profile:

image

I remember this one from the time I tried adding all ancestors redundantly as parents of all classes, which severely degraded startup time. IIRC, the analysis doesn't maintain a set of already processed ancestors, so it repeats work when an interface is inherited through multiple routes. Perhaps we have enough instances of this in collections and/or the reflect/compiler to be a problem.

Member

retronym commented Oct 9, 2016

I'm also interested in the cold performance using JDK9-ea, rather than JDK8.

I didn't see a difference in the cold performance between these JDK version in 2.12.0-SNAPSHOT.

Following on in the direction of your "compile hello world" benchmark, I tried benchmarking scalac -version. This takes 0.2s in 2.11.8, and 1s (!!!) on 2.12.0-SNAPSHOT.

Zooming in with the XCode profiler:

% instruments -t "Time Profiler" java ...

I don't see an obvious hotspot, but I do see an old enemy, generate_default_methods spread through the profile:

image

I remember this one from the time I tried adding all ancestors redundantly as parents of all classes, which severely degraded startup time. IIRC, the analysis doesn't maintain a set of already processed ancestors, so it repeats work when an interface is inherited through multiple routes. Perhaps we have enough instances of this in collections and/or the reflect/compiler to be a problem.

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 9, 2016

Member

Example of our "strongly woven" family tree:

scala> def interfaceCount(cls: Class[_]) = {
  val m = collection.mutable.Map[Class[_], Int]().withDefaultValue(0)
  def loop(cls1: Class[_]) 
    cls1.getInterfaces.foreach(i => m(i) = (m(i) + 1))
    cls1.getInterfaces.foreach(loop)
    if (cls1.getSuperclass != null) loop(cls1.getSuperclass)
  }
  loop(cls)
  m
}
interfaceCount: (cls: Class[_])scala.collection.mutable.Map[Class[_],Int]

scala> println(interfaceCount(Nil.getClass).toSeq.sortBy(-_._2).mkString("\n"))
(interface scala.collection.GenTraversableOnce,75)
(interface scala.collection.Parallelizable,58)
(interface scala.collection.GenTraversableLike,58)
(interface scala.collection.generic.HasNewBuilder,32)
(interface scala.collection.GenIterableLike,26)
(interface scala.Equals,19)
(interface scala.collection.TraversableOnce,17)
(interface scala.collection.generic.FilterMonadic,17)
(interface scala.collection.TraversableLike,17)
(interface scala.collection.generic.GenericTraversableTemplate,15)
(interface scala.collection.GenTraversable,15)
(interface scala.collection.IterableLike,10)
(interface scala.collection.GenSeqLike,8)
(interface scala.collection.GenIterable,8)
(interface scala.collection.Traversable,7)
(interface scala.collection.SeqLike,5)
(interface scala.collection.Iterable,5)
(interface scala.collection.Seq,3)
(interface scala.PartialFunction,3)
(interface scala.Function1,3)
(interface scala.collection.GenSeq,3)
(interface java.io.Serializable,2)
(interface scala.collection.LinearSeqLike,2)
(interface scala.collection.immutable.Traversable,1)
(interface scala.collection.immutable.LinearSeq,1)
(interface scala.collection.immutable.Seq,1)
(interface scala.collection.LinearSeqOptimized,1)
(interface scala.collection.immutable.Iterable,1)
(interface scala.Immutable,1)
(interface scala.Serializable,1)
(interface scala.Product,1)
(interface scala.collection.LinearSeq,1)

HotSpot code involved:

http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/tip/src/share/vm/classfile/defaultMethods.cpp

Member

retronym commented Oct 9, 2016

Example of our "strongly woven" family tree:

scala> def interfaceCount(cls: Class[_]) = {
  val m = collection.mutable.Map[Class[_], Int]().withDefaultValue(0)
  def loop(cls1: Class[_]) 
    cls1.getInterfaces.foreach(i => m(i) = (m(i) + 1))
    cls1.getInterfaces.foreach(loop)
    if (cls1.getSuperclass != null) loop(cls1.getSuperclass)
  }
  loop(cls)
  m
}
interfaceCount: (cls: Class[_])scala.collection.mutable.Map[Class[_],Int]

scala> println(interfaceCount(Nil.getClass).toSeq.sortBy(-_._2).mkString("\n"))
(interface scala.collection.GenTraversableOnce,75)
(interface scala.collection.Parallelizable,58)
(interface scala.collection.GenTraversableLike,58)
(interface scala.collection.generic.HasNewBuilder,32)
(interface scala.collection.GenIterableLike,26)
(interface scala.Equals,19)
(interface scala.collection.TraversableOnce,17)
(interface scala.collection.generic.FilterMonadic,17)
(interface scala.collection.TraversableLike,17)
(interface scala.collection.generic.GenericTraversableTemplate,15)
(interface scala.collection.GenTraversable,15)
(interface scala.collection.IterableLike,10)
(interface scala.collection.GenSeqLike,8)
(interface scala.collection.GenIterable,8)
(interface scala.collection.Traversable,7)
(interface scala.collection.SeqLike,5)
(interface scala.collection.Iterable,5)
(interface scala.collection.Seq,3)
(interface scala.PartialFunction,3)
(interface scala.Function1,3)
(interface scala.collection.GenSeq,3)
(interface java.io.Serializable,2)
(interface scala.collection.LinearSeqLike,2)
(interface scala.collection.immutable.Traversable,1)
(interface scala.collection.immutable.LinearSeq,1)
(interface scala.collection.immutable.Seq,1)
(interface scala.collection.LinearSeqOptimized,1)
(interface scala.collection.immutable.Iterable,1)
(interface scala.Immutable,1)
(interface scala.Serializable,1)
(interface scala.Product,1)
(interface scala.collection.LinearSeq,1)

HotSpot code involved:

http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/tip/src/share/vm/classfile/defaultMethods.cpp

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 9, 2016

Member

This recently fixed JDK bug is in the same ballpark, but appears distinct: https://bugs.openjdk.java.net/browse/JDK-8163969

Member

retronym commented Oct 9, 2016

This recently fixed JDK bug is in the same ballpark, but appears distinct: https://bugs.openjdk.java.net/browse/JDK-8163969

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 9, 2016

Member

It's an interesting thing to keep in mind, but I guess there's nothing we can do about it for RC2 - the presence / absence of mixin forwarders does not have an impact here, right?

I'll do some more benchmarks for the effects related to mixin forwarders when I get back.

Member

lrytz commented Oct 9, 2016

It's an interesting thing to keep in mind, but I guess there's nothing we can do about it for RC2 - the presence / absence of mixin forwarders does not have an impact here, right?

I'll do some more benchmarks for the effects related to mixin forwarders when I get back.

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 9, 2016

Member

When using -XX:TieredStopAtLevel=1, i.e., disabling C2, therefore relying more on CHA, there is a difference in hot performance (with C2 enabled, hot performance was the same):

better-files

  • n: 1834.449 ± 8.637
  • y: 1893.169 ± 12.478

hello

  • n: 101.173 ± 0.382
  • y: 96.638 ± 0.361
Member

lrytz commented Oct 9, 2016

When using -XX:TieredStopAtLevel=1, i.e., disabling C2, therefore relying more on CHA, there is a difference in hot performance (with C2 enabled, hot performance was the same):

better-files

  • n: 1834.449 ± 8.637
  • y: 1893.169 ± 12.478

hello

  • n: 101.173 ± 0.382
  • y: 96.638 ± 0.361
@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 9, 2016

Member

I've lodged a bug for the HotSpot problem with default method resolution performance, I'll link it here when it appears in the tracker.

Here's an attempt to distill the problem in a Java-only test case: https://gist.github.com/retronym/ddedbadc3c764da66ffdb9a911915b40

Member

retronym commented Oct 9, 2016

I've lodged a bug for the HotSpot problem with default method resolution performance, I'll link it here when it appears in the tracker.

Here's an attempt to distill the problem in a Java-only test case: https://gist.github.com/retronym/ddedbadc3c764da66ffdb9a911915b40

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 10, 2016

Member

I did a bit of benchmarking with the debug JVM that I built on linux, with the goal of comparing the impact of the -XX:-UseCHA flag.

But it looks like a bad idea to use such a VM for benchmarking: it is extremely slow (plenty of debug code all over). For example, compiling better-files takes 11s with Oracle 1.8.0_102, but 47s with the debug VM I built (jdk8u branch).

Anyway, here are some numbers I collected on the debug VM, using 2.11.8.

hello, hot performance

  • default: 160.882 ± 5.780
  • -UseCHA: 158.800 ± 5.996

hello, cold performance

  • default: 9072.075 ± 354.355
  • -UseCHA: 6988.007 ± 212.776

better-files, hot performance

  • default: 1578.248 ± 140.107
  • -UseCHA: 1874.994 ± 47.585

better-files, cold performance

  • default: 32128.875 ± 653.279
  • -UseCHA: ~ 55000

Not sure if we can draw any conclusions here.. CHA slows down, maybe due to additional debug logging?

Member

lrytz commented Oct 10, 2016

I did a bit of benchmarking with the debug JVM that I built on linux, with the goal of comparing the impact of the -XX:-UseCHA flag.

But it looks like a bad idea to use such a VM for benchmarking: it is extremely slow (plenty of debug code all over). For example, compiling better-files takes 11s with Oracle 1.8.0_102, but 47s with the debug VM I built (jdk8u branch).

Anyway, here are some numbers I collected on the debug VM, using 2.11.8.

hello, hot performance

  • default: 160.882 ± 5.780
  • -UseCHA: 158.800 ± 5.996

hello, cold performance

  • default: 9072.075 ± 354.355
  • -UseCHA: 6988.007 ± 212.776

better-files, hot performance

  • default: 1578.248 ± 140.107
  • -UseCHA: 1874.994 ± 47.585

better-files, cold performance

  • default: 32128.875 ± 653.279
  • -UseCHA: ~ 55000

Not sure if we can draw any conclusions here.. CHA slows down, maybe due to additional debug logging?

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 11, 2016

Member

Interestingly, the difference cold performance is caused mostly by forwarders in scala-library.jar.

hello, cold performance

  • no forwarders: 3973.261 ± 23.066
  • forwarders only in compiler + reflect: 3926.395 ± 34.604
  • forwarders only in library: 2861.180 ± 22.530
  • forwarders in all jars: 2808.490 ± 31.637
Member

lrytz commented Oct 11, 2016

Interestingly, the difference cold performance is caused mostly by forwarders in scala-library.jar.

hello, cold performance

  • no forwarders: 3973.261 ± 23.066
  • forwarders only in compiler + reflect: 3926.395 ± 34.604
  • forwarders only in library: 2861.180 ± 22.530
  • forwarders in all jars: 2808.490 ± 31.637
@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 11, 2016

Member

One more number: when disabling JIT entirely (-Xint), the forwarders still improve performance. For compiling hello, cold performance, it's 15%:

  • no forwarders: 7031.654 ± 60.182
  • with forwarders: 6099.804 ± 24.948

If we assume that CHA is only used in the optimizers (not sure if that is true), this shows that there is some other factor at play.

Maybe this is related to method resolution, but this is just a guess.

trait T { def f = 1 }
class A extends T
class C { def g(a: A) = a.f }

In C we get an invokevirtual A.f (with or without the forwarder). If there's a forwarder, resolution should be quick (it's right there in A). The forwarder itself does:

class A implements T {
  public f()I
    ALOAD 0
    INVOKESTATIC T.f$ (LT;)I
    IRETURN
}

The lookup of the static method should be quick, too. The static accessor in T does:

interface T {
  public static synthetic f$(LT;)I
    ALOAD 0
    INVOKESPECIAL T.f ()I
    IRETURN
}

The lookup of INVOKESPECIAL T.f should also be quick, it's in the named class.

If A doesn't have a forwarder, resolving A.f is more costly, especially if A extends a large number of interfaces and f is defined somewhere high up (think: collections). We should benchmark a bit more in this area at some point.

Note that the interpreter needs to go through two forwarders, but this is still faster than the version where the default method is directly executed.

Member

lrytz commented Oct 11, 2016

One more number: when disabling JIT entirely (-Xint), the forwarders still improve performance. For compiling hello, cold performance, it's 15%:

  • no forwarders: 7031.654 ± 60.182
  • with forwarders: 6099.804 ± 24.948

If we assume that CHA is only used in the optimizers (not sure if that is true), this shows that there is some other factor at play.

Maybe this is related to method resolution, but this is just a guess.

trait T { def f = 1 }
class A extends T
class C { def g(a: A) = a.f }

In C we get an invokevirtual A.f (with or without the forwarder). If there's a forwarder, resolution should be quick (it's right there in A). The forwarder itself does:

class A implements T {
  public f()I
    ALOAD 0
    INVOKESTATIC T.f$ (LT;)I
    IRETURN
}

The lookup of the static method should be quick, too. The static accessor in T does:

interface T {
  public static synthetic f$(LT;)I
    ALOAD 0
    INVOKESPECIAL T.f ()I
    IRETURN
}

The lookup of INVOKESPECIAL T.f should also be quick, it's in the named class.

If A doesn't have a forwarder, resolving A.f is more costly, especially if A extends a large number of interfaces and f is defined somewhere high up (think: collections). We should benchmark a bit more in this area at some point.

Note that the interpreter needs to go through two forwarders, but this is still faster than the version where the default method is directly executed.

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Oct 11, 2016

Member

I think we should take a decision based on the numbers we have now. (Suggestions for other benchmarks are welcome, if they can be done quickly. I guess we want to have RC2 soon.)

To summarize:

  • 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 probalby multiple underlying causes (CHA, populating vtables), we don't have a clear picture
  • We cannot revert in 2.12.x due to serialization compatibility
Member

lrytz commented Oct 11, 2016

I think we should take a decision based on the numbers we have now. (Suggestions for other benchmarks are welcome, if they can be done quickly. I guess we want to have RC2 soon.)

To summarize:

  • 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 probalby multiple underlying causes (CHA, populating vtables), we don't have a clear picture
  • We cannot revert in 2.12.x due to serialization compatibility
@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Oct 11, 2016

Member

Since emitting the forwarders doesn't adversely affect hot performance, while substantially improving startup on current JVMs, I agree we should merge this PR. Our concern remains that this is an area where we are exposed to intricacies of the JIT and classloading, which could change in future updates, while we are bound to the choice we make now (because flipping the switch affects serialization compatibility).

Member

adriaanm commented Oct 11, 2016

Since emitting the forwarders doesn't adversely affect hot performance, while substantially improving startup on current JVMs, I agree we should merge this PR. Our concern remains that this is an area where we are exposed to intricacies of the JIT and classloading, which could change in future updates, while we are bound to the choice we make now (because flipping the switch affects serialization compatibility).

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 12, 2016

Member

LGTM

Member

retronym commented Oct 12, 2016

LGTM

@adriaanm adriaanm merged commit 1e81a09 into scala:2.12.0 Oct 12, 2016

6 checks passed

cla @lrytz signed the Scala CLA. Thanks!
Details
combined All previous commits successful.
integrate-ide [140] SUCCESS. Took 2 s.
Details
validate-main [164] SUCCESS. Took 108 min.
Details
validate-publish-core [164] SUCCESS. Took 7 min.
Details
validate-test [139] SUCCESS. Took 101 min.
Details
@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Oct 12, 2016

Member

I've spun out the issue with startup performance as scala/scala-dev#243

Member

retronym commented Oct 12, 2016

I've spun out the issue with startup performance as scala/scala-dev#243

retronym added a commit to retronym/scala that referenced this pull request Oct 13, 2016

retronym added a commit to retronym/scala that referenced this pull request Oct 13, 2016

@adriaanm adriaanm modified the milestone: 2.12.0-RC2 Oct 29, 2016

@adriaanm adriaanm added the 2.12 label Oct 29, 2016

@lrytz lrytz deleted the lrytz:sd224 branch Apr 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment