Skip to content

[DAS] [perf] stop blocking in the message scheduler #60440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
pq opened this issue Mar 31, 2025 · 2 comments
Closed

[DAS] [perf] stop blocking in the message scheduler #60440

pq opened this issue Mar 31, 2025 · 2 comments
Assignees
@pq
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. P0 A serious issue requiring immediate resolution type-performance Issue relates to performance or code size

Comments

@pq
Copy link
Member

pq commented Mar 31, 2025 •
edited
Loading

As reported in #60335, server performance regressed significantly in 3.7 and @jensjoha has identified some blocking in the (incomplete) message scheduler as being the likely culprit. This issue tracks a (semantic) revert of this blocking behavior. When we're in the clear and can resume the scheduler work the issue can transition to being about it's reenablement.

/fyi @bwilkerson @mraleph @kenzieschmoll
@pq pq added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. P0 A serious issue requiring immediate resolution type-performance Issue relates to performance or code size labels Mar 31, 2025
@pq pq self-assigned this Mar 31, 2025
@pq
Copy link
Member Author

pq commented Mar 31, 2025

FWIW, it's possible that it may be sufficient to just disable the await that @jensjoha removed in his benchmarking for a quick fix.

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

Line 281 in 19c5d0d

await completer.future;

Investigating in: https://dart-review.googlesource.com/c/sdk/+/419340

copybara-service bot pushed a commit that referenced this issue Apr 1, 2025
@pq
[das] allow interleaved requests in the message scheduler
46699a9 
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]>
copybara-service bot pushed a commit that referenced this issue Apr 1, 2025
@DanTup
[analysis_server] Allow allowOverlappingHandlers to be controlled via…
21c0584 
… initializationOptions

For troubleshooting, this allows the client to override this flag.

I'd originally planned to use ClientConfiguration for this and allow it to be changed on-the-fly, however this can result in a mix of requests and therefore I decided it best to only support setting once during startup (which is part of initialization, and therefore before general requests start being sent).

This will require some Dart-Code work to provide a value here (and without a value, it will always be the servers default).

See #60440

Change-Id: Ie9843543d6d491afb046f3d1106211b7db852605
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419541
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>
@pq
Copy link
Member Author

pq commented Apr 1, 2025

Implemented w/ 46699a9.

@pq pq closed this as completed Apr 1, 2025
copybara-service bot pushed a commit that referenced this issue Apr 10, 2025
@pq
[stable] [das] allow interleaved requests in the message scheduler
0fc5a38 
Issue description: disallowing interleaved message handling requests was causing major performance issues.
What is the fix: stop awaiting handlers and allow interleaving.
Why cherry-pick: this performance issue is heavily impacting a number of users.
Risk: Low, but not none. Some issues have been identified (in the linked issue) and are being worked on concurrently.

Bug: #60440
Change-Id: I176371f5609138eeb8a81a231f4a51a3a00dd37d
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/419340
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421341
Commit-Queue: Phil Quitslund <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Kevin Chisholm <[email protected]>
Reviewed-by: Alexander Thomas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pq pq

Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. P0 A serious issue requiring immediate resolution type-performance Issue relates to performance or code size
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@pq