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 overhead when loading directory #771

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

glubsy
Copy link

@glubsy glubsy commented Dec 1, 2020

  • This avoids loading content of parent directories, only adding the
    requested directory.
  • This drastically reduces the amount of fstat system calls, and avoid
    superfluous items in the response array, i.e. files from directories we
    don't necessarily care about.
  • This should reduce timeouts when loading directories with large numbers
    of files.
    The timeout reduction will probably fix #753, fix #629, fix #518, fix #458, fix #752.

* This avoids loading content of parent directories, only adding the
requested directory.
* This drastically reduces the amount of fstat system calls, and avoid
superfluous items in the response array, i.e. files from directories we
don't necessarily care about.
* This should reduce timeouts when loading directories with large numbers
of files.
@hartmark
Copy link

+1 please merge this

@dr2okevin
Copy link

That changed the average loading time for folders from 10 seconds to 30ms. That is a huge improvement.

@sommer-gei
Copy link

@lrsjng Please can you merge this!? It seems a simple fix with a lot performance boost. #ty

@hartmark
Copy link

This repo has been dead for a while. It seems most development has stopped when main developer stopped.

This repo seems to have most of the PRs merged
https://github.com/tudorminator/h5ai

I'm not using h5ai so much any more and plus I don't have so much time to maintain a fork myself.

@sommer-gei
Copy link

sommer-gei commented Sep 14, 2023

This repo has been dead for a while. It seems most development has stopped when main developer stopped.

:-/ Didn't know that … Ty @hartmark for the info!

This repo seems to have most of the PRs merged https://github.com/tudorminator/h5ai

Also ty for this info, I'll look into it.

Update + FYI: I couldn't figure out the explicit Commit, but I can confirm that the patch (from this PR) is implemented (in the new repo). – So ty @glubsy + @tudorminator, too!

KR

@glubsy
Copy link
Author

glubsy commented Sep 16, 2023

Side note: tudorminator's fork seems to have exactly the same commits as my fork currently.

@hartmark
Copy link

+1 vote for @glubsy as new benevolent dictator 😀

@biast12
Copy link

biast12 commented Sep 16, 2023

Hey you two, would maybe recommend you guys to have a look at some of the newer PRs, for an example, my PR for a useBrowserLang fix #863 or this PR #855, and maybe also some of the closed once

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants