Skip to content

Commit fcfb242

Browse files
committed
Run worksheets asynchronously
A worksheet in an infinite loop shouldn't block the whole language server from responding to queries. We just need to be careful to lock the language server when operating on trees since these operations may not be thread-safe (for example, when a symbol gets forced). We could also allow running multiple worksheets at the same time but this commit doesn't implement that.
1 parent 99ac060 commit fcfb242

File tree

5 files changed

+67
-60
lines changed

5 files changed

+67
-60
lines changed

language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class DottyLanguageServer extends LanguageServer
107107
if (Memory.isCritical()) CompletableFutures.computeAsync { _ => restart() }
108108

109109
/** The driver instance responsible for compiling `uri` */
110-
def driverFor(uri: URI): InteractiveDriver = {
110+
def driverFor(uri: URI): InteractiveDriver = thisServer.synchronized {
111111
val matchingConfig =
112112
drivers.keys.find(config => config.sourceDirectories.exists(sourceDir =>
113113
new File(uri.getPath).getCanonicalPath.startsWith(sourceDir.getCanonicalPath)))
@@ -133,10 +133,10 @@ class DottyLanguageServer extends LanguageServer
133133
CompletableFuture.completedFuture(new Object)
134134
}
135135

136-
def computeAsync[R](fun: CancelChecker => R): CompletableFuture[R] =
136+
def computeAsync[R](fun: CancelChecker => R, synchronize: Boolean = true): CompletableFuture[R] =
137137
CompletableFutures.computeAsync { cancelToken =>
138138
// We do not support any concurrent use of the compiler currently.
139-
thisServer.synchronized {
139+
def computation(): R = {
140140
cancelToken.checkCanceled()
141141
checkMemory()
142142
try {
@@ -147,6 +147,10 @@ class DottyLanguageServer extends LanguageServer
147147
throw ex
148148
}
149149
}
150+
if (synchronize)
151+
synchronized { computation() }
152+
else
153+
computation()
150154
}
151155

152156
override def initialize(params: InitializeParams) = computeAsync { cancelToken =>

language-server/src/dotty/tools/languageserver/Main.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ object Main {
7373
.setInput(in)
7474
.setOutput(out)
7575
// For debugging JSON messages:
76-
// .traceMessages(new java.io.PrintWriter(System.err, true))
76+
//.traceMessages(new java.io.PrintWriter(System.err, true))
7777
.create();
7878

7979
val client = launcher.getRemoteProxy()

language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ private object Evaluator {
2828
* @param cancelChecker The token that indicates whether evaluation has been cancelled.
2929
* @return A JVM running the REPL.
3030
*/
31-
def get(cancelChecker: CancelChecker)(implicit ctx: Context): Option[Evaluator] = {
31+
def get(cancelChecker: CancelChecker)(implicit ctx: Context): Option[Evaluator] = synchronized {
3232
val classpath = ctx.settings.classpath.value
3333
previousEvaluator match {
3434
case Some(cp, evaluator) if evaluator.isAlive() && cp == classpath =>

language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,60 +10,68 @@ import dotty.tools.dotc.core.Flags.Synthetic
1010

1111
import org.eclipse.lsp4j.jsonrpc.CancelChecker
1212

13-
import java.util.concurrent.CancellationException
13+
import java.util.concurrent.{CompletableFuture, CancellationException}
1414

1515
object Worksheet {
1616

1717
/**
1818
* Run `tree` as a worksheet using the REPL.
1919
*
2020
* @param tree The top level object wrapping the worksheet.
21+
* @param treeLock Object on which to lock when doing operations on trees.
2122
* @param sendMessage A mean of communicating the results of evaluation back.
2223
* @param cancelChecker A token to check whether execution should be cancelled.
2324
*/
2425
def run(tree: SourceTree,
26+
treeLock: Object,
2527
sendMessage: (Int, String) => Unit,
2628
cancelChecker: CancelChecker)(
27-
implicit ctx: Context): Unit = synchronized {
29+
implicit ctx: Context): Unit = {
30+
// For now, don't try to run multiple evaluators in parallel, this would require
31+
// changes to the logic of Evaluator.get among other things.
32+
Evaluator.synchronized {
33+
Evaluator.get(cancelChecker) match {
34+
case None =>
35+
sendMessage(1, "Couldn't start the JVM.")
36+
case Some(evaluator) =>
37+
val queries = treeLock.synchronized {
38+
tree.tree match {
39+
case td @ TypeDef(_, template: Template) =>
40+
val seen = collection.mutable.Set.empty[(Int, Int)]
2841

29-
Evaluator.get(cancelChecker) match {
30-
case None =>
31-
sendMessage(1, "Couldn't start JVM.")
32-
case Some(evaluator) =>
33-
tree.tree match {
34-
case td @ TypeDef(_, template: Template) =>
35-
val executed = collection.mutable.Set.empty[(Int, Int)]
36-
37-
template.body.foreach {
38-
case statement: DefTree if statement.symbol.is(Synthetic) =>
39-
()
40-
41-
case statement if evaluator.isAlive() && executed.add(bounds(statement.pos)) =>
42-
try {
43-
cancelChecker.checkCanceled()
44-
val (line, result) = execute(evaluator, statement, tree.source)
45-
if (result.nonEmpty) sendMessage(line, result)
46-
} catch { case _: CancellationException => () }
47-
48-
case _ =>
49-
()
42+
template.body.flatMap {
43+
case statement: DefTree if statement.symbol.is(Synthetic) =>
44+
None
45+
case statement if seen.add(bounds(statement.pos)) =>
46+
Some(query(statement, tree.source))
47+
case _ =>
48+
None
49+
}
5050
}
51-
}
51+
}
52+
queries.foreach { (line, code) =>
53+
cancelChecker.checkCanceled()
54+
val res = evaluator.eval(code).getOrElse("")
55+
cancelChecker.checkCanceled()
56+
if (res.nonEmpty)
57+
sendMessage(line, res)
58+
}
59+
}
5260
}
5361
}
5462

5563
/**
56-
* Extract `tree` from the source and evaluate it in the REPL.
64+
* Extract the line number and source code corresponding to this tree
5765
*
5866
* @param evaluator The JVM that runs the REPL.
5967
* @param tree The compiled tree to evaluate.
6068
* @param sourcefile The sourcefile of the worksheet.
61-
* @return The line in the sourcefile that corresponds to `tree`, and the result.
6269
*/
63-
private def execute(evaluator: Evaluator, tree: Tree, sourcefile: SourceFile): (Int, String) = {
64-
val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString
70+
private def query(tree: Tree, sourcefile: SourceFile): (Int, String) = {
6571
val line = sourcefile.offsetToLine(tree.pos.end)
66-
(line, evaluator.eval(source).getOrElse(""))
72+
val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString
73+
(line, source)
74+
// CompletableFuture.supplyAsync(() => (line, evaluator.eval(source).getOrElse("")))
6775
}
6876

6977
private def bounds(pos: Position): (Int, Int) = (pos.start, pos.end)

language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,23 @@ import java.util.concurrent.{CompletableFuture, ConcurrentHashMap}
1313
@JsonSegment("worksheet")
1414
trait WorksheetService { thisServer: DottyLanguageServer =>
1515

16-
val worksheets: ConcurrentHashMap[URI, CompletableFuture[_]] = new ConcurrentHashMap()
17-
1816
@JsonRequest
19-
def run(params: WorksheetRunParams): CompletableFuture[WorksheetRunResult] = thisServer.synchronized {
20-
val uri = new URI(params.textDocument.getUri)
21-
val future =
22-
computeAsync { cancelChecker =>
23-
try {
24-
val driver = driverFor(uri)
25-
val sendMessage = (line: Int, msg: String) => client.publishOutput(WorksheetRunOutput(params.textDocument, line, msg))
26-
runWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx)
27-
WorksheetRunResult(success = true)
28-
} catch {
29-
case _: Throwable =>
30-
WorksheetRunResult(success = false)
31-
} finally {
32-
worksheets.remove(uri)
33-
}
17+
def run(params: WorksheetRunParams): CompletableFuture[WorksheetRunResult] =
18+
computeAsync(synchronize = false, fun = { cancelChecker =>
19+
val uri = new URI(params.textDocument.getUri)
20+
try {
21+
val driver = driverFor(uri)
22+
val sendMessage =
23+
(line: Int, msg: String) => client.publishOutput(WorksheetRunOutput(params.textDocument, line, msg))
24+
25+
runWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx)
26+
cancelChecker.checkCanceled()
27+
WorksheetRunResult(success = true)
28+
} catch {
29+
case _: Throwable =>
30+
WorksheetRunResult(success = false)
3431
}
35-
worksheets.put(uri, future)
36-
future
37-
}
32+
})
3833

3934
/**
4035
* Run the worksheet at `uri`.
@@ -45,13 +40,13 @@ trait WorksheetService { thisServer: DottyLanguageServer =>
4540
* @param cancelChecker Token to check whether evaluation was cancelled
4641
*/
4742
private def runWorksheet(driver: InteractiveDriver,
48-
uri: URI,
49-
sendMessage: (Int, String) => Unit,
50-
cancelChecker: CancelChecker)(
43+
uri: URI,
44+
sendMessage: (Int, String) => Unit,
45+
cancelChecker: CancelChecker)(
5146
implicit ctx: Context): Unit = {
52-
val trees = driver.openedTrees(uri)
53-
trees.headOption.foreach { tree =>
54-
Worksheet.run(tree, sendMessage, cancelChecker)
47+
val treeOpt = thisServer.synchronized {
48+
driver.openedTrees(uri).headOption
5549
}
50+
treeOpt.foreach(tree => Worksheet.run(tree, thisServer, sendMessage, cancelChecker))
5651
}
5752
}

0 commit comments

Comments
 (0)