WhatsApp / erlang-language-platform

Erlang Language Platform. LSP server and CLI.
https://whatsapp.github.io/erlang-language-platform/
Apache License 2.0
209 stars 16 forks source link

Multiple high CPU eqwalizer tasks and emacs freezing #16

Closed tsloughter closed 4 months ago

tsloughter commented 4 months ago

Describe the bug

I was waiting to open this until I had more info but I've not had time to dig in simple checks and wanted to get this open in case anyone else knows how/where I should look.

The issue is frequently seeing processes like eqwalizerkiPIC5 at 100% cpu and emacs hangs for a few seconds. The hanging doesn't happen every time I see a process at 100% and sometimes the process has arguments, like:

 /tmp/eqwalizerkiPIC5 ipc opentelemetry_exporter_metrics_service_pb  

That file is large, 5980 lines, and open in a buffer but not the one I'll be editing when this happens.

To Reproduce

Not entirely sure but it happens on https://github.com/open-telemetry/opentelemetry-erlang and may be tied to having files like https://github.com/open-telemetry/opentelemetry-erlang/blob/main/apps/opentelemetry_exporter/src/opentelemetry_exporter_metrics_service_pb.erl

Expected behavior

To not constantly see processes at 100%, even when nothing has changed. At the moment I haven't editted anything in minutes but have 3 eqwalizerkiPIC5 processes at 200% cpu.

Actual behavior

Frequent freezing of emacs and, even when not frozen, constant eqwalizer processes using high cpu.

Context

alanz commented 4 months ago

We have a known problem of performance with generated protobuf files.

A workaround in the meantime is to disable eqwalizer for the specific file by adding -eqwalizer(ignore). to the top of the file, or to add a @generated tag in the first 2000 characters of the file.

tsloughter commented 4 months ago

Thanks! Should I leave this open or close it?

alanz commented 4 months ago

Leave it open, we are looking at ways of adding config to control the eqwalizer resource usage, which will happen against this issue.

tomas-abrahamsson commented 4 months ago

... or to add a @generated tag in the first 2000 characters of the file.

Will ELP pick up @generated even if it does not appear first on a (comment) line? I'm trying to find a way to say @generated that doesn't "upset" edoc.

$ cat a.erl
%% this file is @generated
-module(a).
$ cat b.erl                                                            
%% @doc example
%% @generated
-module(b).
$ 
$ erl -noinput -eval 'erlang:display(edoc:file("a.erl", [])), halt(0).' 
ok
$ erl -noinput -eval 'erlang:display(edoc:file("b.erl", [])), halt(0).'
b.erl, in module header: at line 2: warning: tag @generated not recognized.
ok
$ 
alanz commented 4 months ago

We just look for the presence of it in the first 2000 chars. see https://github.com/WhatsApp/erlang-language-platform/blob/425a5dadf249541a5484a51fdc03f5401cfa7056/crates/base_db/src/lib.rs#L210

The edoc manual suggests doing

% % @doc ...
alanz commented 4 months ago

We have also added the ability to set how many eqwalizer instances should run, in the .elp.toml file.

But you must build from source or wait for the next release to be able to use it.

tsloughter commented 4 months ago

Thanks for the quick release @tomas-abrahamsson

But @alanz I just regenerated the files with the added @generated and restarted emacs to be sure but still see the process popping up. I'm now on elp version 1.1.0+build-2024-02-16 and see:

/tmp/eqwalizeruo4hT2 ipc opentelemetry_exporter_metrics_service_pb

despite the top of that file being:

%% -*- coding: utf-8 -*-
%% % this file is @generated
%% @private
%% Automatically generated, do not edit
%% Generated by gpb_compile version 4.21.1
%% Version source: file
-module(opentelemetry_exporter_metrics_service_pb).
alanz commented 4 months ago

We have been looking at the code, and requesting types on hover does not honour the @generated tag. It probably makes sense to have it do so.

In the meantime, can you do an experiment and add -eqwalizer(ignore). to the generated file (by hand), and see if anything changes?

tsloughter commented 4 months ago

Ah yea, just did and it looks like it may have done it. I'd say I need to use it longer to be positive but I was seeing those processes immediately before adding the module attribute and now they are gone.

alanz commented 4 months ago

Ok, we will make the change for types on hover not to do generated.

alanz commented 4 months ago

I think this is done now, will close.

The new release should also make things more stable.