Skip to content

Finalise filesystem_watchers #1606

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

Closed
2 tasks done
alex-courtis opened this issue Sep 26, 2022 · 12 comments
Closed
2 tasks done

Finalise filesystem_watchers #1606

alex-courtis opened this issue Sep 26, 2022 · 12 comments

Comments

@alex-courtis
Copy link
Member

alex-courtis commented Sep 26, 2022

Remove any codepaths that are conditional on either of these options.

Silently remove these options.

Update: there are genuine use cases for disabled watchers. Resolve remaining issues such as large directories.

Tasks:

  • add filesystem_watchers.ignore
  • tear down watcher on failure, notifying the user once
@alex-courtis alex-courtis changed the title chore: always use filesystem_watchers watchers, remove auto_reload_on_write chore: always use filesystem_watchers, remove auto_reload_on_write Sep 26, 2022
@yehuohan
Copy link

filesystem_watchers is a great feature for nvim-tree, and I think there is a case will have a better performance to keep the option filesystem_watchers with false value .

The case is that executing :!make command to build a large c/c++ project with nvim-tree opened, where a lot of intermedia files will be created that will result large memory consumption to nvim.exe.
I caught the case on windows when, and get the memory consumption from task manager.

@alex-courtis
Copy link
Member Author

That is a good observation. No watchers may be a valid use case. This change should be reconsidered.

Before we reject I would be grateful if you could run a performance test. I don't have a windows environment, however it would be very valuable to see what is happening; there may be problems with the watchers that can be fixed.

Please enable profiling, run your large make and post the logs.

@gegoune
Copy link
Collaborator

gegoune commented Oct 22, 2022

Couldn't certain files be ignored by watchers?

@alex-courtis
Copy link
Member Author

alex-courtis commented Oct 22, 2022

Couldn't certain files be ignored by watchers?

That would be a great solution.

Watchers only know about the directory and don't see individual file changes, as they don't get the full inotify event. However... ignoring directories would work very nicely.

We are (hardcoded) ignoring .git subdirectories now and could add regex directories. Use case of meson build directory comes to mind.

I'll be interested to see the results of @yehuohan profiling: perhaps there are some known bad or problematic directories on windows that we could document or defualt.

See also #1670

@yehuohan
Copy link

yehuohan commented Oct 22, 2022

@alex-courtis

I tested with building neovim 0.8 branch from msys2 with following commands saved into a sh file:

mkdir -p build
cd build
cmake -DCMAKE_INSTALL_PREFIX=E:/nvim/neovim/install -G Ninja ..
ninja
ninja install
cd ..

And run nvim-qt (nvim 0.8) with mini nvim config:

let $DotVimDir=resolve(expand('<sfile>:p:h'))
set rtp^=$DotVimDir
call plug#begin($DotVimDir.'/bundle')
    Plug 'kyazdani42/nvim-tree.lua'
call plug#end()

lua << EOF
    local tcb = require('nvim-tree.config').nvim_tree_callback
    require('nvim-tree').setup{
        auto_reload_on_write = false,
        log = {
            enable = true,
            truncate = true,
            types = {
                git = false,
                profile = true,
            },
        },
        filesystem_watchers = { enable = true },
        diagnostics = { enable = false },
        git = { enable = false },
    }
EOF
  • Build neovim from :terminal without opening nvim-tree

The memory consumption of nvim.exe is ok.

without-nt

The detailed testing screen recording:

without-nt.mp4
  • Build neovim from :termianl with opening nvim-tree

The memory consumption of nvim.exe is very large. (Finally I had to kill nvim-qt forcely from task manager for nvim-qt had no response)

with-nt

with-nt.mp4

The attached nvim-tree.log:

[2022-10-22 14:35:32] [profile] START draw
[2022-10-22 14:35:32] [profile] END   draw  2ms
[2022-10-22 14:35:41] [profile] START draw
[2022-10-22 14:35:41] [profile] END   draw  1ms
[2022-10-22 14:35:41] [profile] START draw
[2022-10-22 14:35:41] [profile] END   draw  2ms
[2022-10-22 14:35:41] [profile] START draw
[2022-10-22 14:35:41] [profile] END   draw  2ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  2ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  2ms
[2022-10-22 14:35:42] [profile] START draw
[2022-10-22 14:35:42] [profile] END   draw  1ms
[2022-10-22 14:35:43] [profile] START draw
[2022-10-22 14:35:43] [profile] END   draw  1ms
[2022-10-22 14:35:43] [profile] START draw
[2022-10-22 14:35:43] [profile] END   draw  1ms
[2022-10-22 14:35:43] [profile] START draw
[2022-10-22 14:35:43] [profile] END   draw  1ms
[2022-10-22 14:35:54] [profile] START draw
[2022-10-22 14:35:54] [profile] END   draw  1ms
[2022-10-22 14:35:55] [profile] START draw
[2022-10-22 14:35:55] [profile] END   draw  1ms
[2022-10-22 14:38:53] [profile] START draw
[2022-10-22 14:38:53] [profile] END   draw  8ms
[2022-10-22 14:38:54] [profile] START draw
[2022-10-22 14:38:54] [profile] END   draw  1ms
[2022-10-22 14:39:36] [profile] START draw
[2022-10-22 14:39:36] [profile] END   draw  38ms

By the way, it seemed running build commands with ! will block nvim-tree and it's hard to reproduce the case with !. So I tested with :terminal where can reproduce every time. (Actually the first time I caught the memory consumption case is running build command with a job runner plugin)

@alex-courtis
Copy link
Member Author

alex-courtis commented Oct 22, 2022

Thanks for the detailed demonstration. The problem is demonstrated and it's very useful to see that it is a meson style build.

Unforntunately there's nothing useful in the log; I apologise for not specifying the full settings, which I will update in the readme now.

Please can you re-run with:

    log = {
      enable = true,
      truncate = true,
      types = {
        all = false,
        config = false,
        copy_paste = false,
        dev = false,
        diagnostics = false,
        git = true,
        profile = true,
        watcher = true,
      },

I am hopeful that directory ignoring will resolve this.

@alex-courtis alex-courtis changed the title chore: always use filesystem_watchers, remove auto_reload_on_write finalise filesystem_watchers Oct 23, 2022
@alex-courtis alex-courtis changed the title finalise filesystem_watchers Finalise filesystem_watchers Oct 23, 2022
@yehuohan
Copy link

@alex-courtis I tested with the log setting you provide. The nvim-tree.log is attached below and is about 140Mb after unzip.
The nvim memory consumption is large still. (Please let me know if you need the screen record)

nvim-tree.zip

@alex-courtis
Copy link
Member Author

alex-courtis commented Oct 24, 2022

Many thanks, that is incredibly useful.

Story

It's clear what's going on now: nvim-tree (implies nvim) doesn't have permissions to see these directories:

:; cat nvim-tree.log | grep EPERM | sed -e "s/.*event_cb for //g" | sort | uniq -c
 629879 E:\nvim\neovim\build\CMakeFiles\CMakeTmp\CMakeFiles\cmTC_44004.dir fail : EPERM
 651604 E:\nvim\neovim\build\CMakeFiles\CMakeTmp\CMakeFiles fail : EPERM

Can nvim open files in that directory?

The permissions are something you might want to fix. Unfortunately I am not familiar with windows so I cannot offer any advice...

Runtime Fix

Failures such as these must be handled.

Proposal:

  1. notify the user once upon the first failure
  2. tear down the watcher on any such failure

Configuration Fix

I examined my workflow on a meson/ninja project: I have build in my .gitignore hence it is not watched. Sometimes I will look at build via I and will see updates. This will not suit all users. In this case, git is disabled and .gitignore will not be used.

Proposal:
Allow directories to be ignored as per filters.custom: filesystem_watchers.ignore.

@alex-courtis
Copy link
Member Author

@yehuohan you could try this workaround, similar to my workflow:

  1. Add CMakeFiles to your .gitignore
  2. Set git.enable

1 might need some changes, perhaps build, as I'm not sure where CMakeFiles is located relative to your repository root.

@yehuohan
Copy link

@alex-courtis Sorry for late reply.

I confirmed that nvim can access files under E:\nvim\neovim\build\CMakeFiles\CMakeTmp\CMakeFiles. But files under E:\nvim\neovim\build\CMakeFiles\CMakeTmp\CMakeFiles changed(create and delete) very quickly when running cmake generating command, and I have to try to open the files after breaking the cmake generating process. And also I confirmed that the large memory consumption was resulted only from cmake generating process, but not from cmake building process.

I also tested with git.enable = true and CMakeFiles appened into .gitignore. Yes, there is no large memory consumption any more.

@alex-courtis
Copy link
Member Author

And also I confirmed that the large memory consumption was resulted only from cmake generating process, but not from cmake building process.

It may be that there are many permission changes being made during that process, with some files being temporarily unreadable. Some aspect of the windows filesystem is likely the problem.

I also tested with git.enable = true and CMakeFiles appened into .gitignore. Yes, there is no large memory consumption any more.

We have proven the concept. Using .gitignore is no a solution that can apply to all cases. filesystem_watchers.ignore is thus necessary.

Many thanks @yehuohan for going through this process. We now have all the information and can build a complete solution.

@alex-courtis
Copy link
Member Author

PR for ignore: #1705

@yehuohan I would be very grateful if you could test: #1705 (comment)

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

Successfully merging a pull request may close this issue.

3 participants