From 86244aef21915c18aa1de5e8ecd8c073e9c827a8 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sat, 11 Nov 2017 17:15:06 -0800 Subject: Properly merge groups in `groupAroundNamedTargets` to handle cases where a group has multiple terminals --- core/src/main/scala/mill/define/Task.scala | 4 ++-- core/src/main/scala/mill/eval/Evaluator.scala | 8 +++++--- core/src/main/scala/mill/util/MultiBiMap.scala | 1 + core/src/test/scala/mill/EvaluationTests.scala | 20 ++++++++++++++++++++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/mill/define/Task.scala b/core/src/main/scala/mill/define/Task.scala index 9d0972eb..ad6248d9 100644 --- a/core/src/main/scala/mill/define/Task.scala +++ b/core/src/main/scala/mill/define/Task.scala @@ -41,7 +41,7 @@ object Task extends Applicative.Applyer[Task, Task, Args]{ trait Cacher extends mill.define.Cacher[Task, Target]{ def wrapCached[T](t: Task[T], enclosing: String): Target[T] = new TargetImpl(t, enclosing) } - class Target0[T](t: T) extends Task[T]{ + class Task0[T](t: T) extends Task[T]{ lazy val t0 = t val inputs = Nil def evaluate(args: Args) = t0 @@ -116,7 +116,7 @@ object Task extends Applicative.Applyer[Task, Task, Args]{ def mapCtx[A, B](t: Task[A])(f: (A, Args) => B) = t.mapDest(f) - def zip() = new Task.Target0(()) + def zip() = new Task.Task0(()) def zip[A](a: Task[A]) = a.map(Tuple1(_)) def zip[A, B](a: Task[A], b: Task[B]) = a.zip(b) def zip[A, B, C](a: Task[A], b: Task[B], c: Task[C]) = new Task[(A, B, C)]{ diff --git a/core/src/main/scala/mill/eval/Evaluator.scala b/core/src/main/scala/mill/eval/Evaluator.scala index 947a2bca..186a7869 100644 --- a/core/src/main/scala/mill/eval/Evaluator.scala +++ b/core/src/main/scala/mill/eval/Evaluator.scala @@ -147,9 +147,9 @@ object Evaluator{ grouping.lookupValueOpt(upstream) match{ case None if !labeling.contains(upstream) => grouping.add(targetGroup, upstream) - case Some(upstreamGroup) if upstreamGroup == targetGroup => + case Some(upstreamGroup) + if !labeling.contains(upstream) && upstreamGroup != targetGroup => val upstreamTargets = grouping.removeAll(upstreamGroup) - grouping.addAll(targetGroup, upstreamTargets) case _ => //donothing } @@ -171,7 +171,9 @@ object Evaluator{ val targetOrdering = topoSortedTargets.values.items.zipWithIndex.toMap val output = new MultiBiMap.Mutable[Int, Task[_]] for((groupIndices, i) <- groupOrdering.zipWithIndex){ - val sortedGroup = OSet.from(groupIndices.flatMap(grouping.lookupKey).sortBy(targetOrdering)) + val sortedGroup = OSet.from( + groupIndices.flatMap(grouping.lookupKeyOpt).flatten.sortBy(targetOrdering) + ) output.addAll(i, sortedGroup) } output diff --git a/core/src/main/scala/mill/util/MultiBiMap.scala b/core/src/main/scala/mill/util/MultiBiMap.scala index 88bb172c..2052ef90 100644 --- a/core/src/main/scala/mill/util/MultiBiMap.scala +++ b/core/src/main/scala/mill/util/MultiBiMap.scala @@ -25,6 +25,7 @@ object MultiBiMap{ private[this] val keyToValues = mutable.LinkedHashMap.empty[K, OSet.Mutable[V]] def containsValue(v: V) = valueToKey.contains(v) def lookupKey(k: K) = keyToValues(k) + def lookupKeyOpt(k: K) = keyToValues.get(k) def lookupValue(v: V) = valueToKey(v) def lookupValueOpt(v: V) = valueToKey.get(v) def add(k: K, v: V): Unit = { diff --git a/core/src/test/scala/mill/EvaluationTests.scala b/core/src/test/scala/mill/EvaluationTests.scala index ee7b081e..c39c9a50 100644 --- a/core/src/test/scala/mill/EvaluationTests.scala +++ b/core/src/test/scala/mill/EvaluationTests.scala @@ -165,8 +165,28 @@ object EvaluationTests extends TestSuite{ val grouped = Evaluator.groupAroundNamedTargets(topoSorted, labeling) val groupCount = grouped.keyCount assert(groupCount == 1) + } + 'multiTerminalGroup - { + // Make sure the following graph ends up as a single group, since although + // `right` depends on `left`, both of them depend on the un-cached `task` + // which would force them both to re-compute every time `task` changes + // + // _ left + // / + // task -------- right + object taskTriangle extends Cacher{ + val task = T.task{ 1 } + def left = T{ task() } + def right = T{ task() } + } + val labeling = Discovered.mapping(taskTriangle) + val topoSorted = Evaluator.topoSortedTransitiveTargets(OSet(taskTriangle.right, taskTriangle.left)) + val grouped = Evaluator.groupAroundNamedTargets(topoSorted, labeling) + val groupCount = grouped.keyCount + assert(groupCount == 1) } + 'tasksAreUncached - { // Make sure the tasks `left` and `middle` re-compute every time, while // the target `right` does not -- cgit v1.2.3