Skip to content

findDocument API (work in progress)#80689

Closed
solomatov wants to merge 1 commit intomicrosoft:masterfrom
solomatov:find-text-document
Closed

findDocument API (work in progress)#80689
solomatov wants to merge 1 commit intomicrosoft:masterfrom
solomatov:find-text-document

Conversation

@solomatov
Copy link
Contributor

This is a request for an API.
What do you think about?

I need this to prevent creating Document Indexes like this: https://github.com/microsoft/vscode/pull/80506/files#diff-1e4a9159b9b98358752ca990d9d2f545

Am I doing it correctly?
How can I test it?

@jrieken
Copy link
Member

jrieken commented Sep 11, 2019

This seems like a little sugar on top of vscode.workspace.textDocuments, e.g you can just use textDocuments.find(doc => doc.uri.toString() === someUri.toString())

@solomatov
Copy link
Contributor Author

@jrieken My understanding that this list can be very long, since once document is opened in an editor, it's kept there. So, if a user keeps opens vs.code for many days, and opens many documents, there might be thousands if not more text documents there.

See for example, ExtHostDocumentsAndEditors._documents, it's a map not a list.

What do you think?

@jrieken
Copy link
Member

jrieken commented Sep 11, 2019

I think it's a fair util but too much boilerplate for having this in the API. Yes, the list can get long but I'd say it rarely more than 100 items. VS Code manages documents and when they aren't being worked on they are closed to free resources.

@solomatov
Copy link
Contributor Author

@jrieken See #50874 This is the issue which referenced PR fixes. MD extension was opening a large number of MD documents, via an API, without loading them to the editor. Even though, they weren't opened in the editor, the were kept in memory and weren't thrown away which led to a performance problem.

Also, TextDocument, has a version field, which once incremented, should continue increasing monotonically, so to be consistent, vs.code can't remove TextDocuments.

What do you think?

@jrieken
Copy link
Member

jrieken commented Sep 11, 2019

and weren't thrown away which led to a performance problem.

Trust me, they will eventually be clean-up and they will be closed. That's why there is onDidCloseTextDocument-event. For documents requested from extensions it's controlled here:

export class BoundModelReferenceCollection {
and other document it's mostly driven by the UI, e.g editors that you see.

@solomatov
Copy link
Contributor Author

@jrieken And what about the version field? Is the version information gets thrown away? For example, in the referenced issue, the existence and monotonicity of increase of the version field is relied upon for cache consistency.

@jrieken
Copy link
Member

jrieken commented Sep 11, 2019

When a document is closed its gone. When the same file is opened again, the version id starts at its default again. That has been the case since ever and that's why it's vital to listen to onDidClose and onDidOpen event or to at least check TextDocument#isClosed.

@solomatov
Copy link
Contributor Author

@jrieken Ok, then this definitely isn't needed. Thanks for the clarifications!

@solomatov solomatov closed this Sep 11, 2019
@solomatov solomatov deleted the find-text-document branch September 11, 2019 16:10
@jrieken
Copy link
Member

jrieken commented Sep 11, 2019

Np. Thanks for getting back so quickly

@solomatov
Copy link
Contributor Author

@jrieken Thanks to you for the same as well)

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants