Skip to content

Commit

Permalink
Consider typechecking phase for timeout of save actions
Browse files Browse the repository at this point in the history
So far only the running time of the save action itself was considered in
the timeout. With this change, the typechecking phase of the compiler
that is needed for every compiler driven save action is also considered
in this timeout.

Another change is that after a timeout occurred, all further compiler
driven save actions are aborted. This should be the better behavior
because it is not likely that the time that is needed to typecheck and
refactor a file largely differs between different save actions. Therefore
it is unrealistic that further save actions can succeed. Also, a save
action can not be aborted (because interrupting the compiler thread is
not supported) and therefore another call to the compiler first have to
wait on the previous request to be completed, which makes it even more
unrealistic that further save actions can be completed successfully.

Fixes #1002308
  • Loading branch information
kiritsuku committed May 27, 2015
1 parent 4652c00 commit 1c10f07
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 21 deletions.
@@ -1,6 +1,5 @@
package org.scalaide.ui.internal.editor

import scala.concurrent._
import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration._
import scala.reflect.internal.util.SourceFile
Expand Down Expand Up @@ -39,6 +38,7 @@ import org.scalaide.extensions.saveactions.RemoveTrailingWhitespaceSetting
import org.scalaide.logging.HasLogger
import org.scalaide.util.eclipse.EclipseUtils
import org.scalaide.util.eclipse.EditorUtils
import org.scalaide.util.internal.FutureUtils
import org.scalaide.util.internal.FutureUtils.TimeoutFuture
import org.scalaide.util.internal.eclipse.TextEditUtils

Expand Down Expand Up @@ -153,31 +153,63 @@ trait SaveActionExtensions extends HasLogger {
}

/**
* Applies all save actions that extends [[DocumentSupport]].
* Applies all save actions that extend [[DocumentSupport]].
*/
private def applyDocumentExtensions(udoc: IDocument): Unit = {
for ((setting, ext) <- documentSaveActions if isEnabled(setting.id)) {
val doc = new TextDocument(udoc)
val instance = ext(doc)
performExtension(instance, udoc) {
performDocumentExtension(instance, udoc) {
instance.perform()
}
}
}

/**
* Applies all save actions that extends [[CompilerSupport]].
* Applies all save actions that extend [[CompilerSupport]].
*/
private def applyCompilerExtensions(udoc: IDocument): Unit = {
for ((setting, ext) <- compilerSaveActions if isEnabled(setting.id)) {
createExtensionWithCompilerSupport(ext) foreach { instance =>
performExtension(instance, udoc) {
instance.global.asInstanceOf[IScalaPresentationCompiler].asyncExec {
instance.perform()
}.getOrElse(Seq())()
val timeout = saveActionTimeout

def loop(xs: Seq[(SaveActionSetting, CompilerSupportCreator)]): Unit = xs match {
case Nil

case (setting, ext) :: xs if isEnabled(setting.id)
val res = FutureUtils.performWithTimeout(timeout) {
EclipseUtils.withSafeRunner(s"An error occurred while executing compiler driven save action.") {
createExtensionWithCompilerSupport(ext) map { instance =>
instance.global.asInstanceOf[IScalaPresentationCompiler].asyncExec {
instance.perform()
}.getOrElse(Seq())()
}
}.flatten.getOrElse(Seq())
}
}

res match {
case Success(changes)
EclipseUtils.withSafeRunner(s"An error occurred while applying changes of save action '${setting.id}'.") {
applyChanges(setting.id, changes, udoc)
}
loop(xs)

case Failure(_)
eclipseLog.error(s"""|
|A save action that relies on compiler support didn't complete in a
| given time, it had $timeout time to complete. Because it is
| unlikely that further save actions that rely on compiler support will
| complete in time, no further save actions are executed. Please consider
| to disable save actions that rely on the compiler. The performed save
| action itself couldn't be aborted, therefore if you know that it may
| never complete in future, you may wish to restart your Eclipse to
| clean up your VM.
|
|""".stripMargin.replaceAll("\n", ""))
}

case _ :: xs
loop(xs)
}
loop(compilerSaveActions)
}

/**
Expand All @@ -191,22 +223,17 @@ trait SaveActionExtensions extends HasLogger {
* get a sequence of changes. It is executes in a safe environment that
* catches errors.
*/
private def performExtension(instance: SaveAction, udoc: IDocument)(ext: => Seq[Change]): Unit = {
private def performDocumentExtension(instance: SaveAction, udoc: IDocument)(ext: => Seq[Change]): Unit = {
val id = instance.setting.id
val timeout = saveActionTimeout

val futureToUse: Seq[Change] => Future[Seq[Change]] =
if (IScalaPlugin().noTimeoutMode)
Future(_)
else
TimeoutFuture(timeout)(_)

val f = futureToUse {
val res = FutureUtils.performWithTimeout(timeout) {
EclipseUtils.withSafeRunner(s"An error occurred while executing save action '$id'.") {
ext
}.getOrElse(Seq())
}
Await.ready(f, Duration.Inf).value.get match {

res match {
case Success(changes) =>
EclipseUtils.withSafeRunner(s"An error occurred while applying changes of save action '$id'.") {
applyChanges(id, changes, udoc)
Expand Down
Expand Up @@ -10,7 +10,7 @@ import scala.concurrent.duration.Duration
import scala.concurrent.duration.FiniteDuration
import scala.util.Try

import scala.concurrent.Future
import org.scalaide.core.IScalaPlugin

object FutureUtils {

Expand Down Expand Up @@ -50,4 +50,22 @@ object FutureUtils {
blocking { Try(Await.ready(never[Unit], time)) }
}
}

/**
* Performs an operation, but waits not more than `timeout` for a completion
* of the operation. Returns a [[Success]] that contains the result of `f` if
* it performed in time, otherwise a [[Failure]].
*
* If `IScalaPlugin.noTimeoutMode` is set, the timeout is not considered.
*/
def performWithTimeout[A](timeout: FiniteDuration)(f: => A): Try[A] = {
val futureToUse =
if (IScalaPlugin().noTimeoutMode)
Future(f)
else
TimeoutFuture(timeout)(f)

Await.ready(futureToUse, Duration.Inf).value.get
}

}

0 comments on commit 1c10f07

Please sign in to comment.