Tune File Notify Watchers to share directory file descriptors #1228

thejeffphil · 2023-06-01T20:23:25Z

The same file descriptors will be shared across same server watchers.
When watcher cap unregs, only watcher unique descriptors are rm'd.
When directory created event, add a new watcher for it.
Send eglot--error to eglot-message, like eglot--warn aleady does.
Discussion: Eglot not working with Pyright on Mac OSX because of excessive file watchers #1226

Note: Eglot is part of Emacs, this PR is just for discussion purposes on #1226. And if discussion goes well, I will complete necessary required paperwork.

* The same file descriptors will be shared across same server watchers. * When watcher cap unregs, only watcher unique descriptors are rm'd. * When directory created event, add a new watcher for it. * Send eglot--error to eglot-message, like eglot--warn aleady does. * Discussion: #1226

joaotavora · 2023-06-01T20:29:00Z

This is very interesting, but please explain again what current wasteful behaviour is with a toy example, so that I can understand it.

Then explain how your implementation solves this problem.

jeff-phil · 2023-06-05T22:27:07Z

Added the "toy example" testing items for demonstration to setup a overly large python virtual environment. Then will load up a python file with eglot, and after each call of eglot-register-capability will output the number of file-notify-descriptors and memory.

Pull the branch: tune-file-watchers
Change to PR-tests-do_not_merge directory
Run the test-python.sh:

First time this will create venv and pull lots of packages
Will run emacs -Q --batch -l init.el
Prompt if want to run with base installed eglot.el, or override with the one in branch

With pyright didChangeWatchedFiles will be requested twice from server, notice the second run doubles file descriptors and adds 15-20MB memory. Loading the patch, file descriptors are not increased due to sharing of descriptors.

Here is how it outputs for me, with first run using normal eglot.el, and second run using updated.

$ ./test-python.sh
Loading project (native compiled elisp)...
Loading eldoc (native compiled elisp)...
Loading seq (native compiled elisp)...
Loading flymake (native compiled elisp)...
Loading xref (native compiled elisp)...
Loading jsonrpc (native compiled elisp)...
Loading external-completion (native compiled elisp)...
Baseline stats...
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
******************************************
Total count file-notify-descriptors: 0
Total MB of memory (rss) Emacs is using: 96.06 MB
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
******************************************
Python Mode stats...
Do you want to load the "override" eglot.el vs. installed package? (y or n) n
[eglot] Connected! Server `EGLOT (python/(python-mode))' now managing `(python-mode)' buffers in project `python'.
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
******************************************
Total count file-notify-descriptors: 3742
Total MB of memory (rss) Emacs is using: 125.12 MB
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
******************************************
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
******************************************
Total count file-notify-descriptors: 7484
Total MB of memory (rss) Emacs is using: 154.38 MB
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
******************************************

$ ./test-python.sh
Loading project (native compiled elisp)...
Loading eldoc (native compiled elisp)...
Loading seq (native compiled elisp)...
Loading flymake (native compiled elisp)...
Loading xref (native compiled elisp)...
Loading jsonrpc (native compiled elisp)...
Loading external-completion (native compiled elisp)...
Baseline stats...
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
******************************************
Total count file-notify-descriptors: 0
Total MB of memory (rss) Emacs is using: 96.22 MB
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
******************************************
Python Mode stats...
Do you want to load the "override" eglot.el vs. installed package? (y or n) y
Loading ../eglot.el (source)...
Loading project (native compiled elisp)...
Loading eldoc (native compiled elisp)...
Loading seq (native compiled elisp)...
Loading flymake (native compiled elisp)...
Loading xref (native compiled elisp)...
Loading jsonrpc (native compiled elisp)...
Loading external-completion (native compiled elisp)...
[eglot] Connected! Server `EGLOT (python/(python-mode))' now managing `(python-mode)' buffers in project `python'.
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
******************************************
Total count file-notify-descriptors: 3742
Total MB of memory (rss) Emacs is using: 127.88 MB
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
******************************************
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
******************************************
Total count file-notify-descriptors: 3742
Total MB of memory (rss) Emacs is using: 140.72 MB
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
******************************************

I have included rust and js LSP's in the update, just for extra info. Rust LSP also sends the didChangeWatchedFiles request twice, but the id is the same which means the file descriptors are not duped. The typescript-language-server does not send the didChangeWatchedFiles more than once. I could not find any other Python LSP's that implemented file watchers to compare there.

Anyway, just wanted to finalize this for you since you requested and so you have it.

