SI-6412 alleviates leaks in toolboxes

xeno-by · xeno-by · commit b403c1d7524c · 2012-09-24T14:31:17.000+02:00
Turns importer caches into fully weak hash maps, and also applies manual cleanup to toolboxes every time they are used. It's not enough, because reflection-mem-typecheck test is still leaking at a rate of ~100kb per typecheck, but it's much better than it was before. We'll fix the rest later, after 2.10.0-final. For more information, see https://issues.scala-lang.org/browse/SI-6412 and http://groups.google.com/group/scala-internals/browse_thread/thread/eabcf3d406dab8b2
diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
@@ -1079,12 +1079,12 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
    *  of what file was being compiled when it broke.  Since I really
    *  really want to know, this hack.
    */
-  private var lastSeenSourceFile: SourceFile = NoSourceFile
+  protected var lastSeenSourceFile: SourceFile = NoSourceFile
 
   /** Let's share a lot more about why we crash all over the place.
    *  People will be very grateful.
    */
-  private var lastSeenContext: analyzer.Context = null
+  protected var lastSeenContext: analyzer.Context = null
 
   /** The currently active run
    */
diff --git a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala
@@ -46,6 +46,23 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
         newTermName("__wrapper$" + wrapCount + "$" + java.util.UUID.randomUUID.toString.replace("-", ""))
       }
 
