I cannot be beaten by mysterious side effecting.

25% off the time spent in genjvm. Also eliminates the originalOwner
field in Symbol in favor of a cache which is only updated when the
originalOwner changes. Review by dragos.

5 files changed, 91 insertions, 95 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index f54854a227..c1623c1fbf 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -29,7 +29,7 @@ abstract class GenICode extends SubComponent  {
     ArrayClass, ObjectClass, ThrowableClass, StringClass, NothingClass, NullClass, AnyRefClass,
     Object_equals, Object_isInstanceOf, Object_asInstanceOf, ScalaRunTimeModule,
     BoxedNumberClass, BoxedCharacterClass,
-    getMember, getPrimitiveCompanion
+    getMember, runtimeCompanions
   }
   import scalaPrimitives.{
     isArrayOp, isComparisonOp, isLogicalOp,
@@ -1217,7 +1217,7 @@ abstract class GenICode extends SubComponent  {
       }
 
     private def genLoadModule(ctx: Context, sym: Symbol, pos: Position) {
-      ctx.bb.emit(LOAD_MODULE(getPrimitiveCompanion(sym) getOrElse sym), pos)
+      ctx.bb.emit(LOAD_MODULE(runtimeCompanions.getOrElse(sym, sym)), pos)
     }
 
     def genConversion(from: TypeKind, to: TypeKind, ctx: Context, cast: Boolean) = {
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
index 2bc501cea0..18f82d51be 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
@@ -8,8 +8,8 @@ package scala.tools.nsc
 package backend.jvm
 
 import java.nio.ByteBuffer
-
 import scala.collection.{ mutable, immutable }
+import mutable.{ ListBuffer, LinkedHashSet }
 import scala.tools.nsc.io.AbstractFile
 import scala.tools.nsc.symtab._
 import scala.tools.nsc.symtab.classfile.ClassfileConstants._
@@ -35,8 +35,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
     AnyClass, ObjectClass, ThrowsClass, ThrowableClass, ClassfileAnnotationClass,
     SerializableClass, StringClass, ClassClass, FunctionClass,
     DeprecatedAttr, SerializableAttr, SerialVersionUIDAttr, VolatileAttr,
-    TransientAttr, CloneableAttr, RemoteAttr,
-    getPrimitiveCompanion
+    TransientAttr, CloneableAttr, RemoteAttr
   }
 
   val phaseName = "jvm"
@@ -137,15 +136,19 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
     var jmethod: JMethod = _
 //    var jcode: JExtendedCode = _
 
-    // referenced inner classes
-    var innerClasses: immutable.Set[Symbol] = immutable.ListSet.empty
-
     val fjbgContext = new FJBGContext(49, 0)
 
     val emitSource = debugLevel >= 1
     val emitLines  = debugLevel >= 2
     val emitVars   = debugLevel >= 3
 
+    override def javaName(sym: Symbol): String = {
+      if (sym.isClass && !sym.rawowner.isPackageClass && !sym.isModuleClass)
+        innerClassBuffer += sym
+
+      super.javaName(sym)
+    }
+
     /** Write a class to disk, adding the Scala signature (pickled type
      *  information) and inner classes.
      *
@@ -206,12 +209,14 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
     var isRemoteClass: Boolean = false
     var isParcelableClass = false
 
+    private val innerClassBuffer = new ListBuffer[Symbol]
+
     def genClass(c: IClass) {
       val needsEnclosingMethod: Boolean =
         c.symbol.isClass && (c.symbol.originalEnclosingMethod != NoSymbol)
 
       clasz = c
-      innerClasses = immutable.ListSet.empty
+      innerClassBuffer.clear()
 
       var parents = c.symbol.info.parents
       var ifaces  = JClass.NO_INTERFACES
@@ -300,8 +305,12 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
       val sym = clazz.originalEnclosingMethod
       if (sym.isMethod) {
         log("enclosing method for %s is %s".format(clazz, sym))
-        var outerName = javaName(sym.enclClass)
-        jclass addAttribute fjbgContext.JEnclosingMethodAttribute(jclass, outerName, javaName(sym), javaType(sym))
+        jclass addAttribute fjbgContext.JEnclosingMethodAttribute(
+          jclass,
+          javaName(sym.enclClass),
+          javaName(sym),
+          javaType(sym)
+        )
       }
     }
 
@@ -334,7 +343,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
 	         !m.symbol.isGetter &&
 	         !m.symbol.isSetter) yield javaName(m.symbol)
 
-      val constructor = beanInfoClass.addNewMethod(ACC_PUBLIC, "<init>", JType.VOID, javaTypes(Nil), javaNames(Nil))
+      val constructor = beanInfoClass.addNewMethod(ACC_PUBLIC, "<init>", JType.VOID, new Array[JType](0), new Array[String](0))
       val jcode = constructor.getCode().asInstanceOf[JExtendedCode]
       val strKind = new JObjectType(javaName(StringClass))
       val stringArrayKind = new JArrayType(strKind)
@@ -530,7 +539,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
             verifySig(sym, sig)
 
           val index = jmember.getConstantPool.addUtf8(sig).toShort
-          if (settings.debug.value && settings.verbose.value)
+          if (opt.verboseDebug)
             atPhase(currentRun.erasurePhase) {
               println("add generic sig "+sym+":"+sym.info+" ==> "+sig+" @ "+index)
             }
@@ -587,9 +596,6 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
     }
 
     def addInnerClasses(jclass: JClass) {
-      def addOwnInnerClasses(cls: Symbol): Unit =
-        innerClasses ++= (cls.info.decls filter (_.isClass))
-
       /** The outer name for this inner class. Note that it returns null
        *  when the inner class should not get an index in the constant pool.
        *  That means non-member classes (anonymous). See Section 4.7.5 in the JVMS.
@@ -598,11 +604,9 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
         if (innerSym.isAnonymousClass || innerSym.isAnonymousFunction || innerSym.originalEnclosingMethod != NoSymbol)
           null
         else {
-          var outerName = javaName(innerSym.rawowner)
-          // remove the trailing '$'
-          if (outerName.endsWith("$") && isTopLevelModule(innerSym.rawowner))
-            outerName = outerName dropRight 1
-          outerName
+          val outerName = javaName(innerSym.rawowner)
+          if (isTopLevelModule(innerSym.rawowner)) outerName stripSuffix "$"
+          else outerName
         }
       }
 
@@ -614,25 +618,30 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
 
       // add inner classes which might not have been referenced yet
       atPhase(currentRun.erasurePhase.next) {
-        addOwnInnerClasses(clasz.symbol)
-        addOwnInnerClasses(clasz.symbol.linkedClassOfClass)
+        for (sym <- List(clasz.symbol, clasz.symbol.linkedClassOfClass) ; m <- sym.info.decls ; if m.isClass)
+          innerClassBuffer += m
       }
 
-      if (!innerClasses.isEmpty) {
+      val allInners = innerClassBuffer.toList
+      if (allInners.nonEmpty) {
         val innerClassesAttr = jclass.getInnerClasses()
         // sort them so inner classes succeed their enclosing class
         // to satisfy the Eclipse Java compiler
         //for (innerSym <- innerClasses.toList sortBy (_.name.length)) {
-        for (innerSym <- innerClasses.toList sortBy (_.name.length);
-          outer = outerName(innerSym) if outer != null) {
-          var flags = javaFlags(innerSym)
-          if (innerSym.rawowner.hasModuleFlag)
-            flags |= ACC_STATIC
-
-          innerClassesAttr.addEntry(javaName(innerSym),
-              outer, //outerName(innerSym),
+        for (innerSym <- allInners.distinct sortBy (_.name.length)) {
+          val outer = outerName(innerSym)
+          if (outer != null) {
+            var flags = javaFlags(innerSym)
+            if (innerSym.rawowner.hasModuleFlag)
+              flags |= ACC_STATIC
+
+            innerClassesAttr.addEntry(
+              javaName(innerSym),
+              outer,
               innerName(innerSym),
-              (flags & INNER_CLASSES_FLAGS))
+              (flags & INNER_CLASSES_FLAGS)
+            )
+          }
         }
       }
     }
@@ -686,8 +695,8 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
       jmethod = jclass.addNewMethod(flags,
                                     javaName(m.symbol),
                                     resTpe,
-                                    javaTypes(m.params map (_.kind)),
-                                    javaNames(m.params map (_.sym)))
+                                    m.params map (p => javaType(p.kind)) toArray,
+                                    m.params map (p => javaName(p.sym)) toArray)
 
       addRemoteException(jmethod, m.symbol)
 
@@ -873,17 +882,16 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
     /** Add a forwarder for method m */
     def addForwarder(jclass: JClass, module: Symbol, m: Symbol, accessFlags: Int) {
       val moduleName     = javaName(module)
-      val mirrorName     = moduleName dropRight 1 // dropping '$'
       val methodInfo     = module.thisType.memberInfo(m)
-      val paramJavaTypes = methodInfo.paramTypes map toTypeKind
-      val paramNames     = 0 until paramJavaTypes.length map ("x_" + _) toArray
+      val paramJavaTypes = methodInfo.paramTypes map javaType
+      val paramNames     = 0 until paramJavaTypes.length map ("x_" + _)
 
       val mirrorMethod = jclass.addNewMethod(
         accessFlags,
         javaName(m),
         javaType(methodInfo.resultType),
-        javaTypes(paramJavaTypes),
-        paramNames)
+        paramJavaTypes.toArray,
+        paramNames.toArray)
       val mirrorCode = mirrorMethod.getCode().asInstanceOf[JExtendedCode]
       mirrorCode.emitGETSTATIC(moduleName,
                                nme.MODULE_INSTANCE_FIELD.toString,
@@ -969,8 +977,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
      *  instead be made to add the forwarder methods to the companion class.
      */
     def dumpMirrorClass(clasz: Symbol, sourceFile: String) {
-      val moduleName = javaName(clasz) // + "$"
-      val mirrorName = moduleName dropRight 1
+      val mirrorName = javaName(clasz).init   // drops "$"
       val mirrorClass = fjbgContext.JClass(ACC_SUPER | ACC_PUBLIC | ACC_FINAL,
                                            mirrorName,
                                            JAVA_LANG_OBJECT.getName,
@@ -1726,12 +1733,6 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid {
 
     ////////////////////// Utilities ////////////////////////
 
-    override def javaName(sym: Symbol): String = {
-      if (sym.isClass && !sym.rawowner.isPackageClass && !sym.isModuleClass)
-        innerClasses = innerClasses + sym
-      super.javaName(sym)
-    }
-
     /** Calls to methods in 'sym' need invokeinterface? */
     def needsInterfaceCall(sym: Symbol): Boolean = {
       log("checking for interface call: " + sym.fullName)
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVMUtil.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVMUtil.scala
index 38c22b930c..13c802b2e4 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVMUtil.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVMUtil.scala
@@ -43,6 +43,21 @@ trait GenJVMUtil {
     DOUBLE -> new JObjectType("java.lang.Double")
   )
 
+  private val javaNameCache = {
+    val map = new mutable.HashMap[Symbol, String]()
+    map ++= List(
+      NothingClass        -> RuntimeNothingClass.fullName('/'),
+      RuntimeNothingClass -> RuntimeNothingClass.fullName('/'),
+      NullClass           -> RuntimeNullClass.fullName('/'),
+      RuntimeNullClass    -> RuntimeNullClass.fullName('/')
+    )
+    primitiveCompanions foreach { sym =>
+      map(sym) = "scala/runtime/" + sym.name + "$"
+    }
+
+    map
+  }
+
   /** This trait may be used by tools who need access to
    *  utility methods like javaName and javaType. (for instance,
    *  the Eclipse plugin uses it).
@@ -79,28 +94,13 @@ trait GenJVMUtil {
      *  references from method signatures to these types, because such classes can
      *  not exist in the classpath: the type checker will be very confused.
      */
-    def javaName(sym: Symbol): String = {
-      val suffix = moduleSuffix(sym)
-
-      if (sym == NothingClass)    javaName(RuntimeNothingClass)
-      else if (sym == NullClass)  javaName(RuntimeNullClass)
-      else getPrimitiveCompanion(sym.companionModule) match {
-        case Some(sym)  => javaName(sym)
-        case _          =>
-          val prefix =
-            if (sym.isClass || (sym.isModule && !sym.isMethod)) sym.fullName('/')
-            else sym.simpleName.toString.trim()
-
-          prefix + suffix
-      }
-    }
-
-    def javaNames(syms: List[Symbol]): Array[String] = {
-      val res = new Array[String](syms.length)
-      var i = 0
-      syms foreach (s => { res(i) = javaName(s); i += 1 })
-      res
-    }
+    def javaName(sym: Symbol): String =
+      javaNameCache.getOrElseUpdate(sym, {
+        if (sym.isClass || (sym.isModule && !sym.isMethod))
+          sym.fullName('/') + moduleSuffix(sym)
+        else
+          sym.simpleName.toString.trim() + moduleSuffix(sym)
+      })
 
     def javaType(t: TypeKind): JType = (t: @unchecked) match {
       case UNIT            => JType.VOID
@@ -122,17 +122,11 @@ trait GenJVMUtil {
       if (s.isMethod)
         new JMethodType(
           if (s.isClassConstructor) JType.VOID else javaType(s.tpe.resultType),
-          s.tpe.paramTypes.map(javaType).toArray)
+          s.tpe.paramTypes map javaType toArray
+        )
       else
         javaType(s.tpe)
 
-    def javaTypes(ts: List[TypeKind]): Array[JType] = {
-      val res = new Array[JType](ts.length)
-      var i = 0
-      ts foreach ( t => { res(i) = javaType(t); i += 1 } );
-      res
-    }
-
     protected def genConstant(jcode: JExtendedCode, const: Constant) {
       const.tag match {
         case UnitTag    => ()
diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
index b1ea837cf6..8490c26414 100644
--- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
@@ -623,11 +623,10 @@ trait Definitions extends reflect.generic.StandardDefinitions {
     val unboxMethod = new HashMap[Symbol, Symbol]     // Type -> Method
     val boxMethod = new HashMap[Symbol, Symbol]       // Type -> Method
     val primitiveCompanions = new HashSet[Symbol]     // AnyVal -> Companion
-
     /** Maps a companion object like scala.Int to scala.runtime.Int. */
-    def getPrimitiveCompanion(sym: Symbol) =
-      if (primitiveCompanions(sym)) Some(getModule("scala.runtime." + sym.name))
-      else None
+    lazy val runtimeCompanions = (primitiveCompanions map { sym =>
+      sym -> getModule("scala.runtime." + sym.name)
+    }).toMap
 
     def isUnbox(m: Symbol) = unboxMethod.valuesIterator contains m
     def isBox(m: Symbol) = boxMethod.valuesIterator contains m
diff --git a/src/compiler/scala/tools/nsc/symtab/Symbols.scala b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
index 543dac1665..3e03c92460 100644
--- a/src/compiler/scala/tools/nsc/symtab/Symbols.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
@@ -37,22 +37,20 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
     "_"+nextexid
   }
 
+  /** The original owner of a class. Used by the backend to generate
+   *  EnclosingMethod attributes.
+   */
+  val originalOwner = mutable.HashMap[Symbol, Symbol]()
+
   /** The class for all symbols */
   abstract class Symbol(initOwner: Symbol, initPos: Position, initName: Name) extends AbsSymbol {
     var rawowner = initOwner
+    var rawname  = initName
+    var rawflags = 0L
 
-    /** The original owner of this class. Used by the backend to generate
-     *  EnclosingMethod attributes. */
-    var originalOwner: Symbol = initOwner
-
-    var rawname = initName
-    var rawflags: Long = 0
     private var rawpos = initPos
-
     val id = { ids += 1; ids } // identity displayed when -uniqid
 
-//    assert(id != 7498, initName+"/"+initOwner)
-
     var validTo: Period = NoPeriod
 
     def pos = rawpos
@@ -596,7 +594,12 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
 // Flags, owner, and name attributes --------------------------------------------------------------
 
     def owner: Symbol = rawowner
-    override final def owner_=(owner: Symbol) { rawowner = owner }
+    override final def owner_=(owner: Symbol) {
+      if (originalOwner contains this) ()
+      else originalOwner(this) = rawowner
+
+      rawowner = owner
+    }
 
     def ownerChain: List[Symbol] = this :: owner.ownerChain
 
@@ -1199,8 +1202,7 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
      */
     def originalEnclosingMethod: Symbol = {
       if (isMethod) this
-      else
-        originalOwner.originalEnclosingMethod
+      else originalOwner.getOrElse(this, rawowner).originalEnclosingMethod
     }
 
     /** The top-level class containing this symbol */
@@ -1980,7 +1982,7 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
       } else if (phase.flatClasses && rawowner != NoSymbol && !rawowner.isPackageClass) {
         if (flatname == tpnme.EMPTY) {
           assert(rawowner.isClass, "fatal: %s has owner %s, but a class owner is required".format(rawname+idString, rawowner))
-          flatname = newTypeName(compactify(rawowner.name.toString() + "$" + rawname))
+          flatname = newTypeName(compactify(rawowner.name + "$" + rawname))
         }
         flatname
       } else rawname