From 614d079da31508541015cae3de094601cbe80e0b Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 2 Oct 2023 14:14:30 +0200 Subject: [PATCH 1/2] improvement: show bsp as connected after the first message from build server --- .../metals/BuildServerConnection.scala | 31 ++++++----- .../metals/ServerLivenessMonitor.scala | 55 ++++++++++++------- .../src/main/scala/tests/TestingClient.scala | 4 +- .../tests/ServerLivenessMonitorLspSuite.scala | 4 +- .../tests/ServerLivenessMonitorSuite.scala | 12 ++-- 5 files changed, 64 insertions(+), 42 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala index 62ee1e5736c..6a9ba6622a8 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala @@ -454,10 +454,14 @@ object BuildServerConnection { def setupServer(): Future[LauncherConnection] = { connect().map { case conn @ SocketConnection(_, output, input, _, _) => val tracePrinter = Trace.setupTracePrinter("BSP", bspTraceRoot) - val requestMonitor = - if (addLivenessMonitor) Some(new RequestMonitorImpl) else None + val bspStatusOpt = + if (addLivenessMonitor) + Some(new BspStatus(languageClient, serverName, config.icons)) + else None + val requestMonitorOpt = + bspStatusOpt.map(new RequestMonitorImpl(_)) val wrapper: MessageConsumer => MessageConsumer = - requestMonitor.map(_.wrapper).getOrElse(identity) + requestMonitorOpt.map(_.wrapper).getOrElse(identity) val launcher = new Launcher.Builder[MetalsBuildServer]() .traceMessages(tracePrinter.orNull) @@ -484,17 +488,16 @@ object BuildServerConnection { } val optServerLivenessMonitor = - requestMonitor.map { - new ServerLivenessMonitor( - _, - () => server.workspaceBuildTargets(), - languageClient, - config.metalsToIdleTime, - config.pingInterval, - serverName, - config.icons, - ) - } + for { + bspStatus <- bspStatusOpt + requestMonitor <- requestMonitorOpt + } yield new ServerLivenessMonitor( + requestMonitor, + () => server.workspaceBuildTargets(), + config.metalsToIdleTime, + config.pingInterval, + bspStatus, + ) LauncherConnection( conn, diff --git a/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala b/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala index b0e1df86d1d..c6c78700343 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala @@ -4,6 +4,7 @@ import java.util.concurrent.Executors import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ScheduledFuture import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicReference import scala.concurrent.duration.Duration @@ -23,7 +24,7 @@ trait RequestMonitor { def lastIncoming: Option[Long] } -class RequestMonitorImpl extends RequestMonitor { +class RequestMonitorImpl(bspStatus: BspStatus) extends RequestMonitor { @volatile private var lastOutgoing_ : Option[Long] = None @volatile private var lastIncoming_ : Option[Long] = None @@ -44,7 +45,10 @@ class RequestMonitorImpl extends RequestMonitor { } private def outgoingMessage() = lastOutgoing_ = now - private def incomingMessage(): Unit = lastIncoming_ = now + private def incomingMessage(): Unit = { + bspStatus.connected() + lastIncoming_ = now + } private def now = Some(System.currentTimeMillis()) def lastOutgoing: Option[Long] = lastOutgoing_ @@ -54,22 +58,14 @@ class RequestMonitorImpl extends RequestMonitor { class ServerLivenessMonitor( requestMonitor: RequestMonitor, ping: () => Unit, - client: MetalsLanguageClient, metalsIdleInterval: Duration, pingInterval: Duration, - serverName: String, - icons: Icons, + bspStatus: BspStatus, ) { private val state: AtomicReference[ServerLivenessMonitor.State] = new AtomicReference(ServerLivenessMonitor.Idle) - @volatile private var isServerResponsive = true val scheduler: ScheduledExecutorService = Executors.newScheduledThreadPool(1) - val connectedParams: MetalsStatusParams = - ServerLivenessMonitor.connectedParams(serverName, icons) - val noResponseParams: MetalsStatusParams = - ServerLivenessMonitor.noResponseParams(serverName, icons) - client.metalsStatus(connectedParams) scribe.debug("starting server liveness monitor") def runnable(): Runnable = new Runnable { @@ -99,12 +95,9 @@ class ServerLivenessMonitor( case _ => } if (currState == ServerLivenessMonitor.Running) { - if (notResponding && isServerResponsive) { - scribe.debug("server liveness monitor detected no response") - client.metalsStatus(noResponseParams) - } else if (!notResponding && !isServerResponsive) - client.metalsStatus(connectedParams) - isServerResponsive = !notResponding + if (notResponding) { + bspStatus.noResponse() + } } scribe.debug("server liveness monitor: pinging build server...") ping() @@ -122,13 +115,13 @@ class ServerLivenessMonitor( TimeUnit.MILLISECONDS, ) - def isBuildServerResponsive: Boolean = isServerResponsive + def isBuildServerResponsive: Boolean = bspStatus.isBuildServerResponsive def shutdown(): Unit = { scribe.debug("shutting down server liveness monitor") scheduled.cancel(true) scheduler.shutdown() - client.metalsStatus(ServerLivenessMonitor.disconnectedParams) + bspStatus.connected() } def getState: ServerLivenessMonitor.State = state.get() @@ -147,6 +140,29 @@ object ServerLivenessMonitor { object FirstPing extends State object Running extends State +} + +class BspStatus( + client: MetalsLanguageClient, + serverName: String, + icons: Icons, +) { + private val isServerResponsive = new AtomicBoolean(false) + + def connected(): Unit = + if (isServerResponsive.compareAndSet(false, true)) + client.metalsStatus(BspStatus.connectedParams(serverName, icons)) + def noResponse(): Unit = + if (isServerResponsive.compareAndSet(true, false)) { + scribe.debug("server liveness monitor detected no response") + client.metalsStatus(BspStatus.noResponseParams(serverName, icons)) + } + def disconnected(): Unit = client.metalsStatus(BspStatus.disconnectedParams) + + def isBuildServerResponsive: Boolean = isServerResponsive.get() +} + +object BspStatus { def connectedParams(serverName: String, icons: Icons): MetalsStatusParams = MetalsStatusParams( s"$serverName ${icons.link}", @@ -167,5 +183,4 @@ object ServerLivenessMonitor { command = ServerCommands.ConnectBuildServer.id, commandTooltip = "Reconnect.", ).withStatusType(StatusType.bsp) - } diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 58802111bdc..eaa0f623542 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -16,6 +16,7 @@ import scala.meta.internal.builds.BspErrorHandler import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.decorations.PublishDecorationsParams +import scala.meta.internal.metals.BspStatus import scala.meta.internal.metals.Buffers import scala.meta.internal.metals.ClientCommands import scala.meta.internal.metals.FileOutOfScalaCliBspScope @@ -23,7 +24,6 @@ import scala.meta.internal.metals.Icons import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.ServerCommands -import scala.meta.internal.metals.ServerLivenessMonitor import scala.meta.internal.metals.TextEdits import scala.meta.internal.metals.WorkspaceChoicePopup import scala.meta.internal.metals.clients.language.MetalsInputBoxParams @@ -373,7 +373,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) ) { bspError } else if ( - params.getMessage() == ServerLivenessMonitor + params.getMessage() == BspStatus .noResponseParams("Bill", Icons.default) .logMessage(Icons.default) ) { diff --git a/tests/unit/src/test/scala/tests/ServerLivenessMonitorLspSuite.scala b/tests/unit/src/test/scala/tests/ServerLivenessMonitorLspSuite.scala index 12400fd1d28..3fcc0b08d7a 100644 --- a/tests/unit/src/test/scala/tests/ServerLivenessMonitorLspSuite.scala +++ b/tests/unit/src/test/scala/tests/ServerLivenessMonitorLspSuite.scala @@ -2,11 +2,11 @@ package tests import scala.concurrent.duration.Duration +import scala.meta.internal.metals.BspStatus import scala.meta.internal.metals.Icons import scala.meta.internal.metals.Messages import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MetalsServerConfig -import scala.meta.internal.metals.ServerLivenessMonitor import bill.Bill import ch.epfl.scala.bsp4j.BuildTargetIdentifier @@ -60,7 +60,7 @@ class ServerLivenessMonitorLspSuite extends BaseLspSuite("liveness-monitor") { ) ) _ = Thread.sleep(sleepTime) - noResponseParams = ServerLivenessMonitor.noResponseParams( + noResponseParams = BspStatus.noResponseParams( "Bill", Icons.default, ) diff --git a/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala b/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala index a8f2c0ac2ac..cb9ea60af14 100644 --- a/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala +++ b/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala @@ -8,6 +8,7 @@ import scala.concurrent.ExecutionContext import scala.concurrent.ExecutionContextExecutorService import scala.concurrent.duration.Duration +import scala.meta.internal.metals.BspStatus import scala.meta.internal.metals.Icons import scala.meta.internal.metals.RequestMonitor import scala.meta.internal.metals.ServerLivenessMonitor @@ -22,15 +23,15 @@ class ServerLivenessMonitorSuite extends BaseSuite { val pingInterval = Duration("3s") val server = new ResponsiveServer(pingInterval) val client = new CountMessageRequestsClient + val bspStatus = new BspStatus(client, "responsive-server", Icons.default) val livenessMonitor = new ServerLivenessMonitor( server, () => server.sendRequest(true), - client, metalsIdleInterval = pingInterval * 4, pingInterval, - "responsive-server", - Icons.default, + bspStatus, ) + bspStatus.connected() Thread.sleep(pingInterval.toMillis * 3 / 2) assertEquals(livenessMonitor.getState, ServerLivenessMonitor.Idle) server.sendRequest(false) @@ -46,6 +47,7 @@ class ServerLivenessMonitorSuite extends BaseSuite { server.sendRequest(false) assertEquals(livenessMonitor.getState, ServerLivenessMonitor.Running) assert(client.showMessageRequests == 0) + livenessMonitor.shutdown() } } @@ -85,7 +87,9 @@ class CountMessageRequestsClient extends NoopLanguageClient { var showMessageRequests = 0 override def metalsStatus(params: MetalsStatusParams): Unit = - if (params == ServerLivenessMonitor.disconnectedParams) { + if ( + params == BspStatus.noResponseParams("responsive-server", Icons.default) + ) { showMessageRequests += 1 } } From b9855f00bfb23b8401ee9486426c0776e6c86645 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Thu, 5 Oct 2023 08:24:08 +0200 Subject: [PATCH 2/2] fix mistake --- .../scala/meta/internal/metals/ServerLivenessMonitor.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala b/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala index c6c78700343..7d96008371c 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala @@ -121,7 +121,7 @@ class ServerLivenessMonitor( scribe.debug("shutting down server liveness monitor") scheduled.cancel(true) scheduler.shutdown() - bspStatus.connected() + bspStatus.disconnected() } def getState: ServerLivenessMonitor.State = state.get()