further reduction in inlining time. This time, 40%

Two main improvements:
  (a) an IClass is loaded via ICodeReader at most once.
  (b) given that type-flow analysis results don't change for "external" methods (those loaded via ICodeReader), we can cache them.

The same inlining decisions are made as before, see -Ylog:inliner, with a focus on lines starting with "[log inliner] Inlining"

review by @VladUreche @dragos @paulp

1 files changed, 70 insertions, 48 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
index 3e8ef3f611..b5701f07cb 100644
--- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
+++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
@@ -62,6 +62,12 @@ abstract class Inliners extends SubComponent {
     override def apply(c: IClass) {
       inliner analyzeClass c
     }
+
+    override def run() {
+      super.run()
+      inliner.NonPublicRefs.usesNonPublics.clear()
+      inliner.recentTFAs.clear
+    }
   }
 
   def isBottomType(sym: Symbol) = sym == NullClass || sym == NothingClass
@@ -79,17 +85,10 @@ abstract class Inliners extends SubComponent {
       val Private, Protected, Public = Value
 
       /** Cache whether a method calls private members. */
-      val usesNonPublics: mutable.Map[IMethod, Value] = perRunCaches.newMap()
+      val usesNonPublics = mutable.Map.empty[IMethod, Value]
     }
     import NonPublicRefs._
 
-    /* fresh name counter */
-    val fresh = perRunCaches.newMap[String, Int]() withDefaultValue 0
-    def freshName(s: String): TermName = {
-      fresh(s) += 1
-      newTermName(s + fresh(s))
-    }
-
     private def hasInline(sym: Symbol)    = sym hasAnnotation ScalaInlineClass
     private def hasNoInline(sym: Symbol)  = sym hasAnnotation ScalaNoInlineClass
 
@@ -97,6 +96,28 @@ abstract class Inliners extends SubComponent {
     private var currentIClazz: IClass = _
     private def warn(pos: Position, msg: String) = currentIClazz.cunit.warning(pos, msg)
 
+    val recentTFAs = mutable.Map.empty[Symbol, Tuple2[Boolean, analysis.MethodTFA]]
+    private def getRecentTFA(incm: IMethod): (Boolean, analysis.MethodTFA) = {
+
+        def containsRETURN(blocks: List[BasicBlock]) = blocks exists { bb => bb.lastInstruction.isInstanceOf[RETURN] }
+
+      val opt = recentTFAs.get(incm.symbol)
+      if(opt.isDefined) {
+        // FYI val cachedBBs = opt.get._2.in.keySet
+        // FYI assert(incm.blocks.toSet == cachedBBs)
+        // incm.code.touched plays no role here
+        return opt.get
+      }
+
+      val hasRETURN = containsRETURN(incm.code.blocksList) || (incm.exh exists { eh => containsRETURN(eh.blocks) })
+      var a: analysis.MethodTFA = null
+      if(hasRETURN) { a = new analysis.MethodTFA(incm); a.run }
+
+      if(hasInline(incm.symbol)) { recentTFAs.put(incm.symbol, (hasRETURN, a)) }
+
+      (hasRETURN, a)
+    }
+
     def analyzeClass(cls: IClass): Unit =
       if (settings.inline.value) {
         debuglog("Analyzing " + cls)
@@ -106,7 +127,7 @@ abstract class Inliners extends SubComponent {
         ms foreach { im =>
           if(hasInline(im.symbol)) {
             log("Not inlining into " + im.symbol.originalName.decode + " because it is marked @inline.")
-          } else if(im.hasCode) {
+          } else if(im.hasCode && !im.symbol.isBridge) {
             analyzeMethod(im)
           }
         }
@@ -114,20 +135,21 @@ abstract class Inliners extends SubComponent {
 
     val tfa   = new analysis.MTFAGrowable()
     tfa.stat  = global.opt.printStats
-    val staleOut = new mutable.ListBuffer[BasicBlock]
+    val staleOut      = new mutable.ListBuffer[BasicBlock]
     val splicedBlocks = mutable.Set.empty[BasicBlock]
-    val staleIn  = mutable.Set.empty[BasicBlock]
-
-    // how many times have we already inlined this method here?
-    private val inlinedMethodCount = perRunCaches.newMap[Symbol, Int]() withDefaultValue 0
+    val staleIn       = mutable.Set.empty[BasicBlock]
 
     def analyzeMethod(m: IMethod): Unit = {
-      var sizeBeforeInlining  = if (m.hasCode) m.code.blockCount else 0
-      var instrBeforeInlining = if (m.hasCode) m.code.instructionCount else 0
+      var sizeBeforeInlining  = m.code.blockCount
+      var instrBeforeInlining = m.code.instructionCount
       var retry = false
       var count = 0
-      fresh.clear()
-      inlinedMethodCount.clear()
+
+      // fresh name counter
+      val fresh = mutable.HashMap.empty[String, Int] withDefaultValue 0
+      // how many times have we already inlined this method here?
+      val inlinedMethodCount = mutable.HashMap.empty[Symbol, Int] withDefaultValue 0
+
       val caller = new IMethodInfo(m)
       var info: tfa.lattice.Elem = null
 
@@ -150,10 +172,14 @@ abstract class Inliners extends SubComponent {
           // Until r22824 this line was:
           //   icodes.icode(concreteMethod.enclClass, true)
           //
-          // Changing it to the below was the proximate cause for SI-3882:
+          // Changing it to
+          //   icodes.load(concreteMethod.enclClass)
+          // was the proximate cause for SI-3882:
           //   error: Illegal index: 0 overlaps List((variable par1,LONG))
           //   error: Illegal index: 0 overlaps List((variable par1,LONG))
-          icodes.load(concreteMethod.enclClass)
+          if(!icodes.loaded.contains(concreteMethod.enclClass)) {
+            icodes.load(concreteMethod.enclClass)
+          }
         }
 
         def isAvailable = icodes available concreteMethod.enclClass
@@ -179,7 +205,7 @@ abstract class Inliners extends SubComponent {
           lookupIMethod(concreteMethod, receiver) match {
             case Some(callee) =>
               val inc   = new IMethodInfo(callee)
-              val pair  = new CallerCalleeInfo(caller, inc)
+              val pair  = new CallerCalleeInfo(caller, inc, fresh, inlinedMethodCount)
 
               if (pair isStampedForInlining info.stack) {
                 retry = true
@@ -196,6 +222,7 @@ abstract class Inliners extends SubComponent {
                  * might have changed after the inlining.
                  */
                 usesNonPublics -= m
+                recentTFAs     -= m.symbol
               }
               else {
                 if (settings.debug.value)
@@ -315,11 +342,10 @@ abstract class Inliners extends SubComponent {
 
       def inline        = hasInline(sym)
       def noinline      = hasNoInline(sym)
-      def numInlined    = inlinedMethodCount(sym)
 
       def isBridge      = sym.isBridge
       def isInClosure   = isClosureClass(owner)
-      def isHigherOrder = isHigherOrderMethod(sym)
+      val isHigherOrder = isHigherOrderMethod(sym)
       def isMonadic     = isMonadicMethod(sym)
 
       def handlers      = m.exh
@@ -328,7 +354,7 @@ abstract class Inliners extends SubComponent {
       def length        = blocks.length
       def openBlocks    = blocks filterNot (_.closed)
       def instructions  = m.code.instructions
-      def linearized    = linearizer linearize m
+      // def linearized    = linearizer linearize m
 
       def isSmall       = (length <= SMALL_METHOD_SIZE) && blocks(0).length < 10
       def isLarge       = length > MAX_INLINE_SIZE
@@ -347,9 +373,14 @@ abstract class Inliners extends SubComponent {
       def addHandlers(exhs: List[ExceptionHandler]) = m.exh = exhs ::: m.exh
     }
 
-    class CallerCalleeInfo(val caller: IMethodInfo, val inc: IMethodInfo) {
+    class CallerCalleeInfo(val caller: IMethodInfo, val inc: IMethodInfo, fresh: mutable.Map[String, Int], inlinedMethodCount: collection.Map[Symbol, Int]) {
       def isLargeSum  = caller.length + inc.length - 1 > SMALL_METHOD_SIZE
 
+      private def freshName(s: String): TermName = {
+        fresh(s) += 1
+        newTermName(s + fresh(s))
+      }
+
       /** Inline 'inc' into 'caller' at the given block and instruction.
        *  The instruction must be a CALL_METHOD.
        */
@@ -364,7 +395,7 @@ abstract class Inliners extends SubComponent {
         def newLocal(baseName: String, kind: TypeKind) =
           new Local(caller.sym.newVariable(freshName(baseName), targetPos), kind, false)
 
-        val a = new analysis.MethodTFA(inc.m)
+        val (hasRETURN, a) = getRecentTFA(inc.m)
 
         /* The exception handlers that are active at the current block. */
         val activeHandlers = caller.handlers filter (_ covered block)
@@ -393,7 +424,7 @@ abstract class Inliners extends SubComponent {
           case x     => newLocal("$retVal", x)
         }
 
-        val inlinedLocals = perRunCaches.newMap[Local, Local]()
+        val inlinedLocals = mutable.HashMap.empty[Local, Local]
 
         /** Add a new block in the current context. */
         def newBlock() = {
@@ -475,9 +506,6 @@ abstract class Inliners extends SubComponent {
           inlinedBlock(b).varsInScope ++= (b.varsInScope map inlinedLocals)
         }
 
-        // analyse callee
-        a.run
-
         // re-emit the instructions before the call
         block.open
         block.clear
@@ -494,7 +522,7 @@ abstract class Inliners extends SubComponent {
         // duplicate the other blocks in the callee
         val calleeLin = inc.m.linearizedBlocks()
         calleeLin foreach { bb =>
-          var info = a in bb
+          var info = if(hasRETURN) (a in bb) else null
           def emitInlined(i: Instruction) = inlinedBlock(bb).emit(i, targetPos)
           def emitDrops(toDrop: Int)      = info.stack.types drop toDrop foreach (t => emitInlined(DROP(t)))
 
@@ -510,7 +538,7 @@ abstract class Inliners extends SubComponent {
               case _            => ()
             }
             emitInlined(map(i))
-            info = a.interpret(info, i)
+            info = if(hasRETURN) a.interpret(info, i) else null
           }
           inlinedBlock(bb).close
         }
@@ -538,7 +566,7 @@ abstract class Inliners extends SubComponent {
            |  isSafeToInline: %s
            |  shouldInline: %s
         """.stripMargin.format(
-          inc.m, sameSymbols, inc.numInlined < 2,
+          inc.m, sameSymbols, inlinedMethodCount(inc.sym) < 2,
           inc.m.hasCode, isSafeToInline(stack), shouldInline
         )
       )
@@ -635,28 +663,22 @@ abstract class Inliners extends SubComponent {
 
         var score = 0
 
-        // better not inline inside closures, but hope that the closure itself
-        // is repeatedly inlined
-        if (caller.isInClosure) score -= 2
+        // better not inline inside closures, but hope that the closure itself is repeatedly inlined
+        if (caller.isInClosure)           score -= 2
         else if (caller.inlinedCalls < 1) score -= 1 // only monadic methods can trigger the first inline
 
-        if (inc.isSmall)
-          score += 1
+        if (inc.isSmall) score += 1;
+        if (inc.isLarge) score -= 1;
         if (caller.isSmall && isLargeSum) {
           score -= 1
           debuglog("shouldInline: score decreased to " + score + " because small " + caller + " would become large")
         }
-        if (inc.isLarge)
-          score -= 1
 
-        if (inc.isMonadic)
-          score += 3
-        else if (inc.isHigherOrder)
-          score += 1
-        if (inc.isInClosure)
-          score += 2
-        if (inc.numInlined > 2)
-          score -= 2
+        if (inc.isMonadic)          score += 3
+        else if (inc.isHigherOrder) score += 1
+
+        if (inc.isInClosure)                 score += 2;
+        if (inlinedMethodCount(inc.sym) > 2) score -= 2;
 
         log("shouldInline(" + inc.m + ") score: " + score)