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

Performance issues on Windows #73

Closed
alexbsys opened this issue Sep 24, 2020 · 6 comments
Closed

Performance issues on Windows #73

alexbsys opened this issue Sep 24, 2020 · 6 comments
Assignees
Labels
available on master Fix is done on master branch, issue closed on next release enhancement New feature or request Windows Windows platform is affected
Milestone

Comments

@alexbsys
Copy link

alexbsys commented Sep 24, 2020

ghc::filesystem is slower than std::filesystem on Windows when working with directories that contain many files.
I tested it in directory which contains about 16600 files.
Enumerating files via ghc::filesystem takes about 50-60 seconds on my PC (NTFS, HDD).
Enumerating files via std::filesystem used from example code takes 2.5-5 seconds (same directory).

  fs::path object_path = fs::u8path(start_path);

  for (auto &entry_path : fs::directory_iterator(object_path)) {
    fs::file_status entry_status = fs::status(entry_path);
    // some simple operations like push names to list
  }

I think the problem corresponds to non-optimized iterator operators and 'status' operations.
Please look on call stack on image. Simple iterator increment operation requires call for 'status', which processed all data gathering, strings assigning, etc. So, for single pass for one file, 'status' operation called so many times with gathering a lot of not necessary information.

image

Maybe caching of file names, extensions, root paths, etc. instead calculation on each call, will make library faster.

@gulrak gulrak added enhancement New feature or request Windows Windows platform is affected labels Sep 26, 2020
@gulrak gulrak self-assigned this Sep 26, 2020
@gulrak
Copy link
Owner

gulrak commented Sep 26, 2020

Yeah, that is more than I expected. There is an overhead in ghc::filesystem::path on Windows because of it using the generic representation as internal representation instead of the native one, but I guess this might be more the result of the directory_iterator / directory_entry workings.

I'll do some tests and try to optimize it. I'm quite busy currently, but I plan to work on this and #70, the other Windows issue the next days, hopefully tomorrow.

@gulrak gulrak added this to the v1.3.6 milestone Sep 26, 2020
@gulrak
Copy link
Owner

gulrak commented Sep 26, 2020

I found at least one of the reasons going through the source with the call stack as hint, the evaluation of the permissions "executable" part is quite expensive and most of the time permissions of a directory_entry are not used.

I think I'll defer the X-Part of the flags until permissions are actually used, and additionally try to optimize their evaluation.

I'm on a trip right now (browsing code on the phone), but I think a test of that should be on a branch sometime tomorrow. Not sure about the impact yet.

@gulrak
Copy link
Owner

gulrak commented Sep 28, 2020

Sorry, there is some delay with the availability of a branch on this.

My test was with recursive_directory_iterator as I had no large enough single directory, and there where differences between the results of iterating a huge tree with ghc::filesystem and MS std::filesystem in the number of regular files and the sum of their sizes, so I took quite some time to analyze this to find a possible hidden bug, and it seems I found an issue in the std::filesystem implementation that I'm going to report over there.

I hope to give it another shot after work today, but I wanted to report back why there is no branch yet.

@alexbsys
Copy link
Author

ok, thanks for information! I'm ready for test

gulrak added a commit that referenced this issue Sep 30, 2020
@gulrak
Copy link
Owner

gulrak commented Oct 1, 2020

My work is on branch: feature-73-performance-optimization

Okay, let me start by saying, I was not able to reproduce the huge gap from your measurements. My tests where on my Dev-Laptop, with an SSD (I have no Windows system with HDD available), and I'm using VisualStudio 2019, 16.7.4 and my baseline measurements are:

Recursive iteration over C:\Windows (nested directories, 324681 entries, 247597 files) with additional fs::status() call, as in the given code:

Implementation Time Relative
std::filesystem 47.2s 100%
ghc::filesystem 51.5s +9%
ghc::filesystem with reduced path overhead 49.7s +5%

Not that impressive, still useful, with the difference of internal implementation in mind. I want to point out, that the additional fs::status(entry_path) call is the mail culprit of time taken. The fs::directory_entry from the iterator already has a status, so using fs::file_status entry_status = entry_path.status() inside the loop instead leads to:

Implementation Time Relative
std::filesystem 10.5s 100%
ghc::filesystem 13.1s +25%
ghc::filesystem with reduced path overhead 11.8s +12%

So my optimizations halved the overhead of the different internal representations, I'm happy with that.

I then also created a single test directory with 20k files, to have a comparison without the impact of many directory_iterator creations during the work of the recursive_directory_iterator used on the C:\Windows folder:

Implementation Time
std::filesystem with fs::status(entry_path) 1480ms
ghc::filesystem with fs::status(entry_path) 1570ms
ghc::filesystem with fs::status(entry_path) and path optimizations 1500ms
std::filesystem with entry_path.status() 47ms
ghc::filesystem with entry_path.status() 66ms
ghc::filesystem with entry_path.status() and path optimizations 44ms

So in the best case, no additional status call, just flat iteration, the optimizations make it faster than std::filesystem on average. I guess there is some non-optimal code in there as well, as it should be faster with native storage of the path.

I hope this helps your performance issues as well, even if I couldn't replicates your numbers.

@gulrak gulrak added the available on master Fix is done on master branch, issue closed on next release label Oct 4, 2020
@gulrak
Copy link
Owner

gulrak commented Oct 10, 2020

Released with v1.3.6

@gulrak gulrak closed this as completed Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on master Fix is done on master branch, issue closed on next release enhancement New feature or request Windows Windows platform is affected
Projects
None yet
Development

No branches or pull requests

2 participants