Skip to content

Commit 2b59598

Browse files
bwilkersonCommit Queue
authored andcommitted
Display message scheduler information on the diagnostic pages
This adds a new page to the diagnostic pages that displays the current state of the message scheduler (or rather the messages passing through the scheduler) as well as the most recent history. The next steps after this are to - include this information (and library cycle information) in the generated report - add this (and cycle) information to the analytics we're gathering I put the new page in its own file, and would like to move existing pages to separate files as well. Doing so might trigger moving some files around in the directory structure in order to better organize the code. Change-Id: Ic4f0e72ffbadc1e39702f1f8f13850f5bd5814a5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/431947 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
1 parent d49cd3c commit 2b59598

File tree

8 files changed

+411
-134
lines changed

8 files changed

+411
-134
lines changed

pkg/analysis_server/lib/src/analysis_server.dart

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import 'package:analysis_server/src/protocol_server.dart'
2727
show MessageType;
2828
import 'package:analysis_server/src/protocol_server.dart' as server;
2929
import 'package:analysis_server/src/scheduler/message_scheduler.dart';
30+
import 'package:analysis_server/src/scheduler/scheduler_tracking_listener.dart';
3031
import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
3132
import 'package:analysis_server/src/server/diagnostic_server.dart';
3233
import 'package:analysis_server/src/services/completion/completion_performance.dart';
@@ -296,8 +297,12 @@ abstract class AnalysisServer {
296297
httpClient,
297298
Platform.environment['PUB_HOSTED_URL'],
298299
),
299-
messageScheduler = MessageScheduler(listener: messageSchedulerListener) {
300-
messageScheduler.setServer(this);
300+
messageScheduler = MessageScheduler(
301+
listener:
302+
messageSchedulerListener ??
303+
SchedulerTrackingListener(analyticsManager),
304+
) {
305+
messageScheduler.server = this;
301306
// Set the default URI converter. This uses the resource providers path
302307
// context (unlike the initialized value) which allows tests to override it.
303308
uriConverter = ClientUriConverter.noop(baseResourceProvider.pathContext);

pkg/analysis_server/lib/src/scheduler/message_scheduler.dart

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ final class MessageScheduler {
2929
static bool allowOverlappingHandlers = true;
3030

3131
/// A listener that can be used to watch the scheduler as it manages messages.
32-
final MessageSchedulerListener? _listener;
32+
final MessageSchedulerListener? listener;
3333

3434
/// The [AnalysisServer] associated with the scheduler.
35-
late final AnalysisServer _server;
35+
late final AnalysisServer server;
3636

3737
/// The messages that have been received and are waiting to be handled.
3838
final ListQueue<ScheduledMessage> _pendingMessages =
@@ -50,13 +50,13 @@ final class MessageScheduler {
5050

5151
/// Initialize a newly created message scheduler.
5252
///
53-
/// The caller is expected to set the [_server] immediately after creating the
53+
/// The caller is expected to set the [server] immediately after creating the
5454
/// instance. The only reason the server isn't initialized by the constructor
5555
/// is because the analysis server and the message scheduler can't be created
5656
/// atomically and it was decided that it was cleaner for the scheduler to
5757
/// have the nullable reference to the server rather than the other way
5858
/// around.
59-
MessageScheduler({MessageSchedulerListener? listener}) : _listener = listener;
59+
MessageScheduler({required this.listener});
6060

6161
/// Add the [message] to the end of the pending messages queue.
6262
///
@@ -81,18 +81,18 @@ final class MessageScheduler {
8181
/// - The incoming [legacy.ANALYSIS_REQUEST_UPDATE_CONTENT] message cancels
8282
/// any rename files request that is in progress.
8383
void add(ScheduledMessage message) {
84-
_listener?.addPendingMessage(message);
84+
listener?.addPendingMessage(message);
8585
if (message is LegacyMessage) {
8686
var request = message.request;
8787
var method = request.method;
8888
if (method == legacy.SERVER_REQUEST_CANCEL_REQUEST) {
8989
var id =
9090
legacy.ServerCancelRequestParams.fromRequest(
9191
request,
92-
clientUriConverter: _server.uriConverter,
92+
clientUriConverter: server.uriConverter,
9393
).id;
94-
_listener?.addActiveMessage(message);
95-
(_server as LegacyAnalysisServer).cancelRequest(id);
94+
listener?.addActiveMessage(message);
95+
(server as LegacyAnalysisServer).cancelRequest(id);
9696
// The message needs to be added to the queue of pending messages, but
9797
// it seems like it shouldn't be necessary and that we ought to return
9898
// at this point. However, doing so causes some tests to timeout.
@@ -107,7 +107,7 @@ final class MessageScheduler {
107107
code: lsp.ErrorCodes.ContentModified.toJson(),
108108
reason: 'File content was modified',
109109
);
110-
_listener?.cancelActiveMessage(activeMessage);
110+
listener?.cancelActiveMessage(activeMessage);
111111
}
112112
}
113113
}
@@ -121,8 +121,8 @@ final class MessageScheduler {
121121
// Responses don't go on the queue because there might be an active
122122
// message that can't complete until the response is received. If the
123123
// response was added to the queue then this process could deadlock.
124-
_listener?.addActiveMessage(message);
125-
(_server as LspAnalysisServer).handleMessage(msg, null);
124+
listener?.addActiveMessage(message);
125+
(server as LspAnalysisServer).handleMessage(msg, null);
126126
return;
127127
} else if (msg is lsp.NotificationMessage) {
128128
var method = msg.method;
@@ -133,7 +133,7 @@ final class MessageScheduler {
133133
// by removing the request from the queue. It's done this way to allow
134134
// a response to be sent back to the client saying that the results
135135
// aren't provided because the request was cancelled.
136-
_listener?.addActiveMessage(message);
136+
listener?.addActiveMessage(message);
137137
_processCancellation(msg);
138138
return;
139139
} else if (method == lsp.Method.textDocument_didChange) {
@@ -155,7 +155,7 @@ final class MessageScheduler {
155155
var message = activeMessage.message as lsp.RequestMessage;
156156
if (message.method == incomingMsgMethod) {
157157
activeMessage.cancellationToken?.cancel(reason: reason);
158-
_listener?.cancelActiveMessage(activeMessage);
158+
listener?.cancelActiveMessage(activeMessage);
159159
}
160160
}
161161
}
@@ -164,7 +164,7 @@ final class MessageScheduler {
164164
var message = pendingMessage.message as lsp.RequestMessage;
165165
if (message.method == msg.method) {
166166
pendingMessage.cancellationToken?.cancel(reason: reason);
167-
_listener?.cancelPendingMessage(pendingMessage);
167+
listener?.cancelPendingMessage(pendingMessage);
168168
}
169169
}
170170
}
@@ -180,12 +180,12 @@ final class MessageScheduler {
180180
/// Dispatch the first message in the queue to be executed.
181181
void processMessages() async {
182182
_isProcessing = true;
183-
_listener?.startProcessingMessages();
183+
listener?.startProcessingMessages();
184184
try {
185185
while (_pendingMessages.isNotEmpty) {
186186
var currentMessage = _pendingMessages.removeFirst();
187187
_activeMessages.addLast(currentMessage);
188-
_listener?.addActiveMessage(currentMessage);
188+
listener?.addActiveMessage(currentMessage);
189189
_completer = Completer<void>();
190190
unawaited(
191191
_completer.future.then((_) {
@@ -195,27 +195,27 @@ final class MessageScheduler {
195195
switch (currentMessage) {
196196
case LspMessage():
197197
var lspMessage = currentMessage.message;
198-
(_server as LspAnalysisServer).handleMessage(
198+
(server as LspAnalysisServer).handleMessage(
199199
lspMessage,
200200
cancellationToken: currentMessage.cancellationToken,
201201
_completer,
202202
);
203203
case LegacyMessage():
204204
var request = currentMessage.request;
205-
(_server as LegacyAnalysisServer).handleRequest(
205+
(server as LegacyAnalysisServer).handleRequest(
206206
request,
207207
_completer,
208208
currentMessage.cancellationToken,
209209
);
210210
case DtdMessage():
211-
_server.dtd!.processMessage(
211+
server.dtd!.processMessage(
212212
currentMessage.message,
213213
currentMessage.performance,
214214
currentMessage.responseCompleter,
215215
_completer,
216216
);
217217
case WatcherMessage():
218-
_server.contextManager.handleWatchEvent(currentMessage.event);
218+
server.contextManager.handleWatchEvent(currentMessage.event);
219219
// Handling a watch event is a synchronous process, so there's
220220
// nothing to wait for.
221221
_completer.complete();
@@ -236,24 +236,17 @@ final class MessageScheduler {
236236
// TODO(pq): if not awaited, consider adding a `then` so we can track
237237
// when the future completes. But note that we may see some flakiness in
238238
// tests as message handling gets non-deterministically interleaved.
239-
_listener?.messageCompleted(currentMessage);
239+
listener?.messageCompleted(currentMessage);
240240
}
241241
} catch (error, stackTrace) {
242-
_server.instrumentationService.logException(
242+
server.instrumentationService.logException(
243243
FatalException('Failed to process message', error, stackTrace),
244244
null,
245-
_server.crashReportingAttachmentsBuilder.forException(error),
245+
server.crashReportingAttachmentsBuilder.forException(error),
246246
);
247247
}
248248
_isProcessing = false;
249-
_listener?.endProcessingMessages();
250-
}
251-
252-
/// Set the [AnalysisServer].
253-
///
254-
/// Throws an exception if the server has already been set.
255-
void setServer(AnalysisServer analysisServer) {
256-
_server = analysisServer;
249+
listener?.endProcessingMessages();
257250
}
258251

259252
/// Returns the parameters of a cancellation [message].
@@ -269,7 +262,7 @@ final class MessageScheduler {
269262
? cancelJsonHandler.convertParams(paramsJson)
270263
: null;
271264
} catch (error, stackTrace) {
272-
(_server as LspAnalysisServer).logException(
265+
(server as LspAnalysisServer).logException(
273266
'An error occured while parsing cancel parameters',
274267
error,
275268
stackTrace,
@@ -309,7 +302,7 @@ final class MessageScheduler {
309302
Map<String, Object?>? _getLspOverLegacyParams(legacy.Request request) {
310303
var params = legacy.LspHandleParams.fromRequest(
311304
request,
312-
clientUriConverter: _server.uriConverter,
305+
clientUriConverter: server.uriConverter,
313306
);
314307
return params.lspMessage as Map<String, Object?>;
315308
}
@@ -352,7 +345,7 @@ final class MessageScheduler {
352345
var request = activeMessage.message as lsp.RequestMessage;
353346
if (request.id == params.id) {
354347
activeMessage.cancellationToken?.cancel();
355-
_listener?.cancelActiveMessage(activeMessage);
348+
listener?.cancelActiveMessage(activeMessage);
356349
return;
357350
}
358351
}
@@ -362,7 +355,7 @@ final class MessageScheduler {
362355
var request = pendingMessage.message as lsp.RequestMessage;
363356
if (request.id == params.id) {
364357
pendingMessage.cancellationToken?.cancel();
365-
_listener?.cancelPendingMessage(pendingMessage);
358+
listener?.cancelPendingMessage(pendingMessage);
366359
return;
367360
}
368361
}
@@ -410,9 +403,9 @@ final class MessageScheduler {
410403
code: lsp.ErrorCodes.ContentModified.toJson(),
411404
);
412405
if (isActive) {
413-
_listener?.cancelActiveMessage(lspMessage);
406+
listener?.cancelActiveMessage(lspMessage);
414407
} else {
415-
_listener?.cancelPendingMessage(lspMessage);
408+
listener?.cancelPendingMessage(lspMessage);
416409
}
417410
}
418411
}
@@ -428,9 +421,9 @@ final class MessageScheduler {
428421
code: lsp.ErrorCodes.ContentModified.toJson(),
429422
);
430423
if (isActive) {
431-
_listener?.cancelActiveMessage(lspMessage);
424+
listener?.cancelActiveMessage(lspMessage);
432425
} else {
433-
_listener?.cancelPendingMessage(lspMessage);
426+
listener?.cancelPendingMessage(lspMessage);
434427
}
435428
}
436429
}
@@ -461,9 +454,13 @@ final class MessageScheduler {
461454

462455
abstract class MessageSchedulerListener {
463456
/// Report that the [message] was added to the active message queue.
457+
///
458+
/// This implies that the message is no longer on the pending message queue.
464459
void addActiveMessage(ScheduledMessage message);
465460

466461
/// Report that the [message] was added to the pending message queue.
462+
///
463+
/// This is always the first notification for the [message].
467464
void addPendingMessage(ScheduledMessage message);
468465

469466
/// Report that an active [message] was cancelled.
@@ -476,6 +473,8 @@ abstract class MessageSchedulerListener {
476473
void endProcessingMessages();
477474

478475
/// Report that the [message] has been completed.
476+
///
477+
/// This implies that the message was active and wasn't cancelled.
479478
void messageCompleted(ScheduledMessage message);
480479

481480
/// Report that the loop that processes messages has started to run.

pkg/analysis_server/lib/src/scheduler/scheduled_message.dart

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ final class DtdMessage extends ScheduledMessage {
3030
});
3131

3232
@override
33-
String toString() => message.method.toString();
33+
String get id => 'dtd:${message.method}';
3434
}
3535

3636
/// Represents a message in the Legacy protocol format.
@@ -45,7 +45,7 @@ final class LegacyMessage extends ScheduledMessage {
4545
LegacyMessage({required this.request, this.cancellationToken});
4646

4747
@override
48-
String toString() => request.method;
48+
String get id => 'legacy:${request.method}';
4949
}
5050

5151
/// Represents a message in the LSP protocol format.
@@ -59,26 +59,32 @@ final class LspMessage extends ScheduledMessage {
5959

6060
LspMessage({required this.message, this.cancellationToken});
6161

62-
bool get isRequest => message is lsp.RequestMessage;
63-
6462
@override
65-
String toString() {
63+
String get id {
6664
var msg = message;
6765
return switch (msg) {
68-
RequestMessage() => msg.method.toString(),
69-
NotificationMessage() => msg.method.toString(),
70-
ResponseMessage() => 'ResponseMessage',
71-
Message() => 'Message',
66+
RequestMessage() => 'lsp:${msg.method}',
67+
NotificationMessage() => 'lsp:${msg.method}',
68+
ResponseMessage() => 'lsp:ResponseMessage',
69+
Message() => 'lsp:Message',
7270
};
7371
}
72+
73+
bool get isRequest => message is lsp.RequestMessage;
7474
}
7575

7676
/// A message from a client.
7777
///
7878
/// The message can be either a request, a notification, or a response.
7979
///
8080
/// The client can be an IDE, a command-line tool, or DTD.
81-
sealed class ScheduledMessage {}
81+
sealed class ScheduledMessage {
82+
/// An identifier that identifies this particular kind of message.
83+
String get id;
84+
85+
@override
86+
String toString() => id;
87+
}
8288

8389
/// Represents a message from the file watcher.
8490
///
@@ -90,5 +96,5 @@ final class WatcherMessage extends ScheduledMessage {
9096
WatcherMessage(this.event);
9197

9298
@override
93-
String toString() => '${event.type} ${event.path}';
99+
String get id => 'watch:${event.type} ${event.path}';
94100
}

0 commit comments

Comments
 (0)