Skip to content

Populate addresses of symbols, types and trees after pickler #416

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
Apr 3, 2015
Merged

Populate addresses of symbols, types and trees after pickler #416

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fbad1dc
Export pickler and pickled addresses so that other phases can pickle …
DarkDimius Mar 18, 2015
0d13962
Update address maps of symbols after compaction
DarkDimius Mar 18, 2015
af6f371
Populate addresses of symbols after pickler
DarkDimius Mar 18, 2015
5597014
Review of #394
DarkDimius Mar 18, 2015
e47483a
Address reviewer comments on #416
DarkDimius Mar 25, 2015
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
10 changes: 9 additions & 1 deletion src/dotty/tools/dotc/CompilationUnit.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package dotty.tools
package dotc

import dotty.tools.dotc.core.Types.Type
import dotty.tools.dotc.core.pickling.{TastyBuffer, TastyPickler}
import util.SourceFile
import ast.{tpd, untpd}
import TastyBuffer._
import dotty.tools.dotc.core.Symbols._

class CompilationUnit(val source: SourceFile) {

Expand All @@ -14,5 +18,9 @@ class CompilationUnit(val source: SourceFile) {

def isJava = source.file.name.endsWith(".java")

var pickled: Array[Byte] = Array()
lazy val pickled: TastyPickler = new TastyPickler()

var addrOfTree: tpd.Tree => Option[Addr] = (_ => None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add doc comments for these.


var addrOfSym: Symbol => Option[Addr] = (_ => None)
}
16 changes: 14 additions & 2 deletions src/dotty/tools/dotc/core/pickling/TreePickler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ class TreePickler(pickler: TastyPickler) {
op
fillRef(lengthAddr, currentAddr, relative = true)
}


def addrOfSym(sym: Symbol): Option[Addr] = {
symRefs.get(sym)
}

private var makeSymbolicRefsTo: Symbol = NoSymbol

/** All references to members of class `sym` are pickled
Expand Down Expand Up @@ -531,8 +535,16 @@ class TreePickler(pickler: TastyPickler) {
withLength { pickleType(ann.symbol.typeRef); pickleTree(ann.tree) }
}

def updateMapWithDeltas[T](mp: collection.mutable.Map[T, Addr]) = {
mp.map{
case (key, addr) => (key, adjusted(addr))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use a simple loop here:

for ((key, addr) <- mp.toList) mp(key) = adjusted(addr)

Shorter, clearer, faster.

Copy link
Member

Choose a reason for hiding this comment

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

Updating a mutable data structure during iteration doesn't work in general. I guess this is safe because you don't change the key set, but I would be cautious about using the same approach in general.

If you were to use an mutable.AnyRefMap rather than a mutable.Map (which is a good idea for performance reasons anyway), you can use mp.transformValues(adjusted).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's way I wrote the code this way: at the point when I am updating the datastructure, I'm already iterating over a different one.

Copy link
Contributor

Choose a reason for hiding this comment

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

@retronym: Good point. We could modify like this

for (key <- mp.keysIterator.toBuffer) mp(key) = adjusted(mp(key))

That looks like the most efficient safe solution. I believe efficiency is a concern here - these maps can become large.

Copy link
Member

Choose a reason for hiding this comment

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

Any reason to avoid AnyRefMap? IMO that should be the first choice when you need a map in the compiler. Its main claim to fame is that it can calls equals/hashCode rather than ==/``##` which is a non-negligible performance win.

}.foreach(mp += _)
}


trees.foreach(tree => if (!tree.isEmpty) pickleTree(tree))
assert(forwardSymRefs.isEmpty, i"unresolved symbols: ${forwardSymRefs.keySet.toList}%, %")
compactify()
}
updateMapWithDeltas(symRefs)
}
}
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/transform/NormalizeFlags.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import Flags._, Symbols._
* or alias type definition or a deferred val or def.
*/
class NormalizeFlags extends MiniPhaseTransform with SymTransformer { thisTransformer =>
override def phaseName = "elimLocals"
override def phaseName = "normalizeFlags"

def transformSym(ref: SymDenotation)(implicit ctx: Context) = {
var newFlags = ref.flags &~ Local
Expand Down
13 changes: 7 additions & 6 deletions src/dotty/tools/dotc/transform/Pickler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import Periods._
import Phases._
import collection.mutable

/** This miniphase pickles trees */
/** This phase pickles trees */
class Pickler extends Phase {
import ast.tpd._

Expand All @@ -32,19 +32,20 @@ class Pickler extends Phase {
pickling.println(i"unpickling in run ${ctx.runId}")
if (ctx.settings.YtestPickler.value) beforePickling(unit) = tree.show

val pickler = new TastyPickler
val pickler = unit.pickled
val treePkl = new TreePickler(pickler)
treePkl.pickle(tree :: Nil)
unit.addrOfTree = treePkl.buf.addrOfTree
unit.addrOfSym = treePkl.addrOfSym
if (tree.pos.exists)
new PositionPickler(pickler, treePkl.buf.addrOfTree).picklePositions(tree :: Nil, tree.pos)

unit.pickled = pickler.assembleParts()
def rawBytes = // not needed right now, but useful to print raw format.
unit.pickled.iterator.grouped(10).toList.zipWithIndex.map {
unit.pickled.assembleParts().iterator.grouped(10).toList.zipWithIndex.map {
case (row, i) => s"${i}0: ${row.mkString(" ")}"
}
// println(i"rawBytes = \n$rawBytes%\n%") // DEBUG
if (pickling ne noPrinter) new TastyPrinter(unit.pickled).printContents()
if (pickling ne noPrinter) new TastyPrinter(pickler.assembleParts()).printContents()
}
}

Expand All @@ -60,7 +61,7 @@ class Pickler extends Phase {
ctx.definitions.init
val unpicklers =
for (unit <- units) yield {
val unpickler = new DottyUnpickler(unit.pickled)
val unpickler = new DottyUnpickler(unit.pickled.assembleParts())
unpickler.enter(roots = Set())
unpickler
}
Expand Down