From 291d1f033a790e97298210de29f09a9796406ae3 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sun, 23 Sep 2012 18:58:50 +0200 Subject: SI-6412 fixes leaks in Types.uniques This is the most blatant leak in reflection. There are others, but their impact is much smaller, therefore we'll fix them 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 --- test/files/run/reflection-mem-glbs.scala | 13 +++++++++++++ test/files/run/reflection-mem-tags.scala | 17 +++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 test/files/run/reflection-mem-glbs.scala create mode 100644 test/files/run/reflection-mem-tags.scala (limited to 'test/files') diff --git a/test/files/run/reflection-mem-glbs.scala b/test/files/run/reflection-mem-glbs.scala new file mode 100644 index 0000000000..3f29a914bc --- /dev/null +++ b/test/files/run/reflection-mem-glbs.scala @@ -0,0 +1,13 @@ +import scala.tools.partest.MemoryTest + +trait A { type T <: A } +trait B { type T <: B } + +object Test extends MemoryTest { + override def maxDelta = 10 + override def calcsPerIter = 50000 + override def calc() { + import scala.reflect.runtime.universe._ + glb(List(typeOf[A], typeOf[B])) + } +} \ No newline at end of file diff --git a/test/files/run/reflection-mem-tags.scala b/test/files/run/reflection-mem-tags.scala new file mode 100644 index 0000000000..8815e7dcd8 --- /dev/null +++ b/test/files/run/reflection-mem-tags.scala @@ -0,0 +1,17 @@ +import scala.tools.partest.MemoryTest + +trait A { type T <: A } +trait B { type T <: B } + +object Test extends MemoryTest { + override def maxDelta = 10 + override def calcsPerIter = 100000 + override def calc() { + import scala.reflect.runtime.universe._ + def foo = { + class A { def x = 2; def y: A = new A } + weakTypeOf[A { def z: Int }] + } + foo + } +} \ No newline at end of file -- cgit v1.2.3 From b403c1d7524ccdfc3455b5bc5d5363fdd9c82bec Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Mon, 24 Sep 2012 14:31:17 +0200 Subject: SI-6412 alleviates leaks in toolboxes 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 --- src/compiler/scala/tools/nsc/Global.scala | 4 +- .../scala/tools/reflect/ToolBoxFactory.scala | 21 ++- src/reflect/scala/reflect/internal/Importers.scala | 164 +++++++++++---------- test/files/run/reflection-mem-typecheck.scala | 26 ++++ test/pending/run/reflection-mem-eval.scala | 26 ++++ 5 files changed, 159 insertions(+), 82 deletions(-) create mode 100644 test/files/run/reflection-mem-typecheck.scala create mode 100644 test/pending/run/reflection-mem-eval.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 58fcee4b30..0fbd930ad7 100644 --- 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 index f985eedf99..790c78cbf5 100644 --- 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 index c116928d37..02209d0712 100644 --- 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,8 +58,10 @@ 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 @@ -53,70 +69,70 @@ trait Importers extends api.Importers { self: SymbolTable => 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 new file mode 100644 index 0000000000..a312c2c893 --- /dev/null +++ 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)) + } +} \ No newline at end of file diff --git a/test/pending/run/reflection-mem-eval.scala b/test/pending/run/reflection-mem-eval.scala new file mode 100644 index 0000000000..9045c44cd6 --- /dev/null +++ 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)) + } +} \ No newline at end of file -- cgit v1.2.3