sebastiw / edts

Erlang Development Tool Suite
GNU Lesser General Public License v3.0
353 stars 82 forks source link

project node constantly being reinitialized for each newly opened file #237

Open klajo opened 6 years ago

klajo commented 6 years ago

Hi,

I've been running an older version of edts for a while but decided to upgrade. I then noticed that opening a new file (in the same project) took longer than before (short story: project node is reinitialized).

There seem to be a couple of issue behind this. I saw this in *Messages*:

EDTS [info]: Initializing node for project foo
EDTS [info]: Node foo started
EDTS [info]: Initializing node for project foo
EDTS [info]: Node foo started
EDTS [info]: Successfully intialized node foo
EDTS [info]: Initializing node for project foo
EDTS [info]: Node foo started
EDTS [info]: Initializing node for project foo
EDTS [info]: Node foo started
EDTS [info]: Successfully intialized node foo

It looks like edts is constantly initializing a new node. edts-api-get-nodes checks whether "foo" is a member of ("foo@bar" ...), which means edts-api-node-registeredp always returns nil. See edts-api-get-nodes.

I changed that to this (which removes the "@..." from the list of nodes.

(defun edts-api-get-nodes (&optional no-error)
  "Return all nodes registered with the EDTS server. If NO-ERROR is
non-nil, don't report an error if the request fails."
  (let* (nodes
         (edts-log-inhibit no-error)
         (res (edts-rpc-call "get_nodes" nil)))
    (-map (lambda (node) (car (s-split "@" node)))
          (cdr (assoc 'nodes (cdr (assoc 'body res)))))))

This should mean that edts-api-init-project-node goes into refresh mode instead of init.

The next thing was if I open (follow a dependency, use the module lookup function, open a file directly, ...) a file it starts edts-mode for that buffer. And that nowadays does this:

(defun edts-setup ()
  (edts-log-debug "Setting up edts-mode in buffer %s" (current-buffer))
  (edts-project-init)
  (edts-api-init-project-node)
...

I.e. it'll reinitialize the project node every time a file is opened the first time. For a small project that's likely quick, but for a bigger it takes a while and CPUs run hot (xref etc. I suppose).

Here's the log from when setting the edts-log-level to 'debug:

EDTS [debug]: Sending call to http://0:4587/get_mfas
EDTS [debug]: Reply 200 received for request to http://0:4587/get_mfas
EDTS [debug]: Sending call to http://0:4587/get_function_info
EDTS [debug]: Reply 200 received for request to http://0:4587/get_function_info
EDTS [debug]: Setting up edts-mode in buffer foo.erl
EDTS [debug]: Initializing node foo
EDTS [debug]: Sending async call to http://0:4587/init_node
EDTS [debug]: Running EDTS mode-hooks in foo.erl
EDTS [debug]: Done setting up EDTS mode in foo.erl
EDTS [debug]: Setting up edts-mode in buffer foo.erl
EDTS [debug]: Initializing node foo
EDTS [debug]: Sending async call to http://0:4587/init_node
EDTS [debug]: Running EDTS mode-hooks in foo.erl
EDTS [debug]: Done setting up EDTS mode in foo.erl
EDTS [debug]: Sending call to http://0:4587/get_nodes
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_xref/start
EDTS [debug]: Reply 200 received for request to http://0:4587/init_node
EDTS [info]: Successfully intialized node foo
EDTS [debug]: Plugin call edts_xref:start on foo
EDTS [debug]: Sending async call to http://0:4587/plugins/edts_xref/start
EDTS [debug]: Reply 200 received for request to http://0:4587/get_nodes
EDTS [debug]: Plugin call edts_debug:interpreted_modules on foo
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Plugin call edts_debug:interpreted_modules on otp-foo-otp
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Sending call to http://0:4587/get_nodes
EDTS [debug]: Reply 200 received for request to http://0:4587/get_nodes
EDTS [debug]: Plugin call edts_debug:breakpoints on foo
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Plugin call edts_debug:breakpoints on otp-foo-otp
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Sending call to http://0:4587/get_nodes
EDTS [debug]: Reply 200 received for request to http://0:4587/get_nodes
EDTS [debug]: Plugin call edts_debug:processes on foo
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Plugin call edts_debug:processes on otp-foo-otp
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Sending call to http://0:4587/get_nodes
EDTS [debug]: Reply 200 received for request to http://0:4587/get_nodes
EDTS [debug]: Plugin call edts_debug:interpreted_modules on foo
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Plugin call edts_debug:interpreted_modules on otp-foo-otp
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/interpreted_modules
EDTS [debug]: Sending call to http://0:4587/get_nodes
EDTS [debug]: Reply 200 received for request to http://0:4587/get_nodes
EDTS [debug]: Plugin call edts_debug:breakpoints on foo
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Plugin call edts_debug:breakpoints on otp-foo-otp
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/breakpoints
EDTS [debug]: Sending call to http://0:4587/get_nodes
EDTS [debug]: Reply 200 received for request to http://0:4587/get_nodes
EDTS [debug]: Plugin call edts_debug:processes on foo
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Plugin call edts_debug:processes on otp-foo-otp
EDTS [debug]: Sending call to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_debug/processes
EDTS [debug]: Reply 200 received for request to http://0:4587/plugins/edts_xref/start

Thankful for any help. What would be the intended behavior here?

Cheers, Klas

AeroNotix commented 6 years ago

@tjarvstrand I've removed the parts of the code which refresh the node after the project node is already open and you open a new file within that project and nothing seems to immediately break.

Why does edts need to refresh the project node when you open a new file within that project?

tjarvstrand commented 6 years ago

Well, motivation is to update code path, xref etc if you open a file in a newly created directory (eg. app) in your project. Needless to say, this could be improved immensely, but as it stands I'm a bit hesitant to just remove it.

robertoaloi commented 6 years ago

I can confirm the issue.

Just to clarify, the performance degradation seems due to the way the edts-api-init-project-node function works:

(defun edts-api-init-project-node ()
  (interactive)
  (edts-api-ensure-server-started)
  (if (edts-api-node-registeredp (edts-project-attribute :node-name))
      (progn
        (edts-api-refresh-project-node))
    ;; Ensure project node is started
    (unless (edts-api-node-started-p (edts-project-attribute :node-name))
      (edts-api--start-project-node))
    ;; Register it with the EDTS node
    (edts-api--register-project-node)))

If the node is already registered, the edts-api-refresh-project-node function is called, which calls edts-api-init-node, which is the expensive bit. This seems un-necessary in most scenarios. Moreover, since the edts-api-refresh-project-node function is marked as interactive, it could be removed from here, documented and manually invoked when necessary. An possible approach to tackle this could be to define a refresh function which only executes the required parts of init and not the whole thing. As a side note, one part which is particularly heavy in the initialization seems to be related to edts_dist:remote_load_modules/X.

tjarvstrand commented 6 years ago

IIRC, I think that what it all boils down to is that it would be nice to monitor the file system and apply only the necessary changes as opposed taking the shotgun approach an re-initialize the whole system. That way the incremental refresh should hopefully not affect performance at all.