[go: nahoru, domu]

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

Chat leaks text models #216577

Open
bpasero opened this issue Jun 19, 2024 · 5 comments
Open

Chat leaks text models #216577

bpasero opened this issue Jun 19, 2024 · 5 comments
Assignees
Labels
freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues
Milestone

Comments

@bpasero
Copy link
Member
bpasero commented Jun 19, 2024

When I get a code block as response, I see 2 text models being created with URIs such as:

vscode-copilot-chat-code-block://6c0321c2-3c9c-4f8a-8b14-c9a737ff5a39/response_5/0#%7B%22references%22%3A%5B%5D%7D

vscode-chat-code-block://e7d2cd80-6c6b-470c-af0c-21391c43e841/response_3/0#%7B%22references%22%3A%5B%5D%7D

When opening a new chat I only see the model of vscode-chat-code-block getting disposed.

I think this could be the source of many listener leak warnings in error telemetry.

//cc @jrieken

@bpasero bpasero added the freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues label Jun 19, 2024
@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2024
@bpasero bpasero reopened this Jun 19, 2024
@microsoft microsoft unlocked this conversation Jun 19, 2024
@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2024
@bhavyaus bhavyaus reopened this Jun 19, 2024
@bhavyaus
Copy link
Contributor
bhavyaus commented Jun 19, 2024

Sorry. The triage bot closed it incorrectly. Will reopen as soon as the fix is in.

@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2024
@bpasero bpasero reopened this Jun 19, 2024
@microsoft microsoft unlocked this conversation Jun 19, 2024
@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2024
@bhavyaus bhavyaus reopened this Jun 19, 2024
@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2024
@bhavyaus bhavyaus reopened this Jun 19, 2024
@mjbvz mjbvz self-assigned this Jun 20, 2024
@mjbvz
Copy link
Collaborator
mjbvz commented Jun 20, 2024

vscode-copilot-chat-code-block is created by the copilot extension. It is cleaned up automatically by the extension host

@jrieken It would be great if we could avoid this. Here's how these documents are used:

  • vscode-chat-code-block — Created by core. Copilot registers itself as the exclusive provider for this scheme
  • vscode-copilot-chat-code-block — Created by copilot. These documents are duplicates of the vscode-chat-code-block ones. Copilot requests IntelliSense on these code blocks and language extensions get these requests.

I'd love to just have the vscode-chat-code-block doc. Maybe we could add re-entry logic so that when an exclusive provider calls a command such as executeHoverProvider inside its provide function, the command does not go back to call the exclusive provider

@mjbvz mjbvz added this to the July 2024 milestone Jun 20, 2024
@jrieken
Copy link
Member
jrieken commented Jun 21, 2024

I would not do any re-entry logic. Granted the extension (chat) does not call open and is just a provider, the duplicated models should only be around for a short time, e.g the execution of a particular language feature like hover,

@mjbvz
Copy link
Collaborator
mjbvz commented Jun 24, 2024

To support IntelliSense across code blocks, we need to make sure extensions have TextDocuments for all of them. Unfortunately that means we can't just have one duplicate model, all of them have to be duplicated

Having to keep duplicates open for every single document is not a good solution but I can't think of any other way to get this working with our current apis

@jrieken
Copy link
Member
jrieken commented Jun 25, 2024

Instead of opening all code blocks from a session as text documents (and fighting to keep them open) could we just register a file system for chat code blocks? With that language extensions can simply read all documents/files at their pace and in their ways? I don't know how this plays with TypeScript specifically but I would assume that's generally the better approach and is also more similar to scenarios like web. So, it would be

  • chat implement readonly fs for code blocks, e.g vscode-chat-fs://<session>/<codeblock_N>
  • chat registers (exclusively) for language features using the actual scheme and redirects to the vscode-chat-fs-uris
  • the renderer loads the initial model
  • extensions use workspace.fs to load sibling documents

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues
Projects
None yet
Development

No branches or pull requests

6 participants