From 776bb540ea5c8f89cc30ca787c18b7965c639397 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 26 Mar 2020 21:09:45 +0100 Subject: [PATCH 1/2] LazyAnnotation: avoid unnecessary memory leaks When a deferred annotation has been completed, let the completer closure be garbage-collected to avoid potential leaks. --- .../dotty/tools/dotc/core/Annotations.scala | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Annotations.scala b/compiler/src/dotty/tools/dotc/core/Annotations.scala index 749b8c52e68d..823e256f268b 100644 --- a/compiler/src/dotty/tools/dotc/core/Annotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Annotations.scala @@ -48,16 +48,31 @@ object Annotations { } abstract class LazyAnnotation extends Annotation { - override def symbol(implicit ctx: Context): Symbol - def complete(implicit ctx: Context): Tree - - private var myTree: Tree = null - def tree(implicit ctx: Context): Tree = { - if (myTree == null) myTree = complete(ctx) - myTree - } + protected var mySym: Symbol | (Context => Symbol) + override def symbol(using ctx: Context): Symbol = + assert(mySym != null) + mySym match { + case symFn: (Context => Symbol) @unchecked => + mySym = null + mySym = symFn(ctx) + case sym: Symbol if sym.defRunId != ctx.runId => + mySym = sym.denot.current.symbol + case _ => + } + mySym.asInstanceOf[Symbol] + + protected var myTree: Tree | (Context => Tree) + def tree(using ctx: Context): Tree = + assert(myTree != null) + myTree match { + case treeFn: (Context => Tree) @unchecked => + myTree = null + myTree = treeFn(ctx) + case _ => + } + myTree.asInstanceOf[Tree] - override def isEvaluated: Boolean = myTree != null + override def isEvaluated: Boolean = myTree.isInstanceOf[Tree] } /** An annotation indicating the body of a right-hand side, @@ -120,23 +135,15 @@ object Annotations { /** Create an annotation where the tree is computed lazily. */ def deferred(sym: Symbol)(treeFn: Context ?=> Tree)(implicit ctx: Context): Annotation = new LazyAnnotation { - override def symbol(implicit ctx: Context): Symbol = sym - def complete(implicit ctx: Context) = treeFn(using ctx) + protected var myTree: Tree | (Context => Tree) = ctx => treeFn(using ctx) + protected var mySym: Symbol | (Context => Symbol) = sym } /** Create an annotation where the symbol and the tree are computed lazily. */ - def deferredSymAndTree(symf: Context ?=> Symbol)(treeFn: Context ?=> Tree)(implicit ctx: Context): Annotation = + def deferredSymAndTree(symFn: Context ?=> Symbol)(treeFn: Context ?=> Tree)(implicit ctx: Context): Annotation = new LazyAnnotation { - private var mySym: Symbol = _ - - override def symbol(implicit ctx: Context): Symbol = { - if (mySym == null || mySym.defRunId != ctx.runId) { - mySym = symf(using ctx) - assert(mySym != null) - } - mySym - } - def complete(implicit ctx: Context) = treeFn(using ctx) + protected var mySym: Symbol | (Context => Symbol) = ctx => symFn(using ctx) + protected var myTree: Tree | (Context => Tree) = ctx => treeFn(using ctx) } def deferred(atp: Type, args: List[Tree])(implicit ctx: Context): Annotation = From 5101901fdb3039f6c42f5072db23bd27b55ef52e Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Thu, 26 Mar 2020 21:26:51 +0100 Subject: [PATCH 2/2] Make LazyBodyAnnotation more like LazyAnnotation And address the TODO comment by adding an apply method that takes a `Context ?=> Tree`. Also add `Annotation#isEvaluating`, previously `LazyBodyAnnotation#isEvaluated` was also returning true when evaluation was started but not finished which was confusing. --- .../dotty/tools/dotc/core/Annotations.scala | 40 ++++++++++++------- .../tools/dotc/core/tasty/TreeUnpickler.scala | 4 +- .../tools/dotc/typer/PrepareInlineable.scala | 6 +-- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Annotations.scala b/compiler/src/dotty/tools/dotc/core/Annotations.scala index 823e256f268b..81a33206a624 100644 --- a/compiler/src/dotty/tools/dotc/core/Annotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Annotations.scala @@ -35,6 +35,10 @@ object Annotations { def argumentConstant(i: Int)(implicit ctx: Context): Option[Constant] = for (ConstantType(c) <- argument(i) map (_.tpe)) yield c + /** The tree evaluaton is in progress. */ + def isEvaluating: Boolean = false + + /** The tree evaluation has finished. */ def isEvaluated: Boolean = true def ensureCompleted(implicit ctx: Context): Unit = tree @@ -72,6 +76,7 @@ object Annotations { } myTree.asInstanceOf[Tree] + override def isEvaluating: Boolean = myTree == null override def isEvaluated: Boolean = myTree.isInstanceOf[Tree] } @@ -87,24 +92,31 @@ object Annotations { override def ensureCompleted(implicit ctx: Context): Unit = () } - case class ConcreteBodyAnnotation(body: Tree) extends BodyAnnotation { + class ConcreteBodyAnnotation(body: Tree) extends BodyAnnotation { def tree(implicit ctx: Context): Tree = body } - case class LazyBodyAnnotation(private var bodyExpr: Context => Tree) extends BodyAnnotation { - // TODO: Make `bodyExpr` an IFT once #6865 os in bootstrap - private var evaluated = false - private var myBody: Tree = _ - def tree(implicit ctx: Context): Tree = { - if (evaluated) assert(myBody != null) - else { - evaluated = true - myBody = bodyExpr(ctx) - bodyExpr = null + abstract class LazyBodyAnnotation extends BodyAnnotation { + // Copy-pasted from LazyAnnotation to avoid having to turn it into a trait + protected var myTree: Tree | (Context => Tree) + def tree(using ctx: Context): Tree = + assert(myTree != null) + myTree match { + case treeFn: (Context => Tree) @unchecked => + myTree = null + myTree = treeFn(ctx) + case _ => } - myBody - } - override def isEvaluated: Boolean = evaluated + myTree.asInstanceOf[Tree] + + override def isEvaluating: Boolean = myTree == null + override def isEvaluated: Boolean = myTree.isInstanceOf[Tree] + } + + object LazyBodyAnnotation { + def apply(bodyFn: Context ?=> Tree): LazyBodyAnnotation = + new LazyBodyAnnotation: + protected var myTree: Tree | (Context => Tree) = ctx => bodyFn(using ctx) } object Annotation { diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 07737946ba62..a25d1563f602 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -581,9 +581,9 @@ class TreeUnpickler(reader: TastyReader, forkAt(templateStart).indexTemplateParams()(localContext(sym)) } else if (sym.isInlineMethod) - sym.addAnnotation(LazyBodyAnnotation { ctx0 => + sym.addAnnotation(LazyBodyAnnotation { (using ctx0: Context) => val ctx1 = localContext(sym)(ctx0).addMode(Mode.ReadPositions) - implicit val ctx: Context = sourceChangeContext(Addr(0))(ctx1) + given Context = sourceChangeContext(Addr(0))(ctx1) // avoids space leaks by not capturing the current context forkAt(rhsStart).readTerm() }) diff --git a/compiler/src/dotty/tools/dotc/typer/PrepareInlineable.scala b/compiler/src/dotty/tools/dotc/typer/PrepareInlineable.scala index 9c1c52aa7d8c..1cf6983fedec 100644 --- a/compiler/src/dotty/tools/dotc/typer/PrepareInlineable.scala +++ b/compiler/src/dotty/tools/dotc/typer/PrepareInlineable.scala @@ -216,12 +216,12 @@ object PrepareInlineable { inlined: Symbol, treeExpr: Context => Tree)(implicit ctx: Context): Unit = inlined.unforcedAnnotation(defn.BodyAnnot) match { case Some(ann: ConcreteBodyAnnotation) => - case Some(ann: LazyBodyAnnotation) if ann.isEvaluated => + case Some(ann: LazyBodyAnnotation) if ann.isEvaluated || ann.isEvaluating => case _ => if (!ctx.isAfterTyper) { val inlineCtx = ctx - inlined.updateAnnotation(LazyBodyAnnotation { _ => - implicit val ctx = inlineCtx + inlined.updateAnnotation(LazyBodyAnnotation { + given ctx as Context = inlineCtx val initialErrorCount = ctx.reporter.errorCount var inlinedBody = treeExpr(ctx) if (ctx.reporter.errorCount == initialErrorCount) {