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

Improve performance on Linux #79

Closed
lamyergeier opened this issue Jan 25, 2019 · 14 comments · Fixed by #90
Closed

Improve performance on Linux #79

lamyergeier opened this issue Jan 25, 2019 · 14 comments · Fixed by #90
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted

Comments

@lamyergeier
Copy link

lamyergeier commented Jan 25, 2019

Issuehunt badges

$ sed -e '/^+.*/p' a > b
$ time rm b

real    0m0.361s
user    0m0.161s
sys     0m0.027s
$ sed -e '/^+.*/p' a > b
$ time \rm b

real    0m0.002s
user    0m0.000s
sys     0m0.002s
$ type rm
rm is aliased to trash

$ ls -a -l
-rw-rw-r--  1 nikhil 179K Jan 25 11:27 b

IssueHunt Summary

stroncium stroncium has been rewarded.

Backers (Total: $80.00)

Submitted pull Requests


Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@lamyergeier lamyergeier changed the title 10 times slower than /bin/rm More than 150 times slower than /bin/rm Jan 25, 2019
@sindresorhus
Copy link
Owner

What operating system (+ version) and what Node.js version?

@lamyergeier
Copy link
Author

lamyergeier commented Jan 25, 2019

$ node --version
v10.15.0

Ubuntu 18.4

$ trash --version
1.4.0

@sindresorhus
Copy link
Owner

Ok, so there are multiple reasons why it's slower:

  1. It's not doing the same thing as rm. rm just deletes files, trash has to actually move the files to a different location on your hard drive. That's much slower.
  2. rm is created in C and trash is created in Node.js.
  3. While we have tried to make the Linux implementation as fast as possible, I don't use Linux, so it's not a priority.

I'm gonna keep this issue open as "help wanted" to improve performance on Linux.

@lamyergeier
Copy link
Author

May be we could send the process to the background.

@sindresorhus
Copy link
Owner

You can easily do that yourself: alias trash="trash &".

@sindresorhus sindresorhus reopened this Feb 13, 2019
@sindresorhus sindresorhus changed the title More than 150 times slower than /bin/rm Improve performance on Linux May 1, 2019
@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 2, 2019
@IssueHuntBot
Copy link

@IssueHunt has funded $80.00 to this issue.


@stroncium
Copy link
Contributor

From my tests:

  • baseline for deleting one file ~170ms
  • applying some simple optimizations brings it down ~20ms
  • removing meow brings it down ~35ms
  • removing update-notifier brings it down ~25ms

So, all in all, should be easier to achieve 2 times faster time. Also, none of optimizations are related to linux directly, they affect all platforms.

Another thing to consider is that most of the time is spent in long chains of requires, because there is a lot of really small modules involved instead of bigger ones.

@sindresorhus
Copy link
Owner

removing meow brings it down ~35ms

I don't want to do this, but I'm already planning to optimize meow: sindresorhus/meow#67 sindresorhus/meow#104

removing update-notifier brings it down ~25ms

👍


However, this issue was meant to investigate whether there's a way to optimize the actual Linux deletion code.

@stroncium
Copy link
Contributor

@sindresorhus I don't remember the numbers(plus there is some trickery in profiling short-lived processes), but most of the time is spent initializing node and requiring for simple case (trash one-file), to the point where if you use chrome profiler you won't even see where actual work starts unless you zoom quite a lot.

As for more complex scenarios with multiple files and directories, I don't expect there is much to do, except some place at the beginning where I've seen sync code instead of async, but with much work to do further down the line that shouldn't have much impact.

@sindresorhus
Copy link
Owner

Did you try profiling on a directory with many subdirectories and thousands of files? That's a better real-world benchmark and what we should optimize for.

@TiagoDanin
Copy link

TiagoDanin commented May 17, 2019

My debug using console.time in 4.14.52-1-MANJARO, file with ~801Mb.
image

UPDATE: Full log

@stroncium
Copy link
Contributor

I actually have some results, and am preparing to make PR.

@sindresorhus Long story short, the fix includes using procfs. After writing 10th implementation of that stuff and working on optimizing it yet again for the 10th time, I looked around for libraries to access procfs but none of them seem to do such things, only parse some selected areas. So, naturally, I decided to stop writing same code again and again and at the moment I'm close to releasing well optimized library which is able to work with a huge part of procfs including the stuff needed for trash. The library is also optimized for require times, so shouldn't be too heavy to include. Would you mind me using it here(after I release it and you review it if you want)?

@sindresorhus
Copy link
Owner

The library is also optimized for require times, so shouldn't be too heavy to include. Would you mind me using it here(after I release it and you review it if you want)?

Great! More than happy to use it here.

stroncium added a commit to stroncium/trash that referenced this issue Sep 21, 2019
stroncium added a commit to stroncium/trash that referenced this issue Sep 21, 2019
stroncium added a commit to stroncium/trash that referenced this issue Sep 21, 2019
@issuehunt-oss
Copy link

issuehunt-oss bot commented Nov 17, 2019

@sindresorhus has rewarded $72.00 to @stroncium. See it on IssueHunt

  • 💰 Total deposit: $80.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $8.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants