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.

Sign up for GitHub

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

Jump to bottom

File caching alternative #1992

Merged
merged 19 commits into from Mar 20, 2023
Merged

File caching alternative #1992

merged 19 commits into from Mar 20, 2023

Conversation

harshad1
Copy link
Collaborator

@harshad1 harshad1 commented Mar 5, 2023

In this PR I remove the cached version of File as it could cause problems.

In order to maintain high sort performance, I have added a key based comparison (inspired by how modern python does sorting).

@harshad1
Copy link
Collaborator Author

harshad1 commented Mar 7, 2023

@gsantner This PR is ready for review. Everything seems to be working.

@harshad1 harshad1 changed the title WIP: File caching alternative File caching alternative Mar 10, 2023
gsantner
gsantner reviewed Mar 12, 2023
View reviewed changes
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated
@@ -0,0 +1,122 @@
package net.gsantner.opoc.util;
Copy link
Owner

Choose a reason for hiding this comment

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

Like the other opoc files, please add the public domain/cc0 header

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I copied one and changed the name. Please modify as desired.

app/src/main/java/net/gsantner/opoc/util/GsUtils.java Outdated Show resolved Hide resolved
app/src/main/java/net/gsantner/opoc/util/GsFileUtils.java Outdated Show resolved Hide resolved
@gsantner gsantner added this to the Markor v2.11 milestone Mar 17, 2023
@gsantner
Copy link
Owner

gsantner commented Mar 17, 2023 •
edited

Sorry to say, but with the changes sorting often works not, or even the folder is not loaded.

What I have found:

  • 100% always when I start the app fresh, sorting isn't applied at all seemingly (folders are not at top and i.e. QuickNote which should be alphabetically low, is on top).
  • When switching sort order it somehow happens to properly sort
  • when closing the app and coming back to the filemanager, the folder doesn't load at all. even when pulling down to refresh, or when opening folder again via top menu
screenrecord-2023-03-17_19.33.54.webm

screenshot-2023-03-17__19-31-25

Overall I think it's a quite different approach and it has potential to fail miserably, but on the other side can improve search sort/comparision performance a lot (when compared to fetching file details directly and making disk i/o for every single bit of info of a file)

@harshad1
Copy link
Collaborator Author

Hmmm, I am not able to reproduce any of this and it has been working consistently on my phone for quite a while now.

Investigating

@gsantner
Copy link
Owner

Can I give some info thats helpful? I can also look more into it if I find some time on weekend

@harshad1
Copy link
Collaborator Author

  • Please confirm that you are using the latest version.
  • What device / OS version are you using?
  • Please check if you have any stack traces / exceptions in the logs

@harshad1
Copy link
Collaborator Author

Please also try the version I just pushed up.

@harshad1
Copy link
Collaborator Author

Pushing up another test where I use list keys instead of padded strings. All of these approaches work well for me. Just trying different things.

If you could debug and see what is happening to you then that would be great too.

gsantner
gsantner approved these changes Mar 20, 2023
View reviewed changes
Copy link
Owner

@gsantner gsantner left a comment

Choose a reason for hiding this comment

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

Worked better now after the changes!

Thank you, merging it 👍🏻 🎉 !

@gsantner gsantner merged commit 946eafe into gsantner:master Mar 20, 2023
@harshad1 harshad1 deleted the file_caching_alternative branch October 10, 2023 05:04
@gsantner
Copy link
Owner

gsantner commented Dec 3, 2023 •
edited

@harshad1
sorting by filesize seems to not work properly currently. I assume it's related to the key-string-sorting changes. Can you reproduce that too?

@gsantner gsantner linked an issue Jan 11, 2024 that may be closed by this pull request
FileBrowser: File Modification Time not updated until app restart #2196
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsantner gsantner gsantner approved these changes

Assignees

@harshad1 harshad1

Labels
enhancement
Projects
None yet
Milestone
Markor v2.11
Development

Successfully merging this pull request may close these issues.

FileBrowser: File Modification Time not updated until app restart
2 participants
@harshad1 @gsantner