Skip to content

Commit 46699a9

Browse files
pqCommit Queue
authored andcommitted
[das] allow interleaved requests in the message scheduler
Includes a new `allowOverlappingHandlers` flag to support re-disablement as discussed w/ Danny. Bug: #60440 Change-Id: I007f0276bf8cdbfd418ea5dacb251db3eef8dd3d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419340 Reviewed-by: Brian Wilkerson <[email protected]> Auto-Submit: Phil Quitslund <[email protected]> Commit-Queue: Phil Quitslund <[email protected]>
1 parent 44f8e21 commit 46699a9

File tree

2 files changed

+27
-1
lines changed

2 files changed

+27
-1
lines changed

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ sealed class MessageObject {}
7575
/// and the Diagnostic server. The [MessageScheduler] acts as a hub for all
7676
/// incoming messages and forwards the messages to the appropriate handlers.
7777
final class MessageScheduler {
78+
/// A flag to allow disabling overlapping message handlers.
79+
static bool allowOverlappingHandlers = true;
80+
7881
/// The [AnalysisServer] associated with the scheduler.
7982
late final AnalysisServer server;
8083

@@ -278,7 +281,20 @@ final class MessageScheduler {
278281
completer,
279282
);
280283
}
281-
await completer.future;
284+
285+
// Blocking here with an await on the future was intended to prevent unwanted
286+
// interleaving but was found to cause a significant performance regression.
287+
// For more context see: https://github.com/dart-lang/sdk/issues/60440.
288+
// To re-disable interleaving, set [allowOverlappingHandlers] to `false`.
289+
if (!allowOverlappingHandlers) {
290+
await completer.future;
291+
}
292+
293+
// NOTE that this message is not accurate if [allowOverlappingHandlers] is `true`
294+
// as in that case we're not blocking anymore and the future may not be complete.
295+
// TODO(pq): if not awaited, consider adding a `then` so we can track when the future completes
296+
// but note that we may see some flakiness in tests as message handling gets
297+
// non-deterministically interleaved.
282298
testView?.messageLog.add(
283299
' Complete ${message.runtimeType}: ${message.toString()}',
284300
);

pkg/analysis_server/test/integration/server/message_scheduler_test.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ Exit process messages loop
6868
}
6969

7070
Future<void> test_multipleRequests() async {
71+
if (MessageScheduler.allowOverlappingHandlers) return;
72+
7173
var futures = <Future<void>>[];
7274
futures.add(setRoots(included: [workspaceRootPath], excluded: []));
7375
var request = ExecutionCreateContextParams(
@@ -105,6 +107,8 @@ class LspServerMessageSchedulerTest extends RefactorCodeActionsTest {
105107
}
106108

107109
Future<void> test_documentChange() async {
110+
if (MessageScheduler.allowOverlappingHandlers) return;
111+
108112
const content = '''
109113
void f() {
110114
print('Test!');
@@ -178,6 +182,8 @@ Exit process messages loop
178182
}
179183

180184
Future<void> test_duplicateRequests() async {
185+
if (MessageScheduler.allowOverlappingHandlers) return;
186+
181187
const content = '''
182188
class B {
183189
@^
@@ -254,6 +260,8 @@ Exit process messages loop
254260
}
255261

256262
Future<void> test_multipleRequests() async {
263+
if (MessageScheduler.allowOverlappingHandlers) return;
264+
257265
const content = '''
258266
void main() {
259267
print('Hello world!!');
@@ -290,6 +298,8 @@ Exit process messages loop
290298
}
291299

292300
Future<void> test_response() async {
301+
if (MessageScheduler.allowOverlappingHandlers) return;
302+
293303
const content = '''
294304
void f() {
295305
print('Test!');

0 commit comments

Comments
 (0)