Skip to content

Add optimization to reduce extra iterations of the safe init checker. #17057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 13, 2023
Merged

Add optimization to reduce extra iterations of the safe init checker. #17057

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8595d95
Add optimization to reduce extra iterations of the safe init checker.
q-ata Mar 7, 2023
1b5a2ff
Remove benchmarking output
q-ata Mar 7, 2023
170b36d
Add documentation for new field.
q-ata Mar 7, 2023
ddafb78
Implement suggested change.
q-ata Mar 13, 2023
7347e03
Fix * alignment in comment.
q-ata Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/init/Cache.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ 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.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider make it more precise in the comment: Whether any value in the output cache (this.current).

Minor: the comment * is not aligned.

protected var cacheUsed: Boolean = false

/** Used to avoid allocation, its state does not matter */
protected given MutableTreeWrapper = new MutableTreeWrapper

Expand All @@ -99,7 +105,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the init checker, if we read from global cache, we can think it's still not used. Maybe move it to the method get:

  def get(config: Config, expr: Tree): Option[Res] =
    val res = current.get(config, expr)
    cacheUsed = cacheUsed || res.nonEmpty
    res

value
case None =>
val assumeValue: Res =
this.last.get(config, expr) match
Expand All @@ -124,6 +132,8 @@ class Cache[Config, Res]:

def hasChanged = changed

def isUsed = cacheUsed

/** Prepare cache for the next iteration
*
* 1. Reset changed flag.
Expand All @@ -132,6 +142,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
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/init/Semantic.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,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()
Expand Down