From 564aa7262bd078fda32257a127c3e1c192f393c7 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 14 May 2019 16:06:49 -0700 Subject: [PATCH 1/2] Fix TaskProgress Supershell was not reliably working and I tracked it down to TaskProgress not actually publishing updates during task execution. This seemed to happen because the background task was only run once when the task started up. Once that task exited, no further task reports would be published. The fix is to start a new thread every time we enter EvaluateTask. I verified manually that it did not seem to leak threads because EvaluateTask always calls shutdown, which calls afterAllCompleted, which stops the progress thread. I also decreased the default report period to 100ms. I can't imagine that this will have a big effect on performance. It can be tuned with the sbt.supershell.sleep parameter. --- main/src/main/scala/sbt/EvaluateTask.scala | 1 + .../scala/sbt/internal/TaskProgress.scala | 77 ++++++++++--------- 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/main/src/main/scala/sbt/EvaluateTask.scala b/main/src/main/scala/sbt/EvaluateTask.scala index bed47c27f..e08838bd5 100644 --- a/main/src/main/scala/sbt/EvaluateTask.scala +++ b/main/src/main/scala/sbt/EvaluateTask.scala @@ -467,6 +467,7 @@ object EvaluateTask { // Register with our cancel handler we're about to start. val strat = config.cancelStrategy val cancelState = strat.onTaskEngineStart(runningEngine) + config.progressReporter.initial() try { (state.get(stateBuildStructure), state.get(sessionSettings)) match { case (Some(structure), Some(settings)) => diff --git a/main/src/main/scala/sbt/internal/TaskProgress.scala b/main/src/main/scala/sbt/internal/TaskProgress.scala index 0a3f7bee2..3fb598265 100644 --- a/main/src/main/scala/sbt/internal/TaskProgress.scala +++ b/main/src/main/scala/sbt/internal/TaskProgress.scala @@ -8,18 +8,13 @@ package sbt package internal -import sbt.internal.util.{ - RMap, - ConsoleAppender, - LogOption, - JLine, - ManagedLogger, - ProgressEvent, - ProgressItem -} +import java.util.concurrent.atomic.{ AtomicBoolean, AtomicInteger, AtomicReference } + +import sbt.internal.util._ import sbt.util.Level -import scala.concurrent.{ blocking, Future, ExecutionContext } -import java.util.concurrent.atomic.{ AtomicBoolean, AtomicInteger } + +import scala.annotation.tailrec +import scala.util.control.NonFatal /** * implements task progress display on the shell. @@ -27,46 +22,56 @@ import java.util.concurrent.atomic.{ AtomicBoolean, AtomicInteger } private[sbt] final class TaskProgress(log: ManagedLogger) extends AbstractTaskExecuteProgress with ExecuteProgress[Task] { - private[this] val isReady = new AtomicBoolean(false) private[this] val lastTaskCount = new AtomicInteger(0) - private[this] val isAllCompleted = new AtomicBoolean(false) - private[this] val isStopped = new AtomicBoolean(false) + private[this] val currentProgressThread = new AtomicReference[Option[ProgressThread]](None) + private[this] val sleepDuration = + try System.getProperty("sbt.supershell.sleep", "100").toLong + catch { case NonFatal(_) => 100L } + private[this] final class ProgressThread + extends Thread("task-progress-report-thread") + with AutoCloseable { + private[this] val isClosed = new AtomicBoolean(false) + setDaemon(true) + start() + @tailrec override def run(): Unit = { + if (!isClosed.get()) { + try { + report() + Thread.sleep(sleepDuration) + } catch { + case _: InterruptedException => + } + run() + } + } + + override def close(): Unit = { + isClosed.set(true) + interrupt() + } + } override def initial(): Unit = { + currentProgressThread.get() match { + case None => + currentProgressThread.set(Some(new ProgressThread)) + case _ => + } ConsoleAppender.setTerminalWidth(JLine.terminal.getWidth) } - override def afterReady(task: Task[_]): Unit = { - isReady.set(true) - } + override def afterReady(task: Task[_]): Unit = () override def afterCompleted[A](task: Task[A], result: Result[A]): Unit = () - override def stop(): Unit = { - isStopped.set(true) - } - - import ExecutionContext.Implicits._ - Future { - while (!isReady.get && !isStopped.get) { - blocking { - Thread.sleep(500) - } - } - while (!isAllCompleted.get && !isStopped.get) { - blocking { - report() - Thread.sleep(500) - } - } - } + override def stop(): Unit = currentProgressThread.getAndSet(None).foreach(_.close()) override def afterAllCompleted(results: RMap[Task, Result]): Unit = { // send an empty progress report to clear out the previous report val event = ProgressEvent("Info", Vector(), Some(lastTaskCount.get), None, None) import sbt.internal.util.codec.JsonProtocol._ log.logEvent(Level.Info, event) - isAllCompleted.set(true) + stop() } private[this] val skipReportTasks = Set("run", "bgRun", "fgRun", "scala", "console", "consoleProject") From a820bb56230e84dde55a68bb323171e955cee612 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 14 May 2019 16:12:48 -0700 Subject: [PATCH 2/2] Sort the supershell tasks by task name This should make the output less jumpy. --- .../main/scala/sbt/internal/TaskProgress.scala | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/main/src/main/scala/sbt/internal/TaskProgress.scala b/main/src/main/scala/sbt/internal/TaskProgress.scala index 3fb598265..0c0c04677 100644 --- a/main/src/main/scala/sbt/internal/TaskProgress.scala +++ b/main/src/main/scala/sbt/internal/TaskProgress.scala @@ -80,10 +80,18 @@ private[sbt] final class TaskProgress(log: ManagedLogger) val ltc = lastTaskCount.get val currentTasksCount = currentTasks.size def report0(): Unit = { - val event = ProgressEvent("Info", currentTasks map { task => - val elapsed = timings.get(task).currentElapsedMicros - ProgressItem(taskName(task), elapsed) - }, Some(ltc), None, None) + val event = ProgressEvent( + "Info", + currentTasks + .map { task => + val elapsed = timings.get(task).currentElapsedMicros + ProgressItem(taskName(task), elapsed) + } + .sortBy(_.name), + Some(ltc), + None, + None + ) import sbt.internal.util.codec.JsonProtocol._ log.logEvent(Level.Info, event) }