+      // should be called after every use of ToolBoxGlobal in order to prevent leaks
+      // there's the `withCleanupCaches` method defined below, which provides a convenient interface for that
+      def cleanupCaches(): Unit = {
+        lub(List(IntTpe, IntTpe)) // indirectly clears lubResults and glbResults
+                                  // I didn't want to turn those caches from private to protected
+                                  // in order not to screw up the performance
+        perRunCaches.clearAll()
+        undoLog.clear()
+        analyzer.lastTreeToTyper = EmptyTree
+        lastSeenSourceFile = NoSourceFile
+        lastSeenContext = null
+      }
+
+      def withCleanupCaches[T](body: => T): T =
+        try body
+        finally cleanupCaches()
+
       def verify(expr: Tree): Unit = {
         // Previously toolboxes used to typecheck their inputs before compiling.
         // Actually, the initial demo by Martin first typechecked the reified tree,
@@ -337,7 +354,7 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
     lazy val importer = compiler.mkImporter(u)
     lazy val exporter = importer.reverse
 
-    def typeCheck(tree: u.Tree, expectedType: u.Type, silent: Boolean = false, withImplicitViewsDisabled: Boolean = false, withMacrosDisabled: Boolean = false): u.Tree = {
+    def typeCheck(tree: u.Tree, expectedType: u.Type, silent: Boolean = false, withImplicitViewsDisabled: Boolean = false, withMacrosDisabled: Boolean = false): u.Tree = compiler.withCleanupCaches {
       if (compiler.settings.verbose.value) println("importing "+tree+", expectedType = "+expectedType)
       var ctree: compiler.Tree = importer.importTree(tree)
       var cexpectedType: compiler.Type = importer.importType(expectedType)
@@ -357,7 +374,7 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
       inferImplicit(tree, viewTpe, isView = true, silent = silent, withMacrosDisabled = withMacrosDisabled, pos = pos)
     }
 
-    private def inferImplicit(tree: u.Tree, pt: u.Type, isView: Boolean, silent: Boolean, withMacrosDisabled: Boolean, pos: u.Position): u.Tree = {
+    private def inferImplicit(tree: u.Tree, pt: u.Type, isView: Boolean, silent: Boolean, withMacrosDisabled: Boolean, pos: u.Position): u.Tree = compiler.withCleanupCaches {
       if (compiler.settings.verbose.value) println("importing "+pt, ", tree = "+tree+", pos = "+pos)
       var ctree: compiler.Tree = importer.importTree(tree)
       var cpt: compiler.Type = importer.importType(pt)
diff --git a/src/reflect/scala/reflect/internal/Importers.scala b/src/reflect/scala/reflect/internal/Importers.scala
@@ -1,6 +1,8 @@
 package scala.reflect
 package internal
+
 import scala.collection.mutable.WeakHashMap
+import scala.ref.WeakReference
 
 // SI-6241: move importers to a mirror
 trait Importers extends api.Importers { self: SymbolTable =>
@@ -26,8 +28,20 @@ trait Importers extends api.Importers { self: SymbolTable =>
 
     val from: SymbolTable
 
-    lazy val symMap: WeakHashMap[from.Symbol, Symbol] = new WeakHashMap
-    lazy val tpeMap: WeakHashMap[from.Type, Type] = new WeakHashMap
+    protected lazy val symMap = new Cache[from.Symbol, Symbol]()
+    protected lazy val tpeMap = new Cache[from.Type, Type]()
+    protected class Cache[K <: AnyRef, V <: AnyRef] extends WeakHashMap[K, WeakReference[V]] {
+      def weakGet(key: K): Option[V] = this get key flatMap WeakReference.unapply
+      def weakUpdate(key: K, value: V) = this.update(key, WeakReference(value))
+      def weakGetOrElseUpdate(key: K)(value: => V): V =
+        weakGet(key) match {
+          case Some(result) => result
+          case None =>
+            val result = value
+            this(key) = WeakReference(result)
+            result
+        }
+    }
 
     // fixups and maps prevent stackoverflows in importer
     var pendingSyms = 0
@@ -44,79 +58,81 @@ trait Importers extends api.Importers { self: SymbolTable =>
 
     object reverse extends from.StandardImporter {
       val from: self.type = self
-      for ((fromsym, mysym) <- StandardImporter.this.symMap) symMap += ((mysym, fromsym))
-      for ((fromtpe, mytpe) <- StandardImporter.this.tpeMap) tpeMap += ((mytpe, fromtpe))
+      // FIXME this and reverse should be constantly kept in sync
+      // not just synced once upon the first usage of reverse
+      for ((fromsym, WeakReference(mysym)) <- StandardImporter.this.symMap) symMap += ((mysym, WeakReference(fromsym)))
+      for ((fromtpe, WeakReference(mytpe)) <- StandardImporter.this.tpeMap) tpeMap += ((mytpe, WeakReference(fromtpe)))
     }
 
     // todo. careful import of positions
     def importPosition(pos: from.Position): Position =
       pos.asInstanceOf[Position]
 
     def importSymbol(sym0: from.Symbol): Symbol = {
-      def doImport(sym: from.Symbol): Symbol = {
-        if (symMap.contains(sym))
-          return symMap(sym)
-
-        val myowner = importSymbol(sym.owner)
-        val mypos   = importPosition(sym.pos)
-        val myname  = importName(sym.name).toTermName
-        val myflags = sym.flags
-        def linkReferenced(mysym: TermSymbol, x: from.TermSymbol, op: from.Symbol => Symbol): Symbol = {
-          symMap(x) = mysym
-          mysym.referenced = op(x.referenced)
-          mysym
-        }
-        val mysym = sym match {
-          case x: from.MethodSymbol =>
-            linkReferenced(myowner.newMethod(myname, mypos, myflags), x, importSymbol)
-          case x: from.ModuleSymbol =>
-            linkReferenced(myowner.newModuleSymbol(myname, mypos, myflags), x, importSymbol)
-          case x: from.FreeTermSymbol =>
-            newFreeTermSymbol(importName(x.name).toTermName, x.value, x.flags, x.origin) setInfo importType(x.info)
-          case x: from.FreeTypeSymbol =>
-            newFreeTypeSymbol(importName(x.name).toTypeName, x.flags, x.origin)
-          case x: from.TermSymbol =>
-            linkReferenced(myowner.newValue(myname, mypos, myflags), x, importSymbol)
-          case x: from.TypeSkolem =>
-            val origin = x.unpackLocation match {
-              case null           => null
-              case y: from.Tree   => importTree(y)
-              case y: from.Symbol => importSymbol(y)
+      def doImport(sym: from.Symbol): Symbol =
+        symMap weakGet sym match {
+          case Some(result) => result
+          case _ =>
+            val myowner = importSymbol(sym.owner)
+            val mypos   = importPosition(sym.pos)
+            val myname  = importName(sym.name).toTermName
+            val myflags = sym.flags
+            def linkReferenced(mysym: TermSymbol, x: from.TermSymbol, op: from.Symbol => Symbol): Symbol = {
+              symMap.weakUpdate(x, mysym)
+              mysym.referenced = op(x.referenced)
+              mysym
             }
-            myowner.newTypeSkolemSymbol(myname.toTypeName, origin, mypos, myflags)
-          case x: from.ModuleClassSymbol =>
-            val mysym = myowner.newModuleClass(myname.toTypeName, mypos, myflags)
-            symMap(x) = mysym
-            mysym.sourceModule = importSymbol(x.sourceModule)
-            mysym
-          case x: from.ClassSymbol =>
-            val mysym = myowner.newClassSymbol(myname.toTypeName, mypos, myflags)
-            symMap(x) = mysym
-            if (sym.thisSym != sym) {
-              mysym.typeOfThis = importType(sym.typeOfThis)
-              mysym.thisSym setName importName(sym.thisSym.name)
+            val mysym = sym match {
+              case x: from.MethodSymbol =>
+                linkReferenced(myowner.newMethod(myname, mypos, myflags), x, importSymbol)
+              case x: from.ModuleSymbol =>
+                linkReferenced(myowner.newModuleSymbol(myname, mypos, myflags), x, importSymbol)
+              case x: from.FreeTermSymbol =>
+                newFreeTermSymbol(importName(x.name).toTermName, x.value, x.flags, x.origin) setInfo importType(x.info)
+              case x: from.FreeTypeSymbol =>
+                newFreeTypeSymbol(importName(x.name).toTypeName, x.flags, x.origin)
+              case x: from.TermSymbol =>
+                linkReferenced(myowner.newValue(myname, mypos, myflags), x, importSymbol)
+              case x: from.TypeSkolem =>
+                val origin = x.unpackLocation match {
+                  case null           => null
+                  case y: from.Tree   => importTree(y)
+                  case y: from.Symbol => importSymbol(y)
+                }
+                myowner.newTypeSkolemSymbol(myname.toTypeName, origin, mypos, myflags)
+              case x: from.ModuleClassSymbol =>
+                val mysym = myowner.newModuleClass(myname.toTypeName, mypos, myflags)
+                symMap.weakUpdate(x, mysym)
+                mysym.sourceModule = importSymbol(x.sourceModule)
+                mysym
+              case x: from.ClassSymbol =>
+                val mysym = myowner.newClassSymbol(myname.toTypeName, mypos, myflags)
+                symMap.weakUpdate(x, mysym)
+                if (sym.thisSym != sym) {
+                  mysym.typeOfThis = importType(sym.typeOfThis)
+                  mysym.thisSym setName importName(sym.thisSym.name)
+                }
+                mysym
+              case x: from.TypeSymbol =>
+                myowner.newTypeSymbol(myname.toTypeName, mypos, myflags)
             }
-            mysym
-          case x: from.TypeSymbol =>
-            myowner.newTypeSymbol(myname.toTypeName, mypos, myflags)
-        }
-        symMap(sym) = mysym
-        mysym setFlag Flags.LOCKED
-        mysym setInfo {
-          val mytypeParams = sym.typeParams map importSymbol
-          new LazyPolyType(mytypeParams) {
-            override def complete(s: Symbol) {
-              val result = sym.info match {
-                case from.PolyType(_, res) => res
-                case result => result
+            symMap.weakUpdate(sym, mysym)
+            mysym setFlag Flags.LOCKED
+            mysym setInfo {
+              val mytypeParams = sym.typeParams map importSymbol
+              new LazyPolyType(mytypeParams) {
+                override def complete(s: Symbol) {
+                  val result = sym.info match {
+                    case from.PolyType(_, res) => res
+                    case result => result
+                  }
+                  s setInfo GenPolyType(mytypeParams, importType(result))
+                  s setAnnotations (sym.annotations map importAnnotationInfo)
+                }
               }
-              s setInfo GenPolyType(mytypeParams, importType(result))
-              s setAnnotations (sym.annotations map importAnnotationInfo)
             }
-          }
-        }
-        mysym resetFlag Flags.LOCKED
-      } // end doImport
+            mysym resetFlag Flags.LOCKED
+        } // end doImport
 
       def importOrRelink: Symbol = {
         val sym = sym0 // makes sym visible in the debugger
@@ -186,14 +202,10 @@ trait Importers extends api.Importers { self: SymbolTable =>
       } // end importOrRelink
 
       val sym = sym0
-      if (symMap contains sym) {
-        symMap(sym)
-      } else {
+      symMap.weakGetOrElseUpdate(sym) {
         pendingSyms += 1
-
-        try {
-          symMap getOrElseUpdate (sym, importOrRelink)
-        } finally {
+        try importOrRelink
+        finally {
           pendingSyms -= 1
           tryFixup()
         }
@@ -258,14 +270,10 @@ trait Importers extends api.Importers { self: SymbolTable =>
       def importOrRelink: Type =
         doImport(tpe)
 
-      if (tpeMap contains tpe) {
-        tpeMap(tpe)
-      } else {
+      tpeMap.weakGetOrElseUpdate(tpe) {
         pendingTpes += 1
-
-        try {
-          tpeMap getOrElseUpdate (tpe, importOrRelink)
-        } finally {
+        try importOrRelink
+        finally {
           pendingTpes -= 1
           tryFixup()
         }
diff --git a/test/files/run/reflection-mem-typecheck.scala b/test/files/run/reflection-mem-typecheck.scala
@@ -0,0 +1,26 @@
+import scala.tools.partest.MemoryTest
+
+trait A { type T <: A }
+trait B { type T <: B }
+
+object Test extends MemoryTest {
+  lazy val tb = {
+    import scala.reflect.runtime.universe._
+    import scala.reflect.runtime.{currentMirror => cm}
+    import scala.tools.reflect.ToolBox
+    cm.mkToolBox()
+  }
+
+  override def maxDelta = 10
+  override def calcsPerIter = 8
+  override def calc() {
+    var snippet = """
+      trait A { type T <: A }
+      trait B { type T <: B }
+      def foo[T](x: List[T]) = x
+      foo(List(new A {}, new B {}))
+    """.trim
+    snippet = snippet + "\n" + (List.fill(50)(snippet.split("\n").last) mkString "\n")
+    tb.typeCheck(tb.parse(snippet))
+  }
+}
diff --git a/test/pending/run/reflection-mem-eval.scala b/test/pending/run/reflection-mem-eval.scala
@@ -0,0 +1,26 @@
+import scala.tools.partest.MemoryTest
+
+trait A { type T <: A }
+trait B { type T <: B }
+
+object Test extends MemoryTest {
+  lazy val tb = {
+    import scala.reflect.runtime.universe._
+    import scala.reflect.runtime.{currentMirror => cm}
+    import scala.tools.reflect.ToolBox
+    cm.mkToolBox()
+  }
+
+  override def maxDelta = 10
+  override def calcsPerIter = 3
+  override def calc() {
+    var snippet = """
+      trait A { type T <: A }
+      trait B { type T <: B }
+      def foo[T](x: List[T]) = x
+      foo(List(new A {}, new B {}))
+    """.trim
+    snippet = snippet + "\n" + (List.fill(50)(snippet.split("\n").last) mkString "\n")
+    tb.eval(tb.parse(snippet))
+  }
+}
