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

Speed up SAF 100x #372

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

Speed up SAF 100x #372

wants to merge 3 commits into from

Conversation

lmagyar
Copy link
Contributor

@lmagyar lmagyar commented Sep 3, 2024

Summary:

  • When there are 1000s of files in a dir, even opening a file takes several minutes!!!
  • The root cause is that creating even 1 SafFile involves a call to DocumentFile.findFile() and it calls listFiles(), and that is extremely slow on a big dir.
  • I've copied the Cursor code from RoSafFileSystemView.getFile() (with some minor improvements).
  • Now opening a file is a fraction of a second on a dir with 1000s of files!!!
  • Though listing a dir is still slow, it uses listFiles(), and we can't do anything about it.

One issue with the code below:

  • When there is an invalid path, like /valid-folder/invalid-folder/valid-folder/file.xxx, what should we do with invalid-folder???
  • We should throw an Exception, but this is not allowed here, we can't return null, it will harm SftpSubsystem.
  • Original code returned a SafFile referencing /valid-folder/invalid-folder and finally a file was created with the name of invalid-folder, still not a good outcome. And without any error message.

Possible better solutions:

  • Extend FileSystemView.getFile() with a throws Exception and modify SftpSubsystem's code? I remember you don't like modifying the original server's code.
  • Create some dummy "fails on everything" SafFile+SafFtpFile+SafSshFile, and that will throw the proper FileNotFoundException on the first call? Maybe this is the most analog to what would happen on a real file-system with File. But it smells like an ugly hack.
  • Any idea???

@lmagyar lmagyar mentioned this pull request Sep 3, 2024
@wolpi
Copy link
Owner

wolpi commented Sep 7, 2024

That sounds quite good 🤩
I've to take a deeper look, also together with the other PRs.

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.

2 participants