From 8595d951a9adcc2c57f7ea17b83fb854cae1cf7a Mon Sep 17 00:00:00 2001 From: David Hua Date: Mon, 6 Mar 2023 22:02:01 -0500 Subject: [PATCH 1/5] Add optimization to reduce extra iterations of the safe init checker. --- .../src/dotty/tools/dotc/transform/init/Cache.scala | 9 ++++++++- .../dotty/tools/dotc/transform/init/Semantic.scala | 13 ++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala index 14a52d995131..080c56d6fb5f 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala @@ -75,6 +75,8 @@ class Cache[Config, Res]: */ protected var changed: Boolean = false + protected var cacheUsed: Boolean = false + /** Used to avoid allocation, its state does not matter */ protected given MutableTreeWrapper = new MutableTreeWrapper @@ -99,7 +101,9 @@ class Cache[Config, Res]: */ def cachedEval(config: Config, expr: Tree, cacheResult: Boolean, default: Res)(eval: Tree => Res): Res = this.get(config, expr) match - case Some(value) => value + case Some(value) => + cacheUsed = true + value case None => val assumeValue: Res = this.last.get(config, expr) match @@ -124,6 +128,8 @@ class Cache[Config, Res]: def hasChanged = changed + def isUsed = cacheUsed + /** Prepare cache for the next iteration * * 1. Reset changed flag. @@ -132,6 +138,7 @@ class Cache[Config, Res]: */ def prepareForNextIteration()(using Context) = this.changed = false + this.cacheUsed = false this.last = this.current this.current = Map.empty end Cache diff --git a/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala b/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala index 286e3a124d12..49d9ae4d2b2a 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala @@ -1101,12 +1101,15 @@ object Semantic: * * The class to be checked must be an instantiable concrete class. */ - private def checkClass(classSym: ClassSymbol)(using Cache.Data, Context): Unit = + private def checkClass(classSym: ClassSymbol)(using Cache.Data, Context): Int = val thisRef = ThisRef(classSym) val tpl = classSym.defTree.asInstanceOf[TypeDef].rhs.asInstanceOf[Template] + var accum = 0 + @tailrec def iterate(): Unit = { + accum += 1 given Promoted = Promoted.empty(classSym) given Trace = Trace.empty.add(classSym.defTree) given reporter: Reporter.BufferedReporter = new Reporter.BufferedReporter @@ -1120,7 +1123,7 @@ object Semantic: log("checking " + classSym) { eval(tpl, thisRef, classSym) } reporter.errors.foreach(_.issue) - if cache.hasChanged && reporter.errors.isEmpty then + if cache.hasChanged && reporter.errors.isEmpty && cache.isUsed then // code to prepare cache and heap for next iteration cache.prepareForNextIteration() iterate() @@ -1129,15 +1132,19 @@ object Semantic: } iterate() + + accum - 1 end checkClass /** * Check the specified concrete classes */ def checkClasses(classes: List[ClassSymbol])(using Context): Unit = + var total = 0 given Cache.Data() for classSym <- classes if isConcreteClass(classSym) do - checkClass(classSym) + total += checkClass(classSym) + System.err.nn.println(total) // ----- Semantic definition -------------------------------- type ArgInfo = TraceValue[Value] From 1b5a2fff06d66b463105643c15e2a0dfa7dd3359 Mon Sep 17 00:00:00 2001 From: David Hua Date: Mon, 6 Mar 2023 23:14:41 -0500 Subject: [PATCH 2/5] Remove benchmarking output --- .../src/dotty/tools/dotc/transform/init/Semantic.scala | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala b/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala index 49d9ae4d2b2a..6a0c8cd4216d 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala @@ -1101,15 +1101,12 @@ object Semantic: * * The class to be checked must be an instantiable concrete class. */ - private def checkClass(classSym: ClassSymbol)(using Cache.Data, Context): Int = + private def checkClass(classSym: ClassSymbol)(using Cache.Data, Context): Unit = val thisRef = ThisRef(classSym) val tpl = classSym.defTree.asInstanceOf[TypeDef].rhs.asInstanceOf[Template] - var accum = 0 - @tailrec def iterate(): Unit = { - accum += 1 given Promoted = Promoted.empty(classSym) given Trace = Trace.empty.add(classSym.defTree) given reporter: Reporter.BufferedReporter = new Reporter.BufferedReporter @@ -1132,19 +1129,15 @@ object Semantic: } iterate() - - accum - 1 end checkClass /** * Check the specified concrete classes */ def checkClasses(classes: List[ClassSymbol])(using Context): Unit = - var total = 0 given Cache.Data() for classSym <- classes if isConcreteClass(classSym) do total += checkClass(classSym) - System.err.nn.println(total) // ----- Semantic definition -------------------------------- type ArgInfo = TraceValue[Value] From 170b36d04530fd9596494250041dd47513c62941 Mon Sep 17 00:00:00 2001 From: David Hua Date: Mon, 6 Mar 2023 23:16:36 -0500 Subject: [PATCH 3/5] Add documentation for new field. --- compiler/src/dotty/tools/dotc/transform/init/Cache.scala | 4 ++++ compiler/src/dotty/tools/dotc/transform/init/Semantic.scala | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala index 080c56d6fb5f..2dcd6360554a 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala @@ -75,6 +75,10 @@ class Cache[Config, Res]: */ protected var changed: Boolean = false + /** Whether any value in the cache was accessed after being added. + * If no cached values are used after they are added for the first time + * then another iteration of analysis is not needed. + */ protected var cacheUsed: Boolean = false /** Used to avoid allocation, its state does not matter */ diff --git a/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala b/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala index 6a0c8cd4216d..1aa38e1429df 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Semantic.scala @@ -1137,7 +1137,7 @@ object Semantic: def checkClasses(classes: List[ClassSymbol])(using Context): Unit = given Cache.Data() for classSym <- classes if isConcreteClass(classSym) do - total += checkClass(classSym) + checkClass(classSym) // ----- Semantic definition -------------------------------- type ArgInfo = TraceValue[Value] From ddafb7819c0f36b8583224174bf6de87a1a90c0c Mon Sep 17 00:00:00 2001 From: David Hua Date: Mon, 13 Mar 2023 03:01:21 -0400 Subject: [PATCH 4/5] Implement suggested change. --- compiler/src/dotty/tools/dotc/transform/init/Cache.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala index 2dcd6360554a..4d091d88eefd 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala @@ -85,7 +85,9 @@ class Cache[Config, Res]: protected given MutableTreeWrapper = new MutableTreeWrapper def get(config: Config, expr: Tree): Option[Res] = - current.get(config, expr) + val res = current.get(config, expr) + cacheUsed = cacheUsed || res.nonEmpty + res /** Evaluate an expression with cache * @@ -105,9 +107,7 @@ class Cache[Config, Res]: */ def cachedEval(config: Config, expr: Tree, cacheResult: Boolean, default: Res)(eval: Tree => Res): Res = this.get(config, expr) match - case Some(value) => - cacheUsed = true - value + case Some(value) => value case None => val assumeValue: Res = this.last.get(config, expr) match From 7347e03680f7d436f3c4817101e89f2e37da467c Mon Sep 17 00:00:00 2001 From: David Hua Date: Mon, 13 Mar 2023 04:31:39 -0400 Subject: [PATCH 5/5] Fix * alignment in comment. --- compiler/src/dotty/tools/dotc/transform/init/Cache.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala index 4d091d88eefd..36c0d8c175f8 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Cache.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Cache.scala @@ -75,10 +75,10 @@ class Cache[Config, Res]: */ protected var changed: Boolean = false - /** Whether any value in the cache was accessed after being added. - * If no cached values are used after they are added for the first time - * then another iteration of analysis is not needed. - */ + /** Whether any value in the output cache (this.current) was accessed + * after being added. If no cached values are used after they are added + * for the first time then another iteration of analysis is not needed. + */ protected var cacheUsed: Boolean = false /** Used to avoid allocation, its state does not matter */