A few attempts at micro-optimizing the current hot spots

15 files changed, 87 insertions, 54 deletions

diff --git a/core/src/mill/define/Module.scala b/core/src/mill/define/Module.scala
index 93537f76..0f8e96ce 100644
--- a/core/src/mill/define/Module.scala
+++ b/core/src/mill/define/Module.scala
@@ -56,22 +56,25 @@ object Module{
     lazy val millModuleLine = outer.millOuterCtx.lineNum
 
     def reflect[T: ClassTag] = {
-      outer
-        .getClass
-        .getMethods
-        .filter(!_.getName.contains('$'))
-        .filter(_.getParameterCount == 0)
-        .filter(x => (x.getModifiers & Modifier.STATIC) == 0)
-        .filter(implicitly[ClassTag[T]].runtimeClass isAssignableFrom _.getReturnType)
-        .map(_.invoke(outer).asInstanceOf[T])
+      val runtimeCls = implicitly[ClassTag[T]].runtimeClass
+      for{
+        m <- outer.getClass.getMethods
+        if
+          !m.getName.contains('$') &&
+          m.getParameterCount == 0 &&
+          (m.getModifiers & Modifier.STATIC) == 0 &&
+          runtimeCls.isAssignableFrom(m.getReturnType)
+      } yield m.invoke(outer).asInstanceOf[T]
+
     }
     def reflectNames[T: ClassTag] = {
-      outer
-        .getClass
-        .getMethods
-        .filter(x => (x.getModifiers & Modifier.STATIC) == 0)
-        .filter(implicitly[ClassTag[T]].runtimeClass isAssignableFrom _.getReturnType)
-        .map(_.getName)
+      val runtimeCls = implicitly[ClassTag[T]].runtimeClass
+      for{
+        m <- outer.getClass.getMethods
+        if
+          (m.getModifiers & Modifier.STATIC) == 0 &&
+          runtimeCls.isAssignableFrom(m.getReturnType)
+      } yield m.getName
     }
     // For some reason, this fails to pick up concrete `object`s nested directly within
     // another top-level concrete `object`. This is fine for now, since Mill's Ammonite
diff --git a/core/src/mill/eval/Evaluator.scala b/core/src/mill/eval/Evaluator.scala
index 57d39a24..42015c6f 100644
--- a/core/src/mill/eval/Evaluator.scala
+++ b/core/src/mill/eval/Evaluator.scala
@@ -32,7 +32,7 @@ case class Evaluator[T](outPath: Path,
                         log: Logger,
                         classLoaderSig: Seq[(Path, Long)] = Evaluator.classLoaderSig,
                         workerCache: mutable.Map[Segments, (Int, Any)] = mutable.Map.empty){
-
+  val classLoaderSignHash = classLoaderSig.hashCode()
   def evaluate(goals: Agg[Task[_]]): Evaluator.Results = {
     mkdir(outPath)
 
@@ -82,7 +82,12 @@ case class Evaluator[T](outPath: Path,
     for (((terminal, group), i) <- sortedGroups.items().zipWithIndex){
       // Increment the counter message by 1 to go from 1/10 to 10/10 instead of 0/10 to 9/10
       val counterMsg = (i+1) + "/" + sortedGroups.keyCount
-      val (newResults, newEvaluated) = evaluateGroupCached(terminal, group, results, counterMsg)
+      val (newResults, newEvaluated) = evaluateGroupCached(
+        terminal,
+        group,
+        results,
+        counterMsg)
+
       for(ev <- newEvaluated){
         evaluated.append(ev)
       }
@@ -114,13 +119,16 @@ case class Evaluator[T](outPath: Path,
                           results: collection.Map[Task[_], Result[(Any, Int)]],
                           counterMsg: String): (collection.Map[Task[_], Result[(Any, Int)]], Seq[Task[_]]) = {
 
+    val externalInputsHash = scala.util.hashing.MurmurHash3.orderedHash(
+      group.items.flatMap(_.inputs).filter(!group.contains(_))
+        .flatMap(results(_).asSuccess.map(_.value._2))
+    )
 
-    val externalInputs = group.items.flatMap(_.inputs).filter(!group.contains(_)).toVector
+    val sideHashes = scala.util.hashing.MurmurHash3.orderedHash(
+      group.toIterator.map(_.sideHash)
+    )
 
-    val inputsHash =
-      externalInputs.map(results(_).map(_._2)).hashCode +
-      group.toIterator.map(_.sideHash).toVector.hashCode() +
-      classLoaderSig.hashCode()
+    val inputsHash = externalInputsHash + sideHashes + classLoaderSignHash
 
     terminal match{
       case Left(task) =>
diff --git a/core/src/mill/eval/Result.scala b/core/src/mill/eval/Result.scala
index 690c5d30..d0400599 100644
--- a/core/src/mill/eval/Result.scala
+++ b/core/src/mill/eval/Result.scala
@@ -2,14 +2,16 @@ package mill.eval
 
 sealed trait Result[+T]{
   def map[V](f: T => V): Result[V]
+  def asSuccess: Option[Result.Success[T]] = None
 }
 object Result{
   implicit def create[T](t: => T): Result[T] = {
     try Success(t)
     catch { case e: Throwable => Exception(e, new OuterStack(new java.lang.Exception().getStackTrace)) }
   }
-  case class Success[T](value: T) extends Result[T]{
+  case class Success[+T](value: T) extends Result[T]{
     def map[V](f: T => V) = Result.Success(f(value))
+    override def asSuccess = Some(this)
   }
   case object Skipped extends Result[Nothing]{
     def map[V](f: Nothing => V) = this
diff --git a/core/src/mill/util/Logger.scala b/core/src/mill/util/Logger.scala
index 55ea84cc..714b9add 100644
--- a/core/src/mill/util/Logger.scala
+++ b/core/src/mill/util/Logger.scala
@@ -136,7 +136,7 @@ case class FileLogger(colored: Boolean, file: Path) extends Logger {
   def info(s: String) = outputStream.println(s)
   def error(s: String) = outputStream.println(s)
   def ticker(s: String) = outputStream.println(s)
-  val inStream: InputStream = new ByteArrayInputStream(Array())
+  val inStream: InputStream = DummyInputStream
   override def close() = {
     if (outputStreamUsed)
       outputStream.close()
diff --git a/docs/pages/4 - Modules.md b/docs/pages/4 - Modules.md
index c4dd3f8b..c8d7378c 100644
--- a/docs/pages/4 - Modules.md
+++ b/docs/pages/4 - Modules.md
@@ -137,7 +137,7 @@ object Bar extends mill.define.ExternalModule {
   def baz = T{ 1 }
   def qux() = T.command{ println(baz() + 1) }
 
-  def millDiscover = mill.define.Discover[this.type]
+  lazy val millDiscover = mill.define.Discover[this.type]
 }
 ```
 
diff --git a/main/src/mill/main/MainRunner.scala b/main/src/mill/main/MainRunner.scala
index a4a54df8..4205b6dc 100644
--- a/main/src/mill/main/MainRunner.scala
+++ b/main/src/mill/main/MainRunner.scala
@@ -1,13 +1,15 @@
 package mill.main
 import java.io.{InputStream, OutputStream, PrintStream}
 
+import ammonite.Main
 import ammonite.interp.{Interpreter, Preprocessor}
 import ammonite.ops.Path
 import ammonite.util._
 import mill.eval.{Evaluator, PathRef}
-
 import mill.util.PrintLogger
 
+import scala.annotation.tailrec
+
 
 /**
   * Customized version of [[ammonite.MainRunner]], allowing us to run Mill
@@ -35,8 +37,27 @@ class MainRunner(val config: ammonite.main.Cli.Config,
     while(statAll()) Thread.sleep(100)
   }
 
+  /**
+    * Custom version of [[watchLoop]] that lets us generate the watched-file
+    * signature only on demand, so if we don't have config.watch enabled we do
+    * not pay the cost of generating it
+    */
+  @tailrec final def watchLoop2[T](isRepl: Boolean,
+                                   printing: Boolean,
+                                   run: Main => (Res[T], () => Seq[(Path, Long)])): Boolean = {
+    val (result, watched) = run(initMain(isRepl))
+
+    val success = handleWatchRes(result, printing)
+    if (!config.watch) success
+    else{
+      watchAndWait(watched())
+      watchLoop2(isRepl, printing, run)
+    }
+  }
+
+
   override def runScript(scriptPath: Path, scriptArgs: List[String]) =
-    watchLoop(
+    watchLoop2(
       isRepl = false,
       printing = true,
       mainCfg => {
@@ -58,12 +79,22 @@ class MainRunner(val config: ammonite.main.Cli.Config,
 
         result match{
           case Res.Success(data) =>
-            val (eval, evaluationWatches, res) = data
+            val (eval, evalWatches, res) = data
 
             stateCache = Some(Evaluator.State(eval.rootModule, eval.classLoaderSig, eval.workerCache, interpWatched))
-
-            (Res(res), interpWatched ++ evaluationWatches)
-          case _ => (result, interpWatched)
+            val watched = () => {
+              val alreadyStale = evalWatches.exists(p => p.sig != PathRef(p.path, p.quick).sig)
+              // If the file changed between the creation of the original
+              // `PathRef` and the current moment, use random junk .sig values
+              // to force an immediate re-run. Otherwise calculate the
+              // pathSignatures the same way Ammonite would and hand over the
+              // values, so Ammonite can watch them and only re-run if they
+              // subsequently change
+              if (alreadyStale) evalWatches.map(_.path -> util.Random.nextLong())
+              else evalWatches.map(p => p.path -> Interpreter.pathSignature(p.path))
+            }
+            (Res(res), () => interpWatched ++ watched())
+          case _ => (result, () => interpWatched)
         }
       }
     )
@@ -104,7 +135,7 @@ class MainRunner(val config: ammonite.main.Cli.Config,
          |  // doesn't get picked up during reflective child-module discovery
          |  def millSelf = Some(this)
          |
-         |  implicit def millDiscover: mill.define.Discover[this.type] = mill.define.Discover[this.type]
+         |  implicit lazy val millDiscover: mill.define.Discover[this.type] = mill.define.Discover[this.type]
          |}
          |
          |sealed trait $wrapName extends mill.main.MainModule{
diff --git a/main/src/mill/main/RunScript.scala b/main/src/mill/main/RunScript.scala
index 85930e9b..553f5b69 100644
--- a/main/src/mill/main/RunScript.scala
+++ b/main/src/mill/main/RunScript.scala
@@ -29,7 +29,7 @@ object RunScript{
                 scriptArgs: Seq[String],
                 stateCache: Option[Evaluator.State],
                 log: Logger)
-  : (Res[(Evaluator[Any], Seq[(Path, Long)], Either[String, Seq[Js.Value]])], Seq[(Path, Long)]) = {
+  : (Res[(Evaluator[Any], Seq[PathRef], Either[String, Seq[Js.Value]])], Seq[(Path, Long)]) = {
 
     val (evalState, interpWatched) = stateCache match{
       case Some(s) if watchedSigUnchanged(s.watched) => Res.Success(s) -> s.watched
@@ -58,18 +58,7 @@ object RunScript{
       evaluator <- evalRes
       (evalWatches, res) <- Res(evaluateTasks(evaluator, scriptArgs, multiSelect = false))
     } yield {
-      val alreadyStale = evalWatches.exists(p => p.sig != PathRef(p.path, p.quick).sig)
-      // If the file changed between the creation of the original
-      // `PathRef` and the current moment, use random junk .sig values
-      // to force an immediate re-run. Otherwise calculate the
-      // pathSignatures the same way Ammonite would and hand over the
-      // values, so Ammonite can watch them and only re-run if they
-      // subsequently change
-      val evaluationWatches =
-        if (alreadyStale) evalWatches.map(_.path -> util.Random.nextLong())
-        else evalWatches.map(p => p.path -> Interpreter.pathSignature(p.path))
-
-      (evaluator, evaluationWatches, res.map(_.flatMap(_._2)))
+      (evaluator, evalWatches, res.map(_.flatMap(_._2)))
     }
     (evaluated, interpWatched)
   }
diff --git a/main/test/src/mill/eval/ModuleTests.scala b/main/test/src/mill/eval/ModuleTests.scala
index d2fa7184..f089a251 100644
--- a/main/test/src/mill/eval/ModuleTests.scala
+++ b/main/test/src/mill/eval/ModuleTests.scala
@@ -13,7 +13,7 @@ object ModuleTests extends TestSuite{
     object inner extends mill.Module{
       def y = T{17}
     }
-    def millDiscover = Discover[this.type]
+    lazy val millDiscover = Discover[this.type]
   }
   object Build extends TestUtil.BaseModule{
     def z = T{ ExternalModule.x() + ExternalModule.inner.y() }
@@ -36,7 +36,7 @@ object ModuleTests extends TestSuite{
       object Build extends mill.define.ExternalModule {
 
         def z = T{ ExternalModule.x() + ExternalModule.inner.y() }
-        def millDiscover = Discover[this.type]
+        lazy val millDiscover = Discover[this.type]
       }
 
       intercept[java.lang.AssertionError]{ Build }
diff --git a/main/test/src/mill/util/TestGraphs.scala b/main/test/src/mill/util/TestGraphs.scala
index 750ef015..11f72d02 100644
--- a/main/test/src/mill/util/TestGraphs.scala
+++ b/main/test/src/mill/util/TestGraphs.scala
@@ -180,7 +180,7 @@ object TestGraphs{
   object canOverrideSuper extends TestUtil.BaseModule with BaseModule {
     override def foo = T{ super.foo() ++ Seq("object") }
     override def cmd(i: Int) = T.command{ super.cmd(i)() ++ Seq("object" + i) }
-    def millDiscover: Discover[this.type] = Discover[this.type]
+    override lazy val millDiscover: Discover[this.type] = Discover[this.type]
   }
 
   trait TraitWithModule extends Module{ outer =>
@@ -193,7 +193,7 @@ object TestGraphs{
 
   // Make sure nested objects inherited from traits work
   object TraitWithModuleObject extends TestUtil.BaseModule with TraitWithModule{
-    def millDiscover: Discover[this.type] = Discover[this.type]
+    override lazy val millDiscover: Discover[this.type] = Discover[this.type]
   }
 
 
diff --git a/main/test/src/mill/util/TestUtil.scala b/main/test/src/mill/util/TestUtil.scala
index b30d5d51..d06c7f6a 100644
--- a/main/test/src/mill/util/TestUtil.scala
+++ b/main/test/src/mill/util/TestUtil.scala
@@ -32,7 +32,7 @@ object TestUtil {
                    millName0: sourcecode.Name,
                    overrides: Overrides)
     extends mill.define.BaseModule(getSrcPathBase() / millModuleEnclosing0.value.split("\\.| |#")){
-    def millDiscover: Discover[this.type] = Discover[this.type]
+    lazy val millDiscover: Discover[this.type] = Discover[this.type]
   }
 
   object test{
diff --git a/scalajslib/src/mill/scalajslib/ScalaJSBridge.scala b/scalajslib/src/mill/scalajslib/ScalaJSBridge.scala
index 7c9834b0..96b48704 100644
--- a/scalajslib/src/mill/scalajslib/ScalaJSBridge.scala
+++ b/scalajslib/src/mill/scalajslib/ScalaJSBridge.scala
@@ -79,5 +79,5 @@ trait ScalaJSBridge {
 object ScalaJSBridge extends mill.define.ExternalModule {
 
   def scalaJSBridge = T.worker { new ScalaJSWorker() }
-  def millDiscover = Discover[this.type]
+  lazy val millDiscover = Discover[this.type]
 }
diff --git a/scalajslib/test/src/mill/scalajslib/HelloJSWorldTests.scala b/scalajslib/test/src/mill/scalajslib/HelloJSWorldTests.scala
index abb0d5f8..189e9634 100644
--- a/scalajslib/test/src/mill/scalajslib/HelloJSWorldTests.scala
+++ b/scalajslib/test/src/mill/scalajslib/HelloJSWorldTests.scala
@@ -75,7 +75,7 @@ object HelloJSWorldTests extends TestSuite {
         )
       }
     }
-    override def millDiscover = Discover[this.type]
+    override lazy val millDiscover = Discover[this.type]
   }
 
   val millSourcePath = pwd / 'scalajslib / 'test / 'resources / "hello-js-world"
diff --git a/scalalib/src/mill/scalalib/GenIdea.scala b/scalalib/src/mill/scalalib/GenIdea.scala
index 6cedf606..6304fb14 100644
--- a/scalalib/src/mill/scalalib/GenIdea.scala
+++ b/scalalib/src/mill/scalalib/GenIdea.scala
@@ -23,7 +23,7 @@ object GenIdeaModule extends ExternalModule {
   }
 
   implicit def millScoptEvaluatorReads[T] = new mill.main.EvaluatorScopt[T]()
-  def millDiscover = Discover[this.type]
+  lazy val millDiscover = Discover[this.type]
 }
 object GenIdea {
 
diff --git a/scalalib/src/mill/scalalib/PublishModule.scala b/scalalib/src/mill/scalalib/PublishModule.scala
index 87c8ee32..3cc9fd30 100644
--- a/scalalib/src/mill/scalalib/PublishModule.scala
+++ b/scalalib/src/mill/scalalib/PublishModule.scala
@@ -119,5 +119,5 @@ object PublishModule extends ExternalModule{
 
   implicit def millScoptTargetReads[T] = new mill.main.Tasks.Scopt[T]()
 
-  def millDiscover: mill.define.Discover[this.type] = mill.define.Discover[this.type]
+  lazy val millDiscover: mill.define.Discover[this.type] = mill.define.Discover[this.type]
 }
\ No newline at end of file
diff --git a/scalalib/src/mill/scalalib/ScalaWorkerApi.scala b/scalalib/src/mill/scalalib/ScalaWorkerApi.scala
index 964179a9..7e11f61d 100644
--- a/scalalib/src/mill/scalalib/ScalaWorkerApi.scala
+++ b/scalalib/src/mill/scalalib/ScalaWorkerApi.scala
@@ -42,7 +42,7 @@ object ScalaWorkerApi extends mill.define.ExternalModule {
       Seq(ivy"org.scala-sbt:compiler-interface:1.1.0")
     )
   }
-  def millDiscover = Discover[this.type]
+  lazy val millDiscover = Discover[this.type]
 }
 
 trait ScalaWorkerApi {