joaotavora · 2023-06-06T09:34:40Z

Sorry, this is too complicated for me to run and decipher. I didn't explain myself, a toy example as I envisioned it couldstart from the MRE (the bit that contains an Emacs -Q call) that was eventually supplied in #1226, and then teach me how I can see in an Eglot events log that the server is over-watching files.

It might not me necessary at this point, as I think I have understood the problem sufficiently well,

But it might still be a nice-to-have. So we could start with this.

mkdir mre
cd mre
pip3 install pyright
touch silly-file.txt
emacs test.py -Q -f toggle-debug-on-error -f eglot
# Now explain to me in English what Emacs actions 
# (say switching to Eglot events buffer and  searching 
# for specific bits of text) to take  # to observe that Eglot is 
# behaving suboptimally.
# 
# You can use minimal Emacs lisp code to express your 
# intentions, or add debug code, like advising eglot-register-capability 
# but if it's anything more than 10 lines I probably
# won't find the time to test it.

jeff-phil · 2023-06-06T19:09:38Z

Oops, I definitely over thought toy example.

This is probably as tight as I can get it.

mkdir mre
cd mre
python3 -m venv .venv
source .venv/bin/activate 
pip3 install pyright numpy numpydoc
touch test.py
##
emacs -Q

Once inside of emacs, paste this into scratch and eval-buffer:

(require 'package)
(package-initialize)
(let ((default-directory package-user-dir))
  (normal-top-level-add-subdirs-to-load-path))
(require 'eglot)
(add-to-list 'eglot-server-programs '(python-mode . ("pyright-langserver" "--stdio")))
(advice-add 'eglot-register-capability :after #'(lambda(&rest args)(message "Total count file-notify-descriptors: %s" (hash-table-count file-notify-descriptors))))
(find-file "test.py")
(call-interactively 'eglot)
(switch-to-buffer (messages-buffer))

In Messages buffer, you should see something like:

Total count file-notify-descriptors: 654
Total count file-notify-descriptors: 1308

When that number doubles, that means the file notifiers are duplicated.

You can then load the eglot.el from this PR, and restart eglot and you should just see this:

Total count file-notify-descriptors: 654 [2 times]

which means the file notifiers were not doubled.

joaotavora · 2023-06-06T20:28:35Z

Thanks, this is good and a good test. In fact, it would be easy to make this an automated test

joaotavora · 2023-06-08T14:11:42Z

I've now pushed changes to the same effect to Emacs master, so we should get these enhancements in the next Eglot release:

GitHub-reference: joaotavora/eglot#1228 GitHub-reference: joaotavora/eglot#1226 The Pyright language server issues very heavy file watching requests, which sometimes exceed the OS limit. Most of these file watches are useless, but Pyright insists on issuing them. What's more, for some (absurd?) reason, Pyright issues two file watching requests for the _same_ directories, only to then almost immediately ask to undo the effects of one of these requests. This change to Eglot makes it so that if a single server requests to watch a specific directory twice, only one file watch object is used. Suggested by: https://github.com/thejeffphil * lisp/progmodes/eglot.el (eglot-lsp-server): Change structure of file-watches field. (eglot--on-shutdown): Adapt to new structure. (eglot-register-capability): Rework. (eglot-unregister-capability): Rework. * etc/EGLOT-NEWS: Mention change

GitHub-reference: joaotavora/eglot#1228 GitHub-reference: joaotavora/eglot#1226 * lisp/progmodes/eglot.el (eglot-register-capability): Rework. Suggested by: https://github.com/thejeffphil

Include \'toy example\' for evaluating impact.

87c0bd3

joaotavora closed this Jun 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tune File Notify Watchers to share directory file descriptors #1228

Tune File Notify Watchers to share directory file descriptors #1228

thejeffphil commented Jun 1, 2023

joaotavora commented Jun 1, 2023

jeff-phil commented Jun 5, 2023

joaotavora commented Jun 6, 2023

jeff-phil commented Jun 6, 2023

joaotavora commented Jun 6, 2023

joaotavora commented Jun 8, 2023

Tune File Notify Watchers to share directory file descriptors #1228

Tune File Notify Watchers to share directory file descriptors #1228

Conversation

thejeffphil commented Jun 1, 2023

joaotavora commented Jun 1, 2023

jeff-phil commented Jun 5, 2023

joaotavora commented Jun 6, 2023

jeff-phil commented Jun 6, 2023

joaotavora commented Jun 6, 2023

joaotavora commented Jun 8, 2023