From 4caa766e85b6ee2b1629450df9a18bb76380ee12 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Tue, 7 Aug 2012 19:03:33 +0200 Subject: [PATCH] Enable inlining in constructors. Inlining in constructors has been disabled a long time ago due to some VerifyErrors. Unfortunately, @dragos cannot recall what exactly was the problem. I tried to enable inlining in constructors and I didn't see any problem. `Predef.assert` calls in class constructors are one of the biggest contributors to closure allocation in a compiler so we better off get rid of it. Added a test-case that checks if inlining in constructors works properly. Review by @magarciaEPFL and @paulp. --- .../scala/tools/nsc/backend/opt/Inliners.scala | 2 +- .../instrumented/inline-in-constructors.check | 3 +++ .../instrumented/inline-in-constructors.flags | 1 + .../inline-in-constructors/assert_1.scala | 13 +++++++++++++ .../inline-in-constructors/bar_2.scala | 7 +++++++ .../inline-in-constructors/test_3.scala | 15 +++++++++++++++ 6 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/files/instrumented/inline-in-constructors.check create mode 100644 test/files/instrumented/inline-in-constructors.flags create mode 100644 test/files/instrumented/inline-in-constructors/assert_1.scala create mode 100644 test/files/instrumented/inline-in-constructors/bar_2.scala create mode 100644 test/files/instrumented/inline-in-constructors/test_3.scala diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala index b1d00132ce72..d87a242f1bd0 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala @@ -247,7 +247,7 @@ abstract class Inliners extends SubComponent { debuglog("Analyzing " + cls) this.currentIClazz = cls - val ms = cls.methods filterNot { _.symbol.isConstructor } sorted imethodOrdering + val ms = cls.methods sorted imethodOrdering ms foreach { im => if(hasInline(im.symbol)) { log("Not inlining into " + im.symbol.originalName.decode + " because it is marked @inline.") diff --git a/test/files/instrumented/inline-in-constructors.check b/test/files/instrumented/inline-in-constructors.check new file mode 100644 index 000000000000..c6c9ae4e1588 --- /dev/null +++ b/test/files/instrumented/inline-in-constructors.check @@ -0,0 +1,3 @@ +Method call statistics: + 1 instrumented/Bar.(Z)V + 1 instrumented/Foo.(I)V diff --git a/test/files/instrumented/inline-in-constructors.flags b/test/files/instrumented/inline-in-constructors.flags new file mode 100644 index 000000000000..c9b68d70dc6a --- /dev/null +++ b/test/files/instrumented/inline-in-constructors.flags @@ -0,0 +1 @@ +-optimise diff --git a/test/files/instrumented/inline-in-constructors/assert_1.scala b/test/files/instrumented/inline-in-constructors/assert_1.scala new file mode 100644 index 000000000000..a03757b89c47 --- /dev/null +++ b/test/files/instrumented/inline-in-constructors/assert_1.scala @@ -0,0 +1,13 @@ +package instrumented + +object MyPredef { + @inline + final def assert(assertion: Boolean, message: => Any) { + if (!assertion) + throw new java.lang.AssertionError("assertion failed: " + message) + } +} + +class Foo(x: Int) { + MyPredef.assert(x > 0, "not positive: " + x) +} diff --git a/test/files/instrumented/inline-in-constructors/bar_2.scala b/test/files/instrumented/inline-in-constructors/bar_2.scala new file mode 100644 index 000000000000..418dac5a6769 --- /dev/null +++ b/test/files/instrumented/inline-in-constructors/bar_2.scala @@ -0,0 +1,7 @@ +package instrumented + +/** Class that uses assert compiled in previous compiler run so we check if + inlining in constructors works across different compilation runs */ +class Bar(x: Boolean) { + MyPredef.assert(x, "not true: " + x) +} diff --git a/test/files/instrumented/inline-in-constructors/test_3.scala b/test/files/instrumented/inline-in-constructors/test_3.scala new file mode 100644 index 000000000000..c4d4cc5f3754 --- /dev/null +++ b/test/files/instrumented/inline-in-constructors/test_3.scala @@ -0,0 +1,15 @@ +import scala.tools.partest.instrumented.Instrumentation._ +import instrumented._ + +object Test { + def main(args: Array[String]) { + // force predef initialization before profiling + Predef + MyPredef + startProfiling() + val a = new Foo(2) + val b = new Bar(true) + stopProfiling() + printStatistics() + } +}