Tweak `OSet` to avoid re-creating the `.items` Vector for every operation, making `.items` a lightweight Iterator and forcing users to use `.indexed` if they explicitly need an indexed sequence

6 files changed, 17 insertions, 14 deletions

diff --git a/core/src/main/scala/mill/eval/Evaluator.scala b/core/src/main/scala/mill/eval/Evaluator.scala
index d76f5f4b..3d9fbab0 100644
--- a/core/src/main/scala/mill/eval/Evaluator.scala
+++ b/core/src/main/scala/mill/eval/Evaluator.scala
@@ -38,9 +38,9 @@ class Evaluator(workspacePath: Path,
 
     val failing = new util.MultiBiMap.Mutable[Either[Task[_], LabelledTarget[_]], Result.Failing]
     for((k, vs) <- sortedGroups.items){
-      failing.addAll(k, vs.items.flatMap(results.get(_)).collect{case f: Result.Failing => f})
+      failing.addAll(k, vs.items.flatMap(results.get).collect{case f: Result.Failing => f})
     }
-    Evaluator.Results(goals.items.map(results), evaluated, transitive, failing)
+    Evaluator.Results(goals.indexed.map(results), evaluated, transitive, failing)
   }
 
   def resolveDestPaths(t: LabelledTarget[_]): (Path, Path) = {
@@ -57,7 +57,7 @@ class Evaluator(workspacePath: Path,
     val externalInputs = group.items.flatMap(_.inputs).filter(!group.contains(_))
 
     val inputsHash =
-      externalInputs.toIterator.map(results).toVector.hashCode +
+      externalInputs.map(results).toVector.hashCode +
       group.toIterator.map(_.sideHash).toVector.hashCode()
 
     terminal match{
@@ -179,7 +179,8 @@ object Evaluator{
     */
   def topoSorted(transitiveTargets: OSet[Task[_]]): TopoSorted = {
 
-    val targetIndices = transitiveTargets.items.zipWithIndex.toMap
+    val indexed = transitiveTargets.indexed
+    val targetIndices = indexed.zipWithIndex.toMap
 
     val numberedEdges =
       for(t <- transitiveTargets.items)
@@ -188,6 +189,6 @@ object Evaluator{
     val sortedClusters = Tarjans(numberedEdges)
     val nonTrivialClusters = sortedClusters.filter(_.length > 1)
     assert(nonTrivialClusters.isEmpty, nonTrivialClusters)
-    new TopoSorted(OSet.from(sortedClusters.flatten.map(transitiveTargets.items)))
+    new TopoSorted(OSet.from(sortedClusters.flatten.map(indexed)))
   }
 }
\ No newline at end of file
diff --git a/core/src/main/scala/mill/eval/Tarjans.scala b/core/src/main/scala/mill/eval/Tarjans.scala
index db2536b6..ade335a9 100644
--- a/core/src/main/scala/mill/eval/Tarjans.scala
+++ b/core/src/main/scala/mill/eval/Tarjans.scala
@@ -5,7 +5,7 @@ import scala.collection.mutable
 // Adapted from
 // https://github.com/indy256/codelibrary/blob/c52247216258e84aac442a23273b7d8306ef757b/java/src/SCCTarjan.java
 object Tarjans {
-  def apply(graph0: Seq[Seq[Int]]): Seq[Seq[Int]] = {
+  def apply(graph0: TraversableOnce[TraversableOnce[Int]]): Seq[Seq[Int]] = {
     val graph = graph0.map(_.toArray).toArray
     val n = graph.length
     val visited = new Array[Boolean](n)
diff --git a/core/src/main/scala/mill/util/OSet.scala b/core/src/main/scala/mill/util/OSet.scala
index 82f5299b..ce5d573b 100644
--- a/core/src/main/scala/mill/util/OSet.scala
+++ b/core/src/main/scala/mill/util/OSet.scala
@@ -11,7 +11,8 @@ import scala.collection.mutable
   */
 trait OSet[V] extends TraversableOnce[V]{
   def contains(v: V): Boolean
-  def items: IndexedSeq[V]
+  def items: Iterator[V]
+  def indexed: IndexedSeq[V]
   def flatMap[T](f: V => TraversableOnce[T]): OSet[T]
   def map[T](f: V => T): OSet[T]
   def filter(f: V => Boolean): OSet[V]
@@ -46,7 +47,8 @@ object OSet{
       throw new Exception("Duplicated item inserted into OrderedSet: " + v)
     }
     def appendAll(vs: Seq[V]) = vs.foreach(append)
-    def items: IndexedSeq[V] = set0.toIndexedSeq
+    def items = set0.iterator
+    def indexed: IndexedSeq[V] = items.toIndexedSeq
     def set: collection.Set[V] = set0
 
     def map[T](f: V => T): OSet[T] = {
@@ -75,7 +77,7 @@ object OSet{
       }
     }
 
-    def reverse = OSet.from(items.reverseIterator)
+    def reverse = OSet.from(indexed.reverseIterator)
 
     // Members declared in scala.collection.GenTraversableOnce
     def isTraversableAgain: Boolean = items.isTraversableAgain
@@ -91,11 +93,11 @@ object OSet{
     def hasDefiniteSize: Boolean = items.hasDefiniteSize
     def isEmpty: Boolean = items.isEmpty
     def seq: scala.collection.TraversableOnce[V] = items
-    def toTraversable: Traversable[V] = items
+    def toTraversable: Traversable[V] = items.toTraversable
 
     override def hashCode() = items.hashCode()
     override def equals(other: Any) = other match{
-      case s: OSet[_] => items.equals(s.items)
+      case s: OSet[_] => items.sameElements(s.items)
       case _ => super.equals(other)
     }
     override def toString = items.mkString("OSet(", ", ", ")")
diff --git a/core/src/test/scala/mill/EvaluationTests.scala b/core/src/test/scala/mill/EvaluationTests.scala
index 03118a29..9c856152 100644
--- a/core/src/test/scala/mill/EvaluationTests.scala
+++ b/core/src/test/scala/mill/EvaluationTests.scala
@@ -28,7 +28,7 @@ object EvaluationTests extends TestSuite{
 
       val evaled = evaluator.evaluate(OSet(target))
 
-      val (matchingReturnedEvaled, extra) = evaled.evaluated.items.partition(expEvaled.contains)
+      val (matchingReturnedEvaled, extra) = evaled.evaluated.indexed.partition(expEvaled.contains)
 
       assert(
         evaled.values == Seq(expValue),
diff --git a/core/src/test/scala/mill/JavaCompileJarTests.scala b/core/src/test/scala/mill/JavaCompileJarTests.scala
index 9390d98e..80df57b6 100644
--- a/core/src/test/scala/mill/JavaCompileJarTests.scala
+++ b/core/src/test/scala/mill/JavaCompileJarTests.scala
@@ -62,7 +62,7 @@ object JavaCompileJarTests extends TestSuite{
             }.size
           ))
         }else{
-          Left(evaluated.failing.lookupKey(evaluated.failing.keys().next).items.head)
+          Left(evaluated.failing.lookupKey(evaluated.failing.keys().next).items.next())
         }
 
       }
diff --git a/core/src/test/scala/mill/TestUtil.scala b/core/src/test/scala/mill/TestUtil.scala
index dfe0f3a0..006198a4 100644
--- a/core/src/test/scala/mill/TestUtil.scala
+++ b/core/src/test/scala/mill/TestUtil.scala
@@ -27,7 +27,7 @@ object TestUtil {
   }
   def checkTopological(targets: OSet[Task[_]]) = {
     val seen = mutable.Set.empty[Task[_]]
-    for(t <- targets.items.reverseIterator){
+    for(t <- targets.indexed.reverseIterator){
       seen.add(t)
       for(upstream <- t.inputs){
         assert(!seen(upstream))