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

feat/list-virtualizer: List Virtualizer #4873

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

alexevladgabriel
Copy link

This PR add support for files length over 250 to be rendered.
I'm looking to implement Infinite Scroll pattern for more optimized rendering. (thinking for React Query)

@alexevladgabriel
Copy link
Author

Preview:

2023-09-28.11-24-03.mp4

@devnote-dev
Copy link
Contributor

Wouldn't this still be a potential bottleneck on certain browsers? If so then I'd rather this be paginated from the API then propagated to the client UI.

@alexevladgabriel
Copy link
Author

alexevladgabriel commented Sep 29, 2023

@devnote-dev I will think more on it, there was a discussion about pagination on files explorer. Thinking about infinite scroll for files explorer.

@matthewpi
Copy link
Member

Wouldn't this still be a potential bottleneck on certain browsers? If so then I'd rather this be paginated from the API then propagated to the client UI.

Potentially but the OS calls all the way from Wings don't have a concept of pagination, and the OS calls return unsorted results. So if we did an infinite scroll like that we would be spamming syscalls on the Wings instance to just reduce the payload sent to the client. The main performance concern is rendering 1000s of DOM nodes at once, not if the JSON payload is measured in 10s or 100s of MBs.

@devnote-dev
Copy link
Contributor

There's no reason why the panel can't cache and paginate the response from Wings which solves both issues.

@matthewpi
Copy link
Member

There's no reason why the panel can't cache and paginate the response from Wings which solves both issues.

Except with how fast a cache becomes invalidated when dealing with file listings. The panel doesn't receive live update events meaning any cache would need to be invalidated within seconds to prevent it from becoming stale. It also puts a lot of extra strain on the cache as file listings are high cardinality and the size to store them is relatively large compared to every other cache value.

While it would be easy to determine if a file listing is large enough to deserve a cache, it's not easy to determine whether a request should hit the cache or not, meaning every request to list a directories contents would need to hit the cache, adding extra latency and load on the cache, just to likely end up hitting Wings.

Most people also don't have Redis configured with LRU (though that would also break sessions), meaning unless we added a job to prune old cache entries, the cache would get filled up very quickly.

--

We already load all the file entries into the browser just fine, we just skip rendering all of them to prevent the browser from exploding when it goes to render the DOM.

@devnote-dev
Copy link
Contributor

Perhaps this should be a separate issue, but for now I think that if the current system is to change then it should involve some degree of caching or pagination from Wings, or even a combination of both.

@alexevladgabriel
Copy link
Author

alexevladgabriel commented Oct 3, 2023

Wouldn't this still be a potential bottleneck on certain browsers? If so then I'd rather this be paginated from the API then propagated to the client UI.

Potentially but the OS calls all the way from Wings don't have a concept of pagination, and the OS calls return unsorted results. So if we did an infinite scroll like that we would be spamming syscalls on the Wings instance to just reduce the payload sent to the client. The main performance concern is rendering 1000s of DOM nodes at once, not if the JSON payload is measured in 10s or 100s of MBs.

We can't do a pagination simulation on data received from the wings and just do the on client infinite scrolling on that pagination to solve the issue of rendering?

That way doesn't require internal calls to wings and caching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

None yet

4 participants