swift-nio icon indicating copy to clipboard operation
swift-nio copied to clipboard

`removeItem` performance

Open pepicrft opened this issue 1 year ago • 1 comments

👋🏼 Hi,

First of all, thanks for this amazing work.

We are using the file-system IO components at Tuist, and I'd like to bring up something that we notice regarding the performance of the NIOFileSystem.FileSystem.removeItem function.

When we run it against a large directory with many nested directories and and files, we noticed the performance is much worse than using FileManager or just rm -rf in a Linux environment. We benchmarked the performance with hyperfine and the results are below.

Looking at the implementation, it seems that we are not parallelising at all, so I was wondering if this is something intentional, or if it's ok to refactor that code to run as much as possible in parallel.

Bechmarks

With NIOFileSystem

Benchmark

  • Time (mean ± σ): 67.939 s ± 4.450 s [User: 61.906 s, System: 3.205 s]
  • Range (min … max): 63.684 s … 77.757 s 10 runs

With Foundation's FileManager

Benchmark:

  • Time (mean ± σ): 5.857 s ± 0.251 s [User: 1.430 s, System: 2.067 s]
  • Range (min … max): 5.419 s … 6.190 s 10 runs

With rm -rf

Benchmark: Time (mean ± σ): 3.150 s ± 0.386 s [User: 0.022 s, System: 1.666 s] Range (min … max): 2.225 s … 3.585 s 10 runs

pepicrft avatar Oct 17 '24 14:10 pepicrft

Thanks for filling this! This is definitely something that we would take a patch for and parallelise the removal similar to how we did for copying in https://github.com/apple/swift-nio/pull/2806.

FranzBusch avatar Oct 17 '24 15:10 FranzBusch

@pepicrft I've raised a PR (#3008) to address the issue of handling removal of items sequentially. I seem to be unable to reproduce your results though. The existing logic in main appears to be performing ok, when compared to rm -rf (albeit slower).

If you still have your test setup, would you mind sharing that, so I can try and reproduce that?

mimischi avatar Nov 29 '24 17:11 mimischi

@pepicrft I've raised a PR (#3008) to address the issue of handling removal of items sequentially. I seem to be unable to reproduce your results though. The existing logic in main appears to be performing ok, when compared to rm -rf (albeit slower).

If you still have your test setup, would you mind sharing that, so I can try and reproduce that?

Thanks a lot, @mimischi 🙏🏼. I unfortunately don't have the setup anymore, but I remember it was a directory containing all the dependencies of a project that had been resolved by the SPM. Perhaps you can try with a project that has many dependencies

pepicrft avatar Dec 04 '24 06:12 pepicrft

I've never followed-up here. I've done a few more tests on my PR, but never increased the number of files/directories above five-digits. The benchmarks showed that the new swift-nio is faster than both rm -rf and the FileManager implementation.

mimischi avatar Dec 16 '24 09:12 mimischi

I've never followed-up here. I've done a few more tests on my PR, but never increased the number of files/directories above five-digits. The benchmarks showed that the new swift-nio is faster than both rm -rf and the FileManager implementation.

This is fantastic news. Thanks a lot @mimischi 🙏🏼

pepicrft avatar Dec 17 '24 13:12 pepicrft