Skip to content
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.

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

Provide guidance how language servers should support Virtual Workspaces #123025

Closed
davidanthoff opened this issue May 5, 2021 · 17 comments
Closed
Assignees
Labels
feature-request Request for new features or functionality

Comments

@davidanthoff
Copy link
Contributor

The new Virtual Workspace feature is great! Could you provide guidance how we might support it from our language server?

At the moment, our LSP implementation reads files from disc all the time. I've long hoped that instead of using direct file IO we could just do calls from the server to the client via LSP to read file content, and if that supported custom URI schemes, I guess this would work. But at the moment it is not clear to me how we could get our LS to support a virtual workspace.

@tamuratak
Copy link
Contributor

@jrieken
Copy link
Member

jrieken commented May 6, 2021

I've long hoped that instead of using direct file IO we could just do calls from the server to the client via LSP to read file content,

The VSCode API has this since a while. There is vscode.workspace.fs which should allow you to do the basic file operations like read, write, stat, readdir, delete etc.

@davidanthoff
Copy link
Contributor Author

Yes, I'm aware of the vscode.workspace.fs API. But I can only use that from within the extension, right? Not from within our language server implementation. Essentially I'm thinking that we need the equivalent of vscode.workspace.fs in the language server, i.e. a LSP version that has JSON RPC methods for exactly those kind of file IO operations?

@jrieken
Copy link
Member

jrieken commented May 6, 2021

Not from within our language server implementation. Essentially I'm thinking that we need the equivalent of vscode.workspace.fs in the language server

Yes - only through some custom protocol messages it can be used

@davidanthoff
Copy link
Contributor Author

Would it make sense to extend the LSP to include those kind of protocol messages, and have the LanguageClient class provide a default implementation on the extension side that uses vscode.workspace.fs? It seems that almost any language server implementation that wants to work in a virtual workspace would need this, right?

@dbaeumer
Copy link
Member

There is work under way that will add FS support to LSP.

@heejaechang
Copy link

heejaechang commented Jun 14, 2021

@dbaeumer I hope that includes fs access to outside of workspace root files (it is common libraries to be installed outside of workspace root) and file watcher support for those files as well. also, not all LSP server is asynchronous, so hopefully, those API support synchronous API as well like fs.

@dbaeumer
Copy link
Member

@heejaechang what range of files the provider will support depends on the provider itself and is out of the control of the protocol or VS Code API. For example the file system provider for file supports files outside the workspace since it can read it. The vscode-vfs can only provide access to the files on GitHub for example and therefore under the workspace root. It can't support reading files outside the repository.

As all process to process API we can only make this async.

@jakebailey
Copy link
Member

jakebailey commented Jun 15, 2021

That's going to end up being quite problematic for Pylance/pyright, where we are deeply synchronous during the analysis. I'm wondering how TypeScript is able to handle this, as they have a very similar architecture and are unlikely to be able to rearchitect to be async any time soon.

(Further, our cancellation itself is based on FS accesses to deal with the lack of async message handling.)

@dbaeumer
Copy link
Member

I do have a similar problem with ESLint and I solved it by yielding as much as possible. This can be done without making all the code async (ESLint itself was sync until version 7). Doing this allows for example for normal cancellation and the json-rpc libraries do buffer all messages internally to make this easier. We even do the yielding approach in VS Code itself. When we colorize a file we yield every n lines so that events and other things can be processed. In Pylance/pyright you might be able to easily yield after every file which would only make the top level code async.

I haven't looked at TypeScript since quite some time but I do know that they introduced the concept of workers to be able to yield more efficiently. But you need to talk to the TS people.

@dbaeumer
Copy link
Member

Another approach could be https://github.com/abbr/deasync. But I also heard about problems using it (e.g. total dead locks).

@jakebailey
Copy link
Member

The problem is twofold; the plumbing (as async/await is infectious), and the assumptions we have made about reentrancy.

Libraries like deasync would yield back to the event loop, so it's possible that we then handle some other request that reenters the critical analysis code and breaks our assumptions, so that wouldn't be something I'd want to use.

(deasync and libs like it are also a compiled module so they can hack into V8, which are not so fun to manage for VS Code extensions, as compared to things that users npm install.)

@heejaechang
Copy link

@dbaeumer won't yield cause new message in the event loop to be picked up before finishing processing current message? that will introduce re-entrance and can cause problem for us. (such as updating parse tree in the middle of processing them in the other task). I believe it has same blocking issue on why it is so hard for us to move to async (we need to make our LS able to handle multiple requests)

and unfortunately, any lib that involves native node lib is problem for us since we need to run our LS in browser as well in near future.

that being said, can you show me how you yield in ESLint? I am looking this (https://github.com/microsoft/vscode-eslint/blob/main/server/src/eslintServer.ts) but not sure where I should look.

@dbaeumer
Copy link
Member

The trick is here: https://github.com/microsoft/vscode-eslint/blob/main/server/src/eslintServer.ts#L1029

I take every message from JSON-RPC and but them into my own queue. In that queue I know better when to execute the messages. I even drop message if I know that I can skip the result. https://github.com/microsoft/vscode-eslint/blob/main/server/src/eslintServer.ts#L1130

I do agree that this is not for free but it does allow for more yielding than executing everything sync.

Regarding deasync: I fully agree with the concerns and I kept my hands off something like this since it does ask for problems. I simply wanted to point out the possibility.

@dbaeumer dbaeumer added the feature-request Request for new features or functionality label Oct 18, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

7 participants
@davidanthoff @heejaechang @jrieken @dbaeumer @jakebailey @tamuratak and others