Don't inlinie if the resulting method becomes too large for the JVM

This threshold is really the last resort and should never be reached.
An inlining heuristic that blows up methods to the maximum size
allowed by the JVM is broken.

In the future we need to include some heuristic about code size when
making an inlining decision, see github.com/scala-opt/scala/issues/2

3 files changed, 36 insertions, 3 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala
index 89cbbc793c..c15ed032fb 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BackendReporting.scala
@@ -214,10 +214,15 @@ object BackendReporting {
 
       case SynchronizedMethod(_, _, _) =>
         s"Method $calleeMethodSig cannot be inlined because it is synchronized."
+
+      case ResultingMethodTooLarge(_, _, _, callsiteClass, callsiteName, callsiteDesc) =>
+        s"""The size of the callsite method ${BackendReporting.methodSignature(callsiteClass, callsiteName, callsiteDesc)}
+           |would exceed the JVM method size limit after inlining $calleeMethodSig.
+         """.stripMargin
     }
 
     def emitWarning(settings: ScalaSettings): Boolean = this match {
-      case _: IllegalAccessInstruction | _: MethodWithHandlerCalledOnNonEmptyStack | _: SynchronizedMethod =>
+      case _: IllegalAccessInstruction | _: MethodWithHandlerCalledOnNonEmptyStack | _: SynchronizedMethod | _: ResultingMethodTooLarge =>
         settings.YoptWarningEmitAtInlineFailed
 
       case IllegalAccessCheckFailed(_, _, _, _, _, cause) =>
@@ -231,6 +236,8 @@ object BackendReporting {
   case class MethodWithHandlerCalledOnNonEmptyStack(calleeDeclarationClass: InternalName, name: String, descriptor: String,
                                                     callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning
   case class SynchronizedMethod(calleeDeclarationClass: InternalName, name: String, descriptor: String) extends CannotInlineWarning
+  case class ResultingMethodTooLarge(calleeDeclarationClass: InternalName, name: String, descriptor: String,
+                                     callsiteClass: InternalName, callsiteName: String, callsiteDesc: String) extends CannotInlineWarning
 
   /**
    * Used in the InlineInfo of a ClassBType, when some issue occurred obtaining the inline information.
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala
index a295bc49b1..fe1dbe682b 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala
@@ -10,6 +10,7 @@ package opt
 import scala.annotation.{tailrec, switch}
 import scala.collection.mutable
 import scala.reflect.internal.util.Collections._
+import scala.tools.asm.commons.CodeSizeEvaluator
 import scala.tools.asm.tree.analysis._
 import scala.tools.asm.{MethodWriter, ClassWriter, Label, Opcodes}
 import scala.tools.asm.tree._
@@ -21,6 +22,12 @@ import scala.tools.nsc.backend.jvm.BTypes._
 
 object BytecodeUtils {
 
+  // http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.9.1
+  final val maxJVMMethodSize         = 65535
+
+  // 5% margin, more than enough for the instructions added by the inliner (store / load args, null check for instance methods)
+  final val maxMethodSizeAfterInline = maxJVMMethodSize - (maxJVMMethodSize / 20)
+
   object Goto {
     def unapply(instruction: AbstractInsnNode): Option[JumpInsnNode] = {
       if (instruction.getOpcode == Opcodes.GOTO) Some(instruction.asInstanceOf[JumpInsnNode])
@@ -217,7 +224,7 @@ object BytecodeUtils {
    * to create a separate visitor for computing those values, duplicating the functionality from the
    * MethodWriter.
    */
-  def computeMaxLocalsMaxStack(method: MethodNode) {
+  def computeMaxLocalsMaxStack(method: MethodNode): Unit = {
     val cw = new ClassWriter(ClassWriter.COMPUTE_MAXS)
     val excs = method.exceptions.asScala.toArray
     val mw = cw.visitMethod(method.access, method.name, method.desc, method.signature, excs).asInstanceOf[MethodWriter]
@@ -226,6 +233,21 @@ object BytecodeUtils {
     method.maxStack = mw.getMaxStack
   }
 
+  def codeSizeOKForInlining(caller: MethodNode, callee: MethodNode): Boolean = {
+    // Looking at the implementation of CodeSizeEvaluator, all instructions except tableswitch and
+    // lookupswitch are <= 8 bytes. These should be rare enough for 8 to be an OK rough upper bound.
+    def roughUpperBound(methodNode: MethodNode): Int = methodNode.instructions.size * 8
+
+    def maxSize(methodNode: MethodNode): Int = {
+      val eval = new CodeSizeEvaluator(null)
+      methodNode.accept(eval)
+      eval.getMaxSize
+    }
+
+    (roughUpperBound(caller) + roughUpperBound(callee) > maxMethodSizeAfterInline) &&
+      (maxSize(caller) + maxSize(callee) > maxMethodSizeAfterInline)
+  }
+
   def removeLineNumberNodes(classNode: ClassNode): Unit = {
     for (m <- classNode.methods.asScala) removeLineNumberNodes(m.instructions)
   }
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
index 1a51063231..fd2a2e1b26 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala
@@ -497,7 +497,11 @@ class Inliner[BT <: BTypes](val btypes: BT) {
       callsiteStackHeight > expectedArgs
     }
 
-    if (isSynchronizedMethod(callee)) {
+    if (codeSizeOKForInlining(callsiteMethod, callee)) {
+      Some(ResultingMethodTooLarge(
+        calleeDeclarationClass.internalName, callee.name, callee.desc,
+        callsiteClass.internalName, callsiteMethod.name, callsiteMethod.desc))
+    } else if (isSynchronizedMethod(callee)) {
       // Could be done by locking on the receiver, wrapping the inlined code in a try and unlocking
       // in finally. But it's probably not worth the effort, scala never emits synchronized methods.
       Some(SynchronizedMethod(calleeDeclarationClass.internalName, callee.name, callee.desc))