guard / listen

The Listen gem listens to file modifications and notifies you about the changes.
https://rubygems.org/gems/listen
MIT License
1.93k stars 247 forks source link

fix: Avoid scanning and building entries for silenced directories #542

Closed ElMassimo closed 3 years ago

ElMassimo commented 3 years ago

Description 📖

This pull request patches the Record#build method to avoid recursively scanning and creating entries for directories that are being ignored in the Listener configuration.

Background 📜

Building entries that will be ignored by the listener degrades the initialization performance, and can greatly increase memory consumption.

This is especially noticeable on projects with large .git or node_modules directories.

The Fix 🔨

Use the Silencer configuration to avoid scanning ignored directories.

Benchmarks 📈

The performance difference is significant on startup time.

Using the listen gem in a small Jekyll site:

Before (silenced dirs are scanned)

Record.build(): 0.70397 seconds

After (silenced dirs are ignored)

Record.build(): 0.02439 seconds

Additional Benefits ✨

Directories that are ignored would no longer output symlink warnings since they are no longer being scanned.

ElMassimo commented 3 years ago

@ColinDKelley Hi!

The silencer always uses some defaults, without this patch .git/FETCH_HEAD would show up in the record tree, and cause the test to fail.

Failures:

  1) Listen::Record#build with subdir containing files builds record
     Failure/Error:
       expect(record_tree(record)).
         to eq(
           'dir1' => {},
           'dir1/foo' => { 'bar' => { mtime: 2.3, mode: 0755, size: 42 } },
           'dir2' => {}
         )

       expected: {"dir1"=>{}, "dir1/foo"=>{"bar"=>{:mode=>493, :mtime=>2.3, :size=>42}}, "dir2"=>{}}
            got: {".git"=>{"FETCH_HEAD"=>{:mode=>493, :mtime=>2.3, :size=>42}}, "dir1"=>{}, "dir1/foo"=>{"bar"=>{:mode=>493, :mtime=>2.3, :size=>42}}, "dir2"=>{}}

       (compared using ==)

       Diff:
       @@ -1 +1,2 @@
       +".git" => {"FETCH_HEAD"=>{:mode=>493, :mtime=>2.3, :size=>42}},
        "dir1" => {},

     # ./spec/lib/listen/record_spec.rb:323:in `block (4 levels) in <top (required)>'

Finished in 0.11717 seconds (files took 0.21683 seconds to load)
33 examples, 1 failure

Failed examples:

rspec ./spec/lib/listen/record_spec.rb:321 # Listen::Record#build with subdir containing files builds record
ColinDKelley commented 3 years ago

@ElMassimo Aha! Glad it's being tested. I think it would be an improvement if the test here didn't depend on the defaults of another class. That's an easy change. I'll make it in a PR after I merge this, and tag you.

ColinDKelley commented 3 years ago

BTW, I'm expecting to release this in v3.7.0 in the coming week.