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

Fix duplicate files and other recent regressions #2191

Merged
merged 9 commits into from
May 10, 2023
Merged

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented May 7, 2023

Fixes #2190
Fixes #2189
Fixes #2188

This PR "completes" the faulty commit responsible. The file-rowreference map was not being updated in all relevant places .

The map must be updated whenever a file is added or removed from the model, not just on loading.

NOTE: This PR and the previous one only speed up construction/destruction of the model. The most dramatic speed increase is seen when the Directory is cached and does not have to be reloaded from disk. Loading/sorting a large directory for the first time can still take several seconds (exponent >1 and <2) and there is a noticeable lag when navigating away.

On a modestly powered machine folders containing several thousand item are acceptably responsive, but tens of thousand items still result in significant delays (with no user feedback).

The GTK4 port is already much faster so probably not worth spending too much time improving the Gtk3 version?

@jeremypw jeremypw marked this pull request as draft May 7, 2023 10:53
@peteruithoven
Copy link
Collaborator

Great! This seems to at least fix:

Not sure about #2190. I noticed that with the master branch it duplicated files created outside of Files. With this PR that no longer happens.

@jeremypw jeremypw changed the title Fix duplicate files Fix duplicate files and other recent regressions May 8, 2023
@jeremypw jeremypw marked this pull request as ready for review May 8, 2023 09:32
@jeremypw jeremypw added the Priority: Critical Causes Files to crash, hang or otherwise become unusable label May 9, 2023
@ryonakano
Copy link
Contributor

I can't confirm #2190 at all although I browsed a folder with 100 files (which I created by for i in {1..100}; do touch test$i; done;) so not sure if this branch fixes it, but can confirmed this branch fixes the others.

libcore/ListModel.vala Outdated Show resolved Hide resolved
libcore/ListModel.vala Outdated Show resolved Hide resolved
@jeremypw
Copy link
Collaborator Author

I can't confirm #2190 at all although I browsed a folder with 100 files (which I created by for i in {1..100}; do touch test$i; done;) so not sure if this branch fixes it, but can confirmed this branch fixes the others.

I observed this issue on a slow computer - I have now tried to reproduce on a more modern faster computer without success so must be a race of some kind.

Jeremy Wootten and others added 2 commits May 10, 2023 09:55
Co-authored-by: Ryo Nakano <ryonakaknock3@gmail.com>
@jeremypw
Copy link
Collaborator Author

@ryonakano Thanks for the review and suggestions, which I have now addressed.

Copy link
Contributor

@ryonakano ryonakano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me and works confirmed, thank you for your fix!

@jeremypw jeremypw merged commit 9315942 into master May 10, 2023
@jeremypw jeremypw deleted the fix-duplicate-files branch May 10, 2023 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Critical Causes Files to crash, hang or otherwise become unusable
Projects
None yet
3 participants