Skip to content

Cache quote unpickling #12242

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/quoted/MacroExpansion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ object MacroExpansion {
ctx.property(MacroExpansionPosition)

def context(inlinedFrom: tpd.Tree)(using Context): Context =
ctx.fresh.setProperty(MacroExpansionPosition, SourcePosition(inlinedFrom.source, inlinedFrom.span)).setTypeAssigner(new Typer).withSource(inlinedFrom.source)
QuotesCache.init(ctx.fresh).setProperty(MacroExpansionPosition, SourcePosition(inlinedFrom.source, inlinedFrom.span)).setTypeAssigner(new Typer).withSource(inlinedFrom.source)
}

41 changes: 27 additions & 14 deletions compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -181,25 +181,38 @@ object PickledQuotes {

/** Unpickle TASTY bytes into it's tree */
private def unpickle(pickled: String | List[String], isType: Boolean)(using Context): Tree = {
val bytes = pickled match
case pickled: String => TastyString.unpickle(pickled)
case pickled: List[String] => TastyString.unpickle(pickled)
QuotesCache.getTree(pickled) match
case Some(tree) =>
quotePickling.println(s"**** Using cached quote for TASTY\n$tree")
treeOwner(tree) match
case Some(owner) =>
// Copy the cached tree to make sure the all definitions are unique.
TreeTypeMap(oldOwners = List(owner), newOwners = List(owner)).apply(tree)
case _ =>
tree
Copy link
Contributor

Choose a reason for hiding this comment

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

In case we have optimizations that make the TreeTypeMap and identity function, do we have a test to guard against such optimizations?

Copy link
Contributor Author

@nicolasstucki nicolasstucki Apr 29, 2021

Choose a reason for hiding this comment

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

We already do that optimization outside TreeTypeMap, for example in changeOwner.

We do have tests that would fail if this stops cloning the symbols.


quotePickling.println(s"**** unpickling quote from TASTY\n${TastyPrinter.show(bytes)}")
case _ =>
val bytes = pickled match
case pickled: String => TastyString.unpickle(pickled)
case pickled: List[String] => TastyString.unpickle(pickled)

quotePickling.println(s"**** unpickling quote from TASTY\n${TastyPrinter.show(bytes)}")

val mode = if (isType) UnpickleMode.TypeTree else UnpickleMode.Term
val unpickler = new DottyUnpickler(bytes, mode)
unpickler.enter(Set.empty)
val mode = if (isType) UnpickleMode.TypeTree else UnpickleMode.Term
val unpickler = new DottyUnpickler(bytes, mode)
unpickler.enter(Set.empty)

val tree = unpickler.tree
val tree = unpickler.tree
QuotesCache(pickled) = tree

// Make sure trees and positions are fully loaded
new TreeTraverser {
def traverse(tree: Tree)(using Context): Unit = traverseChildren(tree)
}.traverse(tree)
// Make sure trees and positions are fully loaded
new TreeTraverser {
def traverse(tree: Tree)(using Context): Unit = traverseChildren(tree)
}.traverse(tree)

quotePickling.println(i"**** unpickled quote\n$tree")
tree
quotePickling.println(i"**** unpickled quote\n$tree")

tree
}

}
28 changes: 28 additions & 0 deletions compiler/src/dotty/tools/dotc/quoted/QuotesCache.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package dotty.tools.dotc.quoted

import dotty.tools.dotc.core.Contexts._
import dotty.tools.dotc.util.Property
import dotty.tools.dotc.reporting.trace
import dotty.tools.dotc.ast.tpd

import scala.collection.mutable

object QuotesCache {
import tpd._

/** A key to be used in a context property that caches the unpickled trees */
private val QuotesCacheKey = new Property.Key[collection.mutable.Map[String | List[String], Tree]]


/** Get the cached tree of the quote */
def getTree(pickled: String | List[String])(using Context): Option[Tree] =
ctx.property(QuotesCacheKey).get.get(pickled)

/** Update the cached tree of the quote */
def update(pickled: String | List[String], tree: Tree)(using Context): Unit =
ctx.property(QuotesCacheKey).get.update(pickled, tree)

/** Context with a cache for quote trees and tasty bytes */
def init(ctx: FreshContext): ctx.type =
ctx.setProperty(QuotesCacheKey, collection.mutable.Map.empty)
}
8 changes: 6 additions & 2 deletions staging/src/scala/quoted/staging/QuoteDriver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package staging
import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.Driver
import dotty.tools.dotc.core.Contexts.{Context, ContextBase, FreshContext}
import dotty.tools.dotc.quoted.QuotesCache
import dotty.tools.io.{AbstractFile, Directory, PlainDirectory, VirtualDirectory}
import dotty.tools.repl.AbstractFileClassLoader
import dotty.tools.dotc.reporting._
Expand Down Expand Up @@ -33,8 +34,11 @@ private class QuoteDriver(appClassloader: ClassLoader) extends Driver:
new VirtualDirectory("<quote compilation output>")
end outDir

val ctx0 = setup(settings.compilerArgs.toArray :+ "dummy.scala", initCtx.fresh).get._2
val ctx = setCompilerSettings(ctx0.fresh.setSetting(ctx0.settings.outputDir, outDir), settings)
val ctx = {
val ctx0 = QuotesCache.init(initCtx.fresh)
val ctx1 = setup(settings.compilerArgs.toArray :+ "dummy.scala", ctx0).get._2
setCompilerSettings(ctx1.fresh.setSetting(ctx1.settings.outputDir, outDir), settings)
}

new QuoteCompiler().newRun(ctx).compileExpr(exprBuilder) match
case Right(value) =>
Expand Down