diff options
author | Paul Phillips <paulp@improving.org> | 2010-01-15 18:00:51 +0000 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2010-01-15 18:00:51 +0000 |
commit | 9bd9b9fcc1623ca3b6c98cb7ce12f31581750372 (patch) | |
tree | f3a12a4788db0879fa06666caf335dedf5c3357a | |
parent | 3e1241caeca9af5b05922d38bed1b0480e6da56d (diff) | |
download | scala-9bd9b9fcc1623ca3b6c98cb7ce12f31581750372.tar.gz scala-9bd9b9fcc1623ca3b6c98cb7ce12f31581750372.tar.bz2 scala-9bd9b9fcc1623ca3b6c98cb7ce12f31581750372.zip |
Fix for #2365.
a soft reference around cached classes so they do not interfere with
garbage collection. There is a test case, but it is in pending because I
spent longer trying to get it to fail under partest than I did writing
the actual patch. If you would like to see the behavior which was
corrected, go to
test/pending/run/bug2365
and run that script with scalac built before and after this commit.
Review by dubochet.
-rw-r--r-- | src/compiler/scala/tools/nsc/ast/TreeDSL.scala | 5 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/ast/TreeGen.scala | 3 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/symtab/Definitions.scala | 2 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/CleanUp.scala | 50 | ||||
-rw-r--r-- | test/pending/run/bug2365/Test.scala | 35 | ||||
-rw-r--r-- | test/pending/run/bug2365/bug2365.javaopts | 1 | ||||
-rwxr-xr-x | test/pending/run/bug2365/run | 13 |
7 files changed, 77 insertions, 32 deletions
diff --git a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala index fb65f82740..a788015262 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala @@ -175,7 +175,10 @@ trait TreeDSL { if (target.tpe.typeSymbol == SomeClass) TRUE // is Some[_] else NOT(ID(target) DOT nme.isEmpty) // is Option[_] - def GET() = fn(ID(target), nme.get) + def IS_NULL() = REF(target) ANY_EQ NULL + def NOT_NULL() = REF(target) ANY_NE NULL + + def GET() = fn(REF(target), nme.get) // name of nth indexed argument to a method (first parameter list), defaults to 1st def ARG(idx: Int = 0) = Ident(target.paramss.head(idx)) diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala index c5a8b93c88..7f2bcf99c5 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala @@ -228,6 +228,9 @@ abstract class TreeGen def mkOr(tree1: Tree, tree2: Tree): Tree = Apply(Select(tree1, Boolean_or), List(tree2)) + // wrap the given expression in a SoftReference so it can be gc-ed + def mkSoftRef(expr: Tree): Tree = New(TypeTree(SoftReferenceClass.tpe), List(List(expr))) + def mkCached(cvar: Symbol, expr: Tree): Tree = { val cvarRef = if (cvar.owner.isClass) Select(This(cvar.owner), cvar) else Ident(cvar) Block( diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala index e1cf7a5a7e..e14a7d796c 100644 --- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala +++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala @@ -219,6 +219,8 @@ trait Definitions { def ArrayModule_apply = getMember(ArrayModule, nme.apply) // reflection / structural types + lazy val SoftReferenceClass = getClass("java.lang.ref.SoftReference") + lazy val WeakReferenceClass = getClass("java.lang.ref.WeakReference") lazy val MethodClass = getClass(sn.MethodAsObject) lazy val EmptyMethodCacheClass = getClass("scala.runtime.EmptyMethodCache") lazy val MethodCacheClass = getClass("scala.runtime.MethodCache") diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala index cf217dcd23..c508400711 100644 --- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala +++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala @@ -52,27 +52,6 @@ abstract class CleanUp extends Transform with ast.TreeDSL { private def typedWithPos(pos: Position)(tree: Tree) = localTyper typed { atPos(pos)(tree) } - private def classConstantMethod(pos: Position, sig: String): Symbol = - classConstantMeth.getOrElseUpdate(sig, { - val forName = getMember(ClassClass.linkedModuleOfClass, nme.forName) - val owner = currentOwner.enclClass - - val cvar = owner.newVariable(pos, unit.fresh.newName(pos, "class$Cache")) - .setFlag(PRIVATE | STATIC | MUTABLE | SYNTHETIC).setInfo(ClassClass.tpe) - owner.info.decls enter cvar - val cdef = typedWithPos(pos) { VAL(cvar) === NULL } - - val meth = owner.newMethod(pos, unit.fresh.newName(pos, "class$Method")) - .setFlag(PRIVATE | STATIC | SYNTHETIC).setInfo(MethodType(List(), ClassClass.tpe)) - owner.info.decls enter meth - val mdef = typedWithPos(pos)(DEF(meth) === - gen.mkCached(cvar, Apply(REF(forName), List(Literal(sig)))) - ) - - newDefs.append(cdef, mdef) - meth - }) - override def transformUnit(unit: CompilationUnit) = unit.body = transform(unit.body) @@ -185,7 +164,8 @@ abstract class CleanUp extends Transform with ast.TreeDSL { case MONO_CACHE => - /* Implementation of the cache is as follows for method "def xyz(a: A, b: B)": + /* Implementation of the cache is as follows for method "def xyz(a: A, b: B)" + (but with a SoftReference wrapping reflClass$Cache, similarly in the poly Cache) : var reflParams$Cache: Array[Class[_]] = Array[JClass](classOf[A], classOf[B]) @@ -210,16 +190,19 @@ abstract class CleanUp extends Transform with ast.TreeDSL { addStaticVariableToClass("reflMethod$Cache", MethodClass.tpe, NULL, false) val reflClassCacheSym: Symbol = - addStaticVariableToClass("reflClass$Cache", ClassClass.tpe, NULL, false) + addStaticVariableToClass("reflClass$Cache", SoftReferenceClass.tpe, NULL, false) def getMethodSym = ClassClass.tpe member nme.getMethod_ + def isCacheEmpty(receiver: Symbol): Tree = + reflClassCacheSym.IS_NULL() OR (reflClassCacheSym.GET() ANY_NE REF(receiver)) + addStaticMethodToClass("reflMethod$Method", List(ClassClass.tpe), MethodClass.tpe) { case Pair(reflMethodSym, List(forReceiverSym)) => BLOCK( - IF (REF(reflClassCacheSym) ANY_NE REF(forReceiverSym)) THEN BLOCK( + IF (isCacheEmpty(forReceiverSym)) THEN BLOCK( REF(reflMethodCacheSym) === ((REF(forReceiverSym) DOT getMethodSym)(LIT(method), REF(reflParamsCacheSym))) , - REF(reflClassCacheSym) === REF(forReceiverSym), + REF(reflClassCacheSym) === gen.mkSoftRef(REF(forReceiverSym)), UNIT ) ENDIF, REF(reflMethodCacheSym) @@ -228,7 +211,10 @@ abstract class CleanUp extends Transform with ast.TreeDSL { case POLY_CACHE => - /* Implementation of the cache is as follows for method "def xyz(a: A, b: B)": + /* Implementation of the cache is as follows for method "def xyz(a: A, b: B)" + (but with the addition of a SoftReference wrapped around the MethodCache holder + so that it does not interfere with classloader garbage collection, see ticket + #2365 for details): var reflParams$Cache: Array[Class[_]] = Array[JClass](classOf[A], classOf[B]) @@ -250,23 +236,25 @@ abstract class CleanUp extends Transform with ast.TreeDSL { val reflParamsCacheSym: Symbol = addStaticVariableToClass("reflParams$Cache", theTypeClassArray, fromTypesToClassArrayLiteral(paramTypes), true) - val reflPolyCacheSym: Symbol = - addStaticVariableToClass("reflPoly$Cache", MethodCacheClass.tpe, NEW(TypeTree(EmptyMethodCacheClass.tpe)), false) + def mkNewPolyCache = gen.mkSoftRef(NEW(TypeTree(EmptyMethodCacheClass.tpe))) + val reflPolyCacheSym: Symbol = addStaticVariableToClass("reflPoly$Cache", SoftReferenceClass.tpe, mkNewPolyCache, false) + def getPolyCache = fn(REF(reflPolyCacheSym), nme.get) AS_ATTR MethodCacheClass.tpe addStaticMethodToClass("reflMethod$Method", List(ClassClass.tpe), MethodClass.tpe) { case Pair(reflMethodSym, List(forReceiverSym)) => val methodSym = reflMethodSym.newVariable(ad.pos, mkTerm("method")) setInfo MethodClass.tpe BLOCK( - VAL(methodSym) === ((REF(reflPolyCacheSym) DOT methodCache_find)(REF(forReceiverSym))) , + IF (getPolyCache ANY_EQ NULL) THEN (REF(reflPolyCacheSym) === mkNewPolyCache) ENDIF, + VAL(methodSym) === ((getPolyCache DOT methodCache_find)(REF(forReceiverSym))) , IF (REF(methodSym) OBJ_!= NULL) . THEN (Return(REF(methodSym))) ELSE { def methodSymRHS = ((REF(forReceiverSym) DOT Class_getMethod)(LIT(method), REF(reflParamsCacheSym))) - def cacheRHS = ((REF(reflPolyCacheSym) DOT methodCache_add)(REF(forReceiverSym), REF(methodSym))) + def cacheRHS = ((getPolyCache DOT methodCache_add)(REF(forReceiverSym), REF(methodSym))) BLOCK( REF(methodSym) === methodSymRHS, - REF(reflPolyCacheSym) === cacheRHS, + REF(reflPolyCacheSym) === gen.mkSoftRef(cacheRHS), Return(REF(methodSym)) ) } diff --git a/test/pending/run/bug2365/Test.scala b/test/pending/run/bug2365/Test.scala new file mode 100644 index 0000000000..92b58f4a25 --- /dev/null +++ b/test/pending/run/bug2365/Test.scala @@ -0,0 +1,35 @@ +import scala.tools.nsc.io._ +import java.net.URL + +object A { def apply(d: { def apply(): Int}) = d.apply() } +object A2 { def apply(d: { def apply(): Int}) = d.apply() } +object A3 { def apply(d: { def apply(): Int}) = d.apply() } +object A4 { def apply(d: { def apply(): Int}) = d.apply() } + +class B extends Function0[Int] { + def apply() = 3 +} + +object Test +{ + type StructF0 = { def apply(): Int } + def main(args: Array[String]) { + for(i <- 0 until 150) + println(i + " " + test(A.apply) + " " + test(A2.apply) + " " + test(A3.apply) + " " + test(A3.apply)) + } + + def test(withF0: StructF0 => Int): Int = { + // Some large jar + val ivyJar = File("/local/lib/java/ivy.jar").toURL + // load a class in a separate loader that will be passed to A + val loader = new java.net.URLClassLoader(Array(File(".").toURL, ivyJar)) + // load a real class to fill perm gen space + Class.forName("org.apache.ivy.Ivy", true, loader).newInstance + // create a class from another class loader with an apply: Int method + val b = Class.forName("B", true, loader).newInstance + + // pass instance to a, which will call apply using structural type reflection. + // This should hold on to the class for B, which means bLoader will not get collected + withF0(b.asInstanceOf[StructF0]) + } +} diff --git a/test/pending/run/bug2365/bug2365.javaopts b/test/pending/run/bug2365/bug2365.javaopts new file mode 100644 index 0000000000..357e033c1c --- /dev/null +++ b/test/pending/run/bug2365/bug2365.javaopts @@ -0,0 +1 @@ +-XX:MaxPermSize=25M diff --git a/test/pending/run/bug2365/run b/test/pending/run/bug2365/run new file mode 100755 index 0000000000..f3c44ad086 --- /dev/null +++ b/test/pending/run/bug2365/run @@ -0,0 +1,13 @@ +#!/bin/sh +# +# This script should fail with any build of scala where #2365 +# is not fixed, and otherwise succeed. Failure means running out +# of PermGen space. + +CP=.:/local/lib/java/ivy.jar +# SCALAC=/scala/inst/28/bin/scalac +SCALAC=scalac +RUN_OPTS="-XX:MaxPermSize=25M -verbose:gc" + +$SCALAC -cp $CP *.scala +JAVA_OPTS="${RUN_OPTS}" scala -cp $CP Test |