posit-dev / publisher

MIT License
6 stars 1 forks source link

`TreeProjectFiles` hangs sidebar when a deployment has too many folders #2204

Open mmarchetti opened 3 months ago

mmarchetti commented 3 months ago

When a deployment is selected that has too many folders to display in the Project Files view our recursive TreeProjectFiles component hangs the sidebar while attempting to render.

A project with 1000 files and nested folders works with no issue and a imperceivable delay (0.037568s). However a project with over 65,000 files and several nested folders hangs when the deployment is selected.

dotNomad commented 1 month ago

This still occurs, but we have done some things to make it better.

The home view shows a loading bar (most of the time). It doesn't show one while we wait for the files API to return since that is in response to file watchers. If we made that show a loading bar it would show on every file change / deletion / addition which probably isn't the right call.

I was able to reproduce this, and I completely hung the extension sidebar, attempting to publish a index.html in a large web project. Our files API attempted to follow node module symlinks, and all that fun stuff.

Of course this is a contrived example since we don't expect that sort of content, but for the sake of recreating it...

jonkeane commented 1 month ago

Of course this is a contrived example since we don't expect that sort of content, but for the sake of recreating it...

This honestly doesn't sound super contrived. Publishing static HTML isn't a core differentiator of connect, but it is a feature people use, and lots of nested files isn't unconceivable

dotNomad commented 1 month ago

This honestly doesn't sound super contrived. Publishing static HTML isn't a core differentiator of connect, but it is a feature people use, and lots of nested files isn't unconceivable

If having similar numbers of folders/files to node_modules in a web app is something we are expecting, then this is definitely an issue we should solve.

dotNomad commented 2 weeks ago

Putting some comments here as I do some digging on this issue:

The extension only hangs when a deployment is created or selected that has enough files to cause a hang. The sub-directory feature works great if the workspace has a ton of files, but if the entrypoint has enough sibling/children folder/files we get a hang.

Every other feature appears to be un-affected.

For reference a project with about 1000 items takes 0.037568s to call GET /api/files and works great in the sidebar.

A project with about 60,000 items takes 2.390257791s to call GET /api/files and hangs the sidebar even past the networking call.

The 2.39s delay on the files API isn't enough to cause the hang I'm seeing so I'm going to investigate that further.

dotNomad commented 2 weeks ago

I was able to isolate the issue to our recursive TreeProjectFiles component. With a sizeable amount of folders the render time causes the hang we are seeing.

I will update the title and description issue to reflect my new findings since the original context of the issue is no longer valid.

dotNomad commented 2 weeks ago

Quarto websites with a lot of extensions may hit this issue because of the amount of files in the lib directory.

We could also prevent the renv/library from being expandable to prevent some of the more common cases of this occurring prior to us fixing the issue.

dotNomad commented 2 weeks ago

After further investigation the hang can occur as low as 500 files.

It seems to be related to how many files are displayed at once, how nested the files are, and other factors. Certain projects don't have problems around until around 1,500 files, where a very flat structure causes this issue earlier in the file count.

Based on the benchmark rather than band-aiding this issue with some quick fixes we've decided to address it with virtualizing the Project Files tree items.


To accomplish this we can look at libraries for list virtualization. Some are listed in the Vue docs: https://vuejs.org/guide/best-practices/performance#virtualize-large-lists

We will need to refactor TreeProjectFiles to be flat rather than have nested DOM elements. This is because virtualization (for example with vue-virtual-scroller's Recycle Scroller) relies on the elements being fixed in size[^1].


One additional note, all of the Tree Items must be present in the DOM as opposed to hiding the collapsed items. This is because we do not have separate state managing what files are included or excluded. We instead rely on hearing back from our Files API to give us the state.

We specifically use v-show instead of v-if in the TreeItemCheckbox because of this: https://github.com/posit-dev/publisher/blob/6fe68b986105d6a4380185bb327c0de049e53d2b/extensions/vscode/webviews/homeView/src/components/tree/TreeItemCheckbox.vue#L49

We do this to avoid needing to sync the state of the frontend with the backend, instead relying entirely on the response from our API.

If we re-rendered components our checkbox states could be displayed incorrectly as we wait to hear back from the Files API.

[^1]: Vue Virtual Scroller Recycle Scroller notes