Introduce --dependency-classpath for Mjar (see twitter#240)

This lays the groundwork for fixing twitter#239, but unfortunately eager loading of dependency classpaths is insanely slow (benchMjar goes up from 310ms to 1548ms which makes this solution unacceptable).
xeno-by · Oct 9, 2018 · 1834b78 · 1834b78
1 parent a6b9fbc
commit 1834b78
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 37 deletions.
diff --git a/bench/src/main/scala/rsc/bench/RscMjar.scala b/bench/src/main/scala/rsc/bench/RscMjar.scala
@@ -21,7 +21,10 @@ class RscMjar extends RscBenchmark {
     runCompiler("-cp", bs.rscDeps, "-out", rscOut, bs.files)
 
     val mjarOut = Files.createTempFile("mjar", ".jar")
-    val settings = Settings().withClasspath(List(rscOut)).withOut(mjarOut)
+    val settings = Settings()
+      .withDependencyClasspath(bs.rscClasspath)
+      .withClasspath(List(rscOut))
+      .withOut(mjarOut)
     val reporter = Reporter().withOut(System.out).withErr(System.err)
     Mjar.process(settings, reporter) match {
       case Some(_) => ()

diff --git a/check/src/main/scala/rsc/checkbase/ToolUtil.scala b/check/src/main/scala/rsc/checkbase/ToolUtil.scala
@@ -42,14 +42,19 @@ trait ToolUtil extends CacheUtil with NscUtil {
     }
   }
 
-  def mjar(classpath: List[Path]): ToolResult[Path] = {
-    withConsole { console =>
-      import scala.meta.mjar._
-      val out = Files.createTempFile("out", ".jar")
-      val settings = Settings().withClasspath(classpath).withOut(out)
-      Mjar.process(settings, console.reporter) match {
-        case Some(out) => Right(out)
-        case None => Left(List(console.err))
+  def mjar(dependencyClasspath: List[Path], semanticdb: Path): ToolResult[Path] = {
+    metacp(Nil, dependencyClasspath).right.flatMap { dependencyClasspath =>
+      withConsole { console =>
+        import scala.meta.mjar._
+        val out = Files.createTempFile("out", ".jar")
+        val settings = Settings()
+          .withDependencyClasspath(dependencyClasspath)
+          .withClasspath(List(semanticdb))
+          .withOut(out)
+        Mjar.process(settings, console.reporter) match {
+          case Some(out) => Right(out)
+          case None => Left(List(console.err))
+        }
       }
     }
   }

diff --git a/check/src/main/scala/rsc/checkclasses/Main.scala b/check/src/main/scala/rsc/checkclasses/Main.scala
@@ -16,8 +16,8 @@ object Main extends SimpleBase[Settings, Path, Path] {
   }
 
   def rscResult(settings: Settings) = {
-    val depsSemanticdbs = rsc(settings.cp, settings.deps)
-    val depsMjar = depsSemanticdbs.right.flatMap(depsSemanticdbs => mjar(List(depsSemanticdbs)))
+    val depsSemanticdb = rsc(settings.cp, settings.deps)
+    val depsMjar = depsSemanticdb.right.flatMap(depsSemanticdb => mjar(settings.cp, depsSemanticdb))
     depsMjar.right.flatMap(depsMjar => nsc(settings.cp :+ depsMjar, settings.ins))
   }
 

diff --git a/check/src/main/scala/rsc/checkmjar/Main.scala b/check/src/main/scala/rsc/checkmjar/Main.scala
@@ -16,7 +16,7 @@ object Main extends SimpleBase[Settings, Path, Path] {
 
   def rscResult(settings: Settings) = {
     val semanticdbResult = rsc(settings.cp, settings.ins)
-    semanticdbResult.right.flatMap(path => mjar(List(path)))
+    semanticdbResult.right.flatMap(semanticdbResult => mjar(settings.cp, semanticdbResult))
   }
 
   def checker(settings: Settings, nscResult: Path, rscResult: Path) = {

diff --git a/mjar/mjar/src/main/scala/scala/meta/internal/mjar/Main.scala b/mjar/mjar/src/main/scala/scala/meta/internal/mjar/Main.scala
@@ -24,7 +24,7 @@ class Main(settings: Settings, reporter: Reporter) {
       try {
         val symtab = Symtab(settings)
         val done = mutable.HashSet[String]()
-        symtab.toplevels.foreach { sym =>
+        symtab.todo.foreach { sym =>
           if (!done(sym)) {
             try {
               val companionSym = {

diff --git a/mjar/mjar/src/main/scala/scala/meta/internal/mjar/Symtab.scala b/mjar/mjar/src/main/scala/scala/meta/internal/mjar/Symtab.scala
@@ -3,15 +3,15 @@
 package scala.meta.internal.mjar
 
 import scala.collection.mutable
-import scala.collection.mutable.LinkedHashMap
 import scala.meta.internal.semanticdb._
 import scala.meta.internal.semanticdb.Scala._
 import scala.meta.internal.semanticdb.SymbolOccurrence.{Role => r}
 import scala.meta.mjar._
 
 class Symtab private (
-    infos: LinkedHashMap[String, SymbolInformation],
-    anchors: LinkedHashMap[String, String]) {
+    infos: mutable.Map[String, SymbolInformation],
+    anchors: mutable.Map[String, String],
+    val todo: mutable.UnrolledBuffer[String]) {
   def apply(sym: String): SymbolInformation = {
     infos(sym)
   }
@@ -32,28 +32,19 @@ class Symtab private (
     infos(sym) = info
   }
 
-  lazy val toplevels: List[String] = {
-    infos.keys.filter { sym =>
-      val info = apply(sym)
-      if (info.isPackageObject || info.isClass || info.isInterface ||
-          info.isObject || info.isTrait) {
-        info.symbol.owner.desc.isPackage
-      } else {
-        false
-      }
-    }.toList
-  }
-
   def anchor(sym: String): Option[String] = {
     anchors.get(sym)
   }
 }
 
 object Symtab {
   def apply(settings: Settings): Symtab = {
-    val infos = LinkedHashMap[String, SymbolInformation]()
-    val anchors = LinkedHashMap[String, String]()
-    settings.classpath.foreach { path =>
+    val unifiedClasspath = (settings.dependencyClasspath ++ settings.classpath).distinct
+    val infos = mutable.Map[String, SymbolInformation]()
+    val anchors = mutable.Map[String, String]()
+    val todo = mutable.UnrolledBuffer[String]()
+    unifiedClasspath.foreach { path =>
+      val inTodo = settings.classpath.contains(path)
       Locator(path) { (_, payload) =>
         payload.documents.foreach { document =>
           val ranges = mutable.Map[String, Range]()
@@ -66,20 +57,24 @@ object Symtab {
             }
           }
           document.symbols.foreach { info =>
-            if (info.symbol.isGlobal) {
-              infos(info.symbol) = info
+            val sym = info.symbol
+            if (sym.isGlobal) {
+              infos(sym) = info
               if (settings.debug) {
                 var anchor = document.uri
-                ranges.get(info.symbol).foreach { range =>
+                ranges.get(sym).foreach { range =>
                   anchor += s":${range.startLine + 1}"
                 }
-                anchors(info.symbol) = anchor
+                anchors(sym) = anchor
+              }
+              if (inTodo && sym.owner.desc.isPackage && !sym.desc.isPackage) {
+                todo += sym
               }
             }
           }
         }
       }
     }
-    new Symtab(infos, anchors)
+    new Symtab(infos, anchors, todo)
   }
 }
diff --git a/mjar/mjar/src/main/scala/scala/meta/mjar/Settings.scala b/mjar/mjar/src/main/scala/scala/meta/mjar/Settings.scala
@@ -10,12 +10,14 @@ final class Settings private (
     val abi: Abi,
     val classpath: List[Path],
     val debug: Boolean,
+    val dependencyClasspath: List[Path],
     val out: Path) {
   private def this() = {
     this(
       abi = Scalac211,
       classpath = Nil,
       debug = false,
+      dependencyClasspath = Nil,
       out = Paths.get("out.jar")
     )
   }
@@ -32,6 +34,10 @@ final class Settings private (
     copy(debug = debug)
   }
 
+  def withDependencyClasspath(dependencyClasspath: List[Path]): Settings = {
+    copy(dependencyClasspath = dependencyClasspath)
+  }
+
   def withOut(out: Path): Settings = {
     copy(out = out)
   }
@@ -40,9 +46,15 @@ final class Settings private (
       abi: Abi = abi,
       classpath: List[Path] = classpath,
       debug: Boolean = debug,
+      dependencyClasspath: List[Path] = dependencyClasspath,
       out: Path = out
   ): Settings = {
-    new Settings(abi = abi, classpath = classpath, debug = debug, out = out)
+    new Settings(
+      abi = abi,
+      classpath = classpath,
+      debug = debug,
+      dependencyClasspath = dependencyClasspath,
+      out = out)
   }
 }
 
@@ -62,6 +74,9 @@ object Settings {
           None
         case "--debug" +: rest if allowOptions =>
           loop(settings.copy(debug = true), true, rest)
+        case "--dependency-classpath" +: s_dcp +: rest =>
+          val dcp = s_dcp.split(pathSeparator).map(p => Paths.get(p)).toList
+          loop(settings.copy(dependencyClasspath = settings.dependencyClasspath ++ dcp), true, rest)
         case "--release" +: rest if allowOptions =>
           loop(settings.copy(debug = false), true, rest)
         case "-out" +: s_out +: rest if allowOptions =>