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

Revert "reverse virtuallist refactor" #5255

Closed
wants to merge 34 commits into from
Closed

Revert "reverse virtuallist refactor" #5255

wants to merge 34 commits into from

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Jun 17, 2024

pulled out from #5061 (do not merge before)

mifi and others added 4 commits June 13, 2024 13:40
@mifi mifi mentioned this pull request Jun 17, 2024
11 tasks
Copy link
Contributor

Diff output files
No diff

Base automatically changed from google-photos to main June 18, 2024 09:13
@aduh95
Copy link
Member

aduh95 commented Jun 27, 2024

What's the status here Mikael?

@aduh95 aduh95 marked this pull request as draft July 2, 2024 13:37
@lakesare
Copy link
Contributor

lakesare commented Jul 3, 2024

The reason we have performance issues (can reproduce in 4.x too) is because we lost the functionality of VirtualList somewhere along the way. All performance issues disappear when we limit the number of rendered rows to, e.g., 100 rows (everything else stays the same - we process 10k files, etc., only the number of rendered files matters for performance).


Proof

Here is GoogleDrive in 4.x branch, virtualList option is true, and yet 1500 items are all rendered:

Screenshot 2024-07-03 at 17 03 41

Here is what it should look like instead: https://jsfiddle.net/developit/qqan9pdo.


@mifi, would you like to debug our VirtualList implementation while you're refactoring it?

@lakesare lakesare self-requested a review July 3, 2024 12:09
@mifi mifi mentioned this pull request Jul 3, 2024
1 task
@mifi
Copy link
Contributor Author

mifi commented Jul 3, 2024

recreated as #5307 because i dunno what happened here. Also included your TODO @lakesare

@mifi mifi closed this Jul 3, 2024
@aduh95 aduh95 deleted the tsify-virtuallist branch July 5, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants