Skip to content
This repository was archived by the owner on Sep 1, 2020. It is now read-only.

Commit 449f2a7

Browse files
committed
Lazy fields null out fields that are used only ...
Lazy fields null out fields that are used only in their initializer. When the lazy value is forced, it will null out all private fields that are used only by the current lazy value. This should reduce memory leaks, see scala#720
1 parent 4231751 commit 449f2a7

File tree

3 files changed

+103
-12
lines changed

3 files changed

+103
-12
lines changed

src/compiler/scala/tools/nsc/backend/icode/GenICode.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ abstract class GenICode extends SubComponent {
6969
override def apply(unit: CompilationUnit): Unit = {
7070
this.unit = unit
7171
unit.icode.clear
72-
log("Generating icode for " + unit)
72+
informProgress("Generating icode for " + unit)
7373
gen(unit.body)
7474
this.unit = null
7575
}

src/compiler/scala/tools/nsc/transform/Mixin.scala

Lines changed: 84 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@ package transform
99

1010
import symtab._
1111
import Flags._
12-
import scala.collection.mutable.ListBuffer
1312
import scala.tools.nsc.util.{Position,NoPosition}
14-
import scala.collection.mutable.HashMap
13+
import collection.mutable.{ListBuffer, HashMap}
1514

1615
abstract class Mixin extends InfoTransform with ast.TreeDSL {
1716
import global._
@@ -364,6 +363,50 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
364363
tp
365364
}
366365

366+
import scala.collection._
367+
368+
/** Return a map of single-use fields to the lazy value that uses them during initialization.
369+
* Each field has to be private and defined in the enclosing class, and there must
370+
* be exactly one lazy value using it.
371+
*
372+
* Such fields will be nulled after the initializer has memoized the lazy value.
373+
*/
374+
def singleUseFields(templ: Template): collection.Map[Symbol, List[Symbol]] = {
375+
val usedIn = new mutable.HashMap[Symbol, List[Symbol]] {
376+
override def default(key: Symbol) = Nil
377+
}
378+
379+
object SingleUseTraverser extends Traverser {
380+
override def traverse(tree: Tree) {
381+
tree match {
382+
case Assign(lhs, rhs) => traverse(rhs) // assignments don't count
383+
case _ =>
384+
if (tree.hasSymbol && tree.symbol != NoSymbol) {
385+
val sym = tree.symbol
386+
if ((sym.hasFlag(ACCESSOR) || (sym.isTerm && !sym.isMethod))
387+
&& sym.hasFlag(PRIVATE)
388+
&& !(currentOwner.isGetter && currentOwner.accessed == sym) // getter
389+
&& !definitions.isValueClass(sym.tpe.resultType.typeSymbol)
390+
&& sym.owner == templ.symbol.owner
391+
&& !tree.isDef) {
392+
log("added use in: " + currentOwner + " -- " + tree)
393+
usedIn(sym) ::= currentOwner
394+
395+
}
396+
}
397+
super.traverse(tree)
398+
}
399+
}
400+
}
401+
SingleUseTraverser(templ)
402+
log("usedIn: " + usedIn)
403+
usedIn filter { pair =>
404+
val member = pair._2.head
405+
(member.isValue
406+
&& member.hasFlag(LAZY)
407+
&& pair._2.tail.isEmpty) }
408+
}
409+
367410
// --------- term transformation -----------------------------------------------
368411

369412
protected def newTransformer(unit: CompilationUnit): Transformer =
@@ -384,6 +427,9 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
384427
private var localTyper: erasure.Typer = _
385428
private def typedPos(pos: Position)(tree: Tree) = localTyper typed { atPos(pos)(tree) }
386429

430+
/** Map lazy values to the fields they should null after initialization. */
431+
private var lazyValNullables: mutable.MultiMap[Symbol, Symbol] = _
432+
387433
import scala.collection._
388434

389435
/** Map a field symbol to a unique integer denoting its position in the class layout.
@@ -612,17 +658,27 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
612658
* where bitmap$n is an int value acting as a bitmap of initialized values. It is
613659
* the 'n' is (offset / 32), the MASK is (1 << (offset % 32)).
614660
*/
615-
def mkLazyDef(clazz: Symbol, init: List[Tree], retVal: Tree, offset: Int): Tree = {
661+
def mkLazyDef(clazz: Symbol, lzyVal: Symbol, init: List[Tree], retVal: Tree, offset: Int): Tree = {
662+
def nullify(sym: Symbol): Tree = {
663+
val sym1 = if (sym.hasFlag(ACCESSOR)) sym.accessed else sym
664+
Select(This(clazz), sym1) === LIT(null)
665+
}
666+
667+
616668
val bitmapSym = bitmapFor(clazz, offset)
617669
val mask = LIT(1 << (offset % FLAGS_PER_WORD))
618670
def cond = mkTest(clazz, mask, bitmapSym, true)
671+
val nulls = (lazyValNullables(lzyVal).toList.sort(_.id < _.id) map nullify)
619672
def syncBody = init ::: List(mkSetFlag(clazz, offset), UNIT)
620673

674+
log("nulling fields inside " + lzyVal + ": " + nulls)
621675
val result =
622-
IF (cond) THEN gen.mkSynchronized(
623-
gen mkAttributedThis clazz,
624-
IF (cond) THEN BLOCK(syncBody: _*) ENDIF
625-
) ENDIF
676+
IF (cond) THEN BLOCK(
677+
(gen.mkSynchronized(
678+
gen mkAttributedThis clazz,
679+
IF (cond) THEN BLOCK(syncBody: _*) ENDIF
680+
)
681+
:: nulls): _*) ENDIF
626682

627683
typedPos(init.head.pos)(BLOCK(result, retVal))
628684
}
@@ -653,10 +709,10 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
653709
if sym.hasFlag(LAZY) && rhs != EmptyTree && !clazz.isImplClass =>
654710
assert(fieldOffset.isDefinedAt(sym))
655711
val rhs1 = if (sym.tpe.resultType.typeSymbol == UnitClass)
656-
mkLazyDef(clazz, List(rhs), UNIT, fieldOffset(sym))
712+
mkLazyDef(clazz, sym, List(rhs), UNIT, fieldOffset(sym))
657713
else {
658714
val Block(stats, res) = rhs
659-
mkLazyDef(clazz, stats, Select(This(clazz), res.symbol), fieldOffset(sym))
715+
mkLazyDef(clazz, sym, stats, Select(This(clazz), res.symbol), fieldOffset(sym))
660716
}
661717
treeCopy.DefDef(stat, mods, name, tp, vp, tpt, rhs1)
662718

@@ -790,13 +846,13 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
790846
if (sym.hasFlag(LAZY) && sym.isGetter) {
791847
val rhs1 =
792848
if (sym.tpe.resultType.typeSymbol == UnitClass)
793-
mkLazyDef(clazz, List(Apply(staticRef(initializer(sym)), List(gen.mkAttributedThis(clazz)))), UNIT, fieldOffset(sym))
849+
mkLazyDef(clazz, sym, List(Apply(staticRef(initializer(sym)), List(gen.mkAttributedThis(clazz)))), UNIT, fieldOffset(sym))
794850
else {
795851
val assign = atPos(sym.pos) {
796852
Assign(Select(This(sym.accessed.owner), sym.accessed) /*gen.mkAttributedRef(sym.accessed)*/ ,
797853
Apply(staticRef(initializer(sym)), gen.mkAttributedThis(clazz) :: Nil))
798854
}
799-
mkLazyDef(clazz, List(assign), Select(This(clazz), sym.accessed), fieldOffset(sym))
855+
mkLazyDef(clazz, sym, List(assign), Select(This(clazz), sym.accessed), fieldOffset(sym))
800856
}
801857
rhs1
802858
} else if (sym.getter(sym.owner).tpe.resultType.typeSymbol == UnitClass) {
@@ -855,6 +911,22 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
855911
stats1
856912
}
857913

914+
private def nullableFields(templ: Template) = {
915+
val nullables = new mutable.HashMap[Symbol, mutable.Set[Symbol]] with mutable.MultiMap[Symbol, Symbol] {
916+
override def default(key: Symbol) = mutable.Set.empty
917+
}
918+
919+
// if there are no lazy fields, take the fast path and save a traversal of the whole AST
920+
if (templ.symbol.owner.info.decls.exists(_.hasFlag(LAZY))) {
921+
// check what fields can be nulled for
922+
val uses = singleUseFields(templ)
923+
for ((field, users) <- uses; lazyFld <- users) {
924+
nullables.addBinding(lazyFld, field)
925+
}
926+
}
927+
nullables
928+
}
929+
858930
/** The transform that gets applied to a tree after it has been completely
859931
* traversed and possible modified by a preTransform.
860932
* This step will
@@ -884,6 +956,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
884956
// change parents of templates to conform to parents in the symbol info
885957
val parents1 = currentOwner.info.parents map (t => TypeTree(t) setPos tree.pos)
886958

959+
lazyValNullables = nullableFields(tree.asInstanceOf[Template])
887960
// add all new definitions to current class or interface
888961
val body1 = addNewDefs(currentOwner, body)
889962

test/files/run/lazy-leaks.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class Lazy(f: => Int) {
2+
lazy val get: Int = f
3+
}
4+
5+
object Test extends Application
6+
{
7+
val buffer = new scala.collection.mutable.ListBuffer[Lazy]
8+
9+
// This test requires 4 Mb of RAM if Lazy is discarding thunks
10+
// It consumes 4 Gb of RAM if Lazy is not discarding thunks
11+
12+
for (val idx <- Iterator.range(0, 1024)) {
13+
val data = new Array[Int](1024*1024)
14+
val lz: Lazy = new Lazy(data.length)
15+
buffer += lz
16+
lz.get
17+
}
18+
}

0 commit comments

Comments
 (0)