From 4ec6339794c2d72eb02527b769b8f3972719cb5b Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Wed, 23 Sep 2020 12:07:56 -0700 Subject: [PATCH 1/2] Fix logical bug in task report scheduling The conditional for whether to make task progress events repeatable was inverted. This wasn't actually noticeable because the function doReport() was being schedule which had a guard to prevent it from running more frequently than the report period. --- main/src/main/scala/sbt/internal/TaskProgress.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/main/src/main/scala/sbt/internal/TaskProgress.scala b/main/src/main/scala/sbt/internal/TaskProgress.scala index 4234daf1b..ccb3c2eda 100644 --- a/main/src/main/scala/sbt/internal/TaskProgress.scala +++ b/main/src/main/scala/sbt/internal/TaskProgress.scala @@ -48,8 +48,9 @@ private[sbt] class TaskProgress( val delay = duration.toMillis try { val future = - if (recurring) scheduler.schedule(runnable, delay, TimeUnit.MILLISECONDS) - else scheduler.scheduleAtFixedRate(runnable, delay, delay, TimeUnit.MILLISECONDS) + if (recurring) + scheduler.scheduleAtFixedRate(runnable, delay, delay, TimeUnit.MILLISECONDS) + else scheduler.schedule(runnable, delay, TimeUnit.MILLISECONDS) pending.add(future) () => Util.ignoreResult(future.cancel(true)) } catch { From d930cb198757de54cff04a7da6b77bbf86467db3 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Wed, 23 Sep 2020 12:09:31 -0700 Subject: [PATCH 2/2] Don't do progress work on the main thread I noticed that no-op compile was slower in https://github.com/sbt/sbt/issues/5508 using 1.4.0-RC2 than 1.4.0-RC1. It took around 400ms with 1.4.0-RC2 and 200-250ms on RC1. Git bisect brought me to 41afe9fbdb0e311e35e3f7d9af225e65901a488b which I remembered I'd been slightly concerned about from a performance perspective but didn't get around to testing. The problem is that we were blocking the task from running while determing whether or not we should force a progress report. We can do that work on the background thread instead so the task can begin running immediately. --- main/src/main/scala/sbt/internal/TaskProgress.scala | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/main/src/main/scala/sbt/internal/TaskProgress.scala b/main/src/main/scala/sbt/internal/TaskProgress.scala index ccb3c2eda..f8308c5ef 100644 --- a/main/src/main/scala/sbt/internal/TaskProgress.scala +++ b/main/src/main/scala/sbt/internal/TaskProgress.scala @@ -107,10 +107,15 @@ private[sbt] class TaskProgress( override def afterReady(task: Task[_]): Unit = if (!closed.get) { - if (skipReportTasks.contains(getShortName(task))) { - lastTaskCount.set(-1) // force a report for remote clients - report() - } else Util.ignoreResult(active.put(task, schedule(threshold, recurring = false)(doReport()))) + try { + Util.ignoreResult(executor.submit((() => { + if (skipReportTasks.contains(getShortName(task))) { + lastTaskCount.set(-1) // force a report for remote clients + report() + } else + Util.ignoreResult(active.put(task, schedule(threshold, recurring = false)(doReport()))) + }): Runnable)) + } catch { case _: RejectedExecutionException => } } else { logger.debug(s"called afterReady for ${taskName(task)} after task progress was closed") }