[ty] Add background request task support #19041

dhruvmanila · 2025-06-30T05:43:17Z

Summary

This PR adds a new trait to support running a request in the background.

Currently, there exists a BackgroundDocumentRequestHandler trait which is similar but is scoped to a specific document (file in an editor context). The new trait BackgroundRequestHandler is not tied to a specific document nor a specific project but it's for the entire workspace.

This is added to support running workspace wide requests like computing the workspace diagnostics or workspace symbols.

Note: There's a slight difference with what a "workspace" means between the server and ty. Currently, there's a 1-1 relationship between a workspace in an editor and the project database corresponding to that workspace in ty but this could change in the future when Micha adds support for multiple workspaces or multi-root workspaces.

The data that would be required by the request handler (based on implementing workspace diagnostics) is the list of databases (ProjectDatabse) corresponding to the projects in the workspace and the index (Index) that contains the open documents. The WorkspaceSnapshot represents this and is passed to the handler similar to DocumentSnapshot.

Test Plan

This is used in implementing the workspace diagnostics which is where this is tested.

github-actions · 2025-06-30T05:46:48Z

`mypy_primer` results

Changes were detected when running on open source projects

operator (https://github.com/canonical/operator)
- TOTAL MEMORY USAGE: ~106MB
+ TOTAL MEMORY USAGE: ~97MB

trio (https://github.com/python-trio/trio)
-     memo fields = ~156MB
+     memo fields = ~142MB

isort (https://github.com/pycqa/isort)
- TOTAL MEMORY USAGE: ~80MB
+ TOTAL MEMORY USAGE: ~88MB

vision (https://github.com/pytorch/vision)
- TOTAL MEMORY USAGE: ~334MB
+ TOTAL MEMORY USAGE: ~368MB

mkdocs (https://github.com/mkdocs/mkdocs)
-     memo fields = ~106MB
+     memo fields = ~97MB

tornado (https://github.com/tornadoweb/tornado)
- TOTAL MEMORY USAGE: ~156MB
+ TOTAL MEMORY USAGE: ~171MB

bandersnatch (https://github.com/pypa/bandersnatch)
-     memo fields = ~72MB
+     memo fields = ~66MB

paasta (https://github.com/yelp/paasta)
-     memo fields = ~156MB
+     memo fields = ~171MB

django-stubs (https://github.com/typeddjango/django-stubs)
- TOTAL MEMORY USAGE: ~171MB
+ TOTAL MEMORY USAGE: ~189MB

meson (https://github.com/mesonbuild/meson)
-     memo fields = ~334MB
+     memo fields = ~304MB

BurntSushi

Nice! My only comments are requesting docs, probably because I'm somewhat unfamiliar with this part of the LSP. :-)

BurntSushi · 2025-07-02T16:17:56Z

crates/ty_server/src/server/api.rs

@@ -135,7 +135,52 @@ where
    }))
 }

-fn background_request_task<R: traits::BackgroundDocumentRequestHandler>(
+#[expect(dead_code)]


Could you add a comment saying why this code is kept around even though it isn't used?

I split the workspace diagnostics work into two PRs to ease up the review process. This is going to be used in the follow-up PR (#18939). Sorry, I should've highlighted this as a review comment!

BurntSushi · 2025-07-02T16:22:48Z

crates/ty_server/src/server/api/traits.rs

+        snapshot: WorkspaceSnapshot,
+        client: &Client,
+        params: <<Self as RequestHandler>::RequestType as Request>::Params,
+    ) -> super::Result<<<Self as RequestHandler>::RequestType as Request>::Result>;


One thing I'd find helpful here is some prose (perhaps on the module doc) that briefly explains the traits here and how they're connected.

If you think this code is going to be heavily refactored soon, then it might not make sense to add docs.

Definitely! Most of this stuff is currently in my and Micha's mind but it would be useful to have it written down. I'll prioritize it soon as there are now more people who'll be working on the server.

codspeed-hq · 2025-07-03T11:07:53Z

CodSpeed WallTime Performance Report

Merging #19041 will degrade performances by 4.82%

_{Comparing dhruv/background-request-handler (1252436) with main (e212dc2)}

Summary

❌ 1 regressions
✅ 7 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
❌	`multithreaded[pydantic]`	321 ms	337.3 ms	-4.82%

MichaReiser · 2025-07-07T09:26:42Z

crates/ty_server/src/server/api/traits.rs

+/// A request handler that can be run on a background thread.
+pub(super) trait BackgroundRequestHandler: RetriableRequestHandler {
+    fn run(
+        snapshot: WorkspaceSnapshot,


I wonder if we should call this SessionSnapshot instead. It's unfortunate that the LSP specification made such a mess of the term workspace (which can refer to an open folder, or all open foldlers).

MichaReiser · 2025-07-07T09:30:10Z

crates/ty_server/src/session.rs

@@ -223,6 +224,14 @@ impl Session {
        self.index().key_from_url(url)
    }

+    pub(crate) fn take_workspace_snapshot(&self) -> WorkspaceSnapshot {
+        WorkspaceSnapshot {
+            projects: AssertUnwindSafe(self.projects.values().cloned().collect()),


I'd suggest removing the AssertUnwindSafe here and insteaed wrap the entire snapshot in crates/ty_server/src/server/api.rs in an AssertUnwindSafe and add a comment saying that it's safe to move it across unwind boundaries because the value isn't used after unwinding.

That would mean that the snapshot parameter type of BackgroundRequestHandler::run would need to be AssertUnwindSafe<SessionSnapshot>. Or, are you suggesting to wrap the AssertUnwindSafe on the entire closure that is the first argument to catch_unwind?

~~The former doesn't seems to work as the compiler still errors stating that some reference may not be safe to be transferred across the catch_unwind boundary.~~

Edit: Nevermind, it works.

MichaReiser · 2025-07-07T09:30:31Z

Nice!

dhruvmanila · 2025-07-07T14:03:10Z

Addressed these review comments in #19177.

This PR addresses the post-merge review comments from #19041, specifically it: - Rename `WorkspaceSnapshot` to `SessionSnapshot` - Rename `take_workspace_snapshot` to `take_session_snapshot` - Rename `take_snapshot` to `take_document_snapshot` - Move `AssertUnwindSafe` closer to the `catch_unwind` call which requires the assertion

dhruvmanila requested review from carljm, MichaReiser, AlexWaygood, sharkdp and dcreager as code owners June 30, 2025 05:43

dhruvmanila added internal An internal refactor or improvement ty Multi-file analysis & type inference labels Jun 30, 2025

dhruvmanila requested review from BurntSushi and removed request for dcreager, carljm, sharkdp and AlexWaygood June 30, 2025 13:48

BurntSushi approved these changes Jul 2, 2025

View reviewed changes

dhruvmanila added 3 commits July 3, 2025 09:31

[ty] Add background request task support

80cb820

Fix clippy

a1fa874

Merge branch 'main' into dhruv/background-request-handler

1252436

dhruvmanila force-pushed the dhruv/background-request-handler branch from 0e72155 to 1252436 Compare July 3, 2025 10:57

dhruvmanila enabled auto-merge (squash) July 3, 2025 10:59

dhruvmanila merged commit a95c18a into main Jul 3, 2025
35 checks passed

dhruvmanila deleted the dhruv/background-request-handler branch July 3, 2025 11:01

MichaReiser reviewed Jul 7, 2025

View reviewed changes

dhruvmanila mentioned this pull request Jul 7, 2025

Rename to SessionSnapshot, move unwind assertion closer #19177

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ty] Add background request task support #19041

[ty] Add background request task support #19041

Uh oh!

dhruvmanila commented Jun 30, 2025

Uh oh!

github-actions bot commented Jun 30, 2025 •

edited

Loading

Uh oh!

BurntSushi left a comment

Uh oh!

BurntSushi Jul 2, 2025

Uh oh!

dhruvmanila Jul 3, 2025

Uh oh!

BurntSushi Jul 2, 2025

Uh oh!

dhruvmanila Jul 3, 2025

Uh oh!

Uh oh!

codspeed-hq bot commented Jul 3, 2025

Uh oh!

MichaReiser Jul 7, 2025

Uh oh!

MichaReiser Jul 7, 2025

Uh oh!

dhruvmanila Jul 7, 2025

Uh oh!

dhruvmanila Jul 7, 2025 •

edited

Loading

Uh oh!

MichaReiser commented Jul 7, 2025

Uh oh!

dhruvmanila commented Jul 7, 2025

Uh oh!

Uh oh!

[ty] Add background request task support #19041

[ty] Add background request task support #19041

Uh oh!

Conversation

dhruvmanila commented Jun 30, 2025

Summary

Test Plan

Uh oh!

github-actions bot commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

BurntSushi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codspeed-hq bot commented Jul 3, 2025

Merging #19041 will degrade performances by 4.82%

Summary

Benchmarks breakdown

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented Jul 7, 2025

Uh oh!

dhruvmanila commented Jul 7, 2025

Uh oh!

Uh oh!

github-actions bot commented Jun 30, 2025 •

edited

Loading

`mypy_primer` results

dhruvmanila Jul 7, 2025 •

edited

Loading