Memory leak

[Mavaxavou]

I'm working in the Frama-C project and when I edit the file src/plugins/eva/engine/abstractions.ml, the LSP starts consumming tremendous amout of memory for each modification. This behavior appears even when simply adding comments.

If it can help the bug fix without having to dive into the Frama-C project, the file is full of complex constructions combining GADT and first class modules. As it is the only file (that I'm aware of) that trigger the bug, and also the most complex one of the project (again, that I'm aware of), I guessed it may be linked.

I'm on Arch, my Ocaml is at version 4.14.0 and my LSP is at version 1.15.1-4.14. The bug seems reproductible on other OS, a collegue have the same problem on an Ubuntu. I've not managed to test on other Ocaml or LSP versions for now.

[voodoos]

Thanks for the report @Mavaxavou

It could be related to this Merlin issue. (It is not clear if it is a real "memory leak" of simply the editor hammering merlin and thus ocaml's typer which can be expensive on functor-heavy code).

I am curious if you could check if the behavior you describe is happening at any place in the file or if it is worse at the beginning or at the end ?

Another useful thing to do would be to try with Merlin directly without the OCaml-LSP frontend. The easiest way would be to use Emacs with tuareg/merlin modes and play with completion which also queries Merlin at each keystroke when erasing an identifier.

[AllanBlanchard]

I am curious if you could check if the behavior you describe is happening at any place in the file or if it is worse at the beginning or at the end ?

It seems to me that it is worse at the beginning of the file.

Another useful thing to do would be to try with Merlin directly without the OCaml-LSP frontend.

It also happens with Merlin alone.

[voodoos]

It also happens with Merlin alone

Right, thanks for checking. We could move this conversation to the Merlin tracker, but I feel like this is as much a frontend than a backend issue.

It seems to me that it is worse at the beginning of the file.

That's interesting. Merlin has a smart caching mechanism for the buffer's typing: after an edit it will only re-type the end of the buffer starting at the edited item. This seems to confirm that the issue is due to the alliance of two things:

• typing (which is done by the compiler) being slow and memory expensive in functor-heavy modules and,
• the editor is sending too many successive queries (with every key-press)

And of course there could be a leak in the typer itself.

As a first remediation I think throttling requests when they are slow ? Or making sure that completion, for example, is triggered only on the first character and then on every dots, the rest can be filtered by the editor.

@rgrinberg is there some throttling mechanism in the LSP protocol ?

[3Rafal]

@voodoos, I wonder if instead of filtering/throttling expensive requests, we could just cancel them with: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#cancelRequest

wdyt?

[rgrinberg]

We already support user requested cancellation. Do you think server requested cancellation would help?

[3Rafal]

I'm not sure... I was exploring ideas different than throttling, that are actually supported by LSP.

It was something I came up with when reading Implementation Considerations. I don't think that server can cancel arbitrary requests though...

[voodoos]

An update on this issue. We finally found the culprit in OCaml's typer: a cache for functor application was growing because of staled idents. We have a patch for Merlin that appear to fix that issue when testing on a heavily functorized codebase: https://github.com/ocaml/merlin/pull/1609

@AllanBlanchard it would be great if you could try this branch (https://github.com/ocaml/merlin/tree/cleanup-caches) of Merlin and see if it also fixes your issue.

[AllanBlanchard]

When I pin the branch, I get the following error:

#=== ERROR while compiling merlin.4.7-414 =====================================#
# context     2.0.8 | linux/x86_64 | ocaml-base-compiler.4.14.0 | pinned(git+https://github.com/ocaml/merlin.git#cleanup-caches#00b85a01)
# path        ~/.opam/4.14.0/.opam-switch/build/merlin.4.7-414
# command     ~/.opam/opam-init/hooks/sandbox.sh build dune build -p merlin -j 11
# exit-code   1
# env-file    ~/.opam/log/merlin-22480-899ac0.env
# output-file ~/.opam/log/merlin-22480-899ac0.out
### output ###
# [...]
# ocamlmerlin.c:262:40: warning: ‘%s’ directive output may be truncated writing up to 4096 bytes into a region of size 102 [-Wformat-truncation=]
#   262 |     snprintf(address.sun_path, 104, "./%s", socketname);
#       |                                        ^~
# In file included from /usr/include/stdio.h:894,
#                  from ocamlmerlin.c:2:
# /usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 3 and 4099 bytes into a destination of size 104
#    71 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
#       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#    72 |                                    __glibc_objsize (__s), __fmt,
#       |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#    73 |                                    __va_arg_pack ());
#       |                                    ~~~~~~~~~~~~~~~~~

<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
┌─ The following actions failed
│ λ build merlin 4.7-414
└─ 
╶─ No changes have been performed

[voodoos]

🤔 No change were made recently to that part of ocamlmerlin.c and the log shows only warnings ? So the error must be something else ? The CI passed so it is surprising, maybe it's due to pinning and dependencies.

Could you copy the complete log from ~/.opam/log/merlin-22480-899ac0.out ?

[AllanBlanchard]

$ cat .opam/log/merlin-22480-899ac0.out 
(cd _build/default && /home/allanb/.opam/4.14.0/bin/ocamlc.opt -w -40 -open Ocaml_utils -open Ocaml_parsing -open Ocaml_typing -open Merlin_kernel -open Merlin_utils -open Merlin_analysis -open Merlin_kernel -g -bin-annot -I src/frontend/ocamlmerlin/.ocamlmerlin_server.eobjs/byte -I /home/allanb/.opam/4.14.0/lib/merlin-lib/analysis -I /home/allanb/.opam/4.14.0/lib/merlin-lib/config -I /home/allanb/.opam/4.14.0/lib/merlin-lib/kernel -I /home/allanb/.opam/4.14.0/lib/merlin-lib/ocaml_parsing -I /home/allanb/.opam/4.14.0/lib/merlin-lib/ocaml_typing -I /home/allanb/.opam/4.14.0/lib/merlin-lib/ocaml_utils -I /home/allanb/.opam/4.14.0/lib/merlin-lib/os_ipc -I /home/allanb/.opam/4.14.0/lib/merlin-lib/query_commands -I /home/allanb/.opam/4.14.0/lib/merlin-lib/query_protocol -I /home/allanb/.opam/4.14.0/lib/merlin-lib/utils -I /home/allanb/.opam/4.14.0/lib/yojson -no-alias-deps -open Dune__exe -o src/frontend/ocamlmerlin/.ocamlmerlin_server.eobjs/byte/dune__exe__Query_json.cmo -c -impl src/frontend/ocamlmerlin/query_json.ml)
File "src/frontend/ocamlmerlin/query_json.ml", line 202, characters 16-38:
202 |   | Occurrences (`Ident_at pos, scope) ->
                      ^^^^^^^^^^^^^^^^^^^^^^
Error: This pattern matches values of type 'a * 'b
       but a pattern was expected which matches values of type
         [ `Ident_at of Merlin_kernel.Msource.position ]
(cd _build/default/src/frontend/ocamlmerlin && /usr/bin/gcc -O2 -fno-strict-aliasing -fwrapv -pthread -fPIC -oocamlmerlin.exe ocamlmerlin.c)
ocamlmerlin.c: In function ‘start_server.constprop’:
ocamlmerlin.c:356:40: warning: ‘%s’ directive output may be truncated writing up to 4096 bytes into a region of size 102 [-Wformat-truncation=]
  356 |     snprintf(address.sun_path, 104, "./%s", socketname);
      |                                        ^~
In file included from /usr/include/stdio.h:894,
                 from ocamlmerlin.c:2:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 3 and 4099 bytes into a destination of size 104
   71 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   72 |                                    __glibc_objsize (__s), __fmt,
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   73 |                                    __va_arg_pack ());
      |                                    ~~~~~~~~~~~~~~~~~
ocamlmerlin.c:380:41: warning: ‘__builtin___snprintf_chk’ output may be truncated before the last format character [-Wformat-truncation=]
  380 |     snprintf(socket_path, PATHSZ, "%s/%s", path_socketdir(), socketname);
      |                                         ^
In file included from /usr/include/stdio.h:894,
                 from ocamlmerlin.c:2:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output 2 or more bytes (assuming 4098) into a destination of size 4097
   71 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   72 |                                    __glibc_objsize (__s), __fmt,
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   73 |                                    __va_arg_pack ());
      |                                    ~~~~~~~~~~~~~~~~~
ocamlmerlin.c: In function ‘connect_socket.constprop’:
ocamlmerlin.c:262:40: warning: ‘%s’ directive output may be truncated writing up to 4096 bytes into a region of size 102 [-Wformat-truncation=]
  262 |     snprintf(address.sun_path, 104, "./%s", socketname);
      |                                        ^~
In file included from /usr/include/stdio.h:894,
                 from ocamlmerlin.c:2:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 3 and 4099 bytes into a destination of size 104
   71 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   72 |                                    __glibc_objsize (__s), __fmt,
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   73 |                                    __va_arg_pack ());
      |                                    ~~~~~~~~~~~~~~~~~

[AllanBlanchard]

Oh, I think I understand ... (Indeed, I pinned only merlin, now it is ok)

[AllanBlanchard]

Ok, this branch also fixes the issue on Frama-C. Great!

[voodoos]

Great, we will try to make a release soon :-)

[voodoos]

Should be fixed with latest release. Don't hesitate to reopen if it is not.