It takes 15 seconds for completions to appear in large files after errors

[Jarred-Sumner]

Zig Version

0.11.0-dev.2401+348751462

Zig Language Server Version

HEAD

Steps to Reproduce

Make a change in this file https://github.com/oven-sh/bun/blob/0cc56e8efce9c7d4905b3649827bf9b40a677b25/src/js_parser.zig#L5901

Expected Behavior

It should be instant

Actual Behavior

It seems to process a queue of errors and each error to process takes considerable time, so the more times you save the slower it gets.

Here is a 15 second gif (note that it was not slowed down)

[SuperAuguste]

Instinct says this might be code actions being gathered? Any ideas @Techatrix? :)

[Jarred-Sumner]

When I disable the error lens VSCode extension, it seems to be better - but it would be best not to disable that as it is very helpful

edit: nevermind, error lens is unrelated

[Techatrix]

Most of the time is spend constructing the document scope whenever a file changes. I previously investigated what caused document scope generation to be slow and I found out that it spends most of its time allocating in the GPA.

Here is tracy while editing Sema.zig. (I used Sema.zig instead of js_parser.zig because its a more extreme example)
I've added a tracy so that you can see that even deinit takes a lot of time.

I explored a few options that could optimize this. The most effective and simple one is replacing the GPA with malloc.
This has once been suggested by nullptrdevs here.

If you edit a file and keep it syntactically correct so that ast_check is going to be run often, setting prefer_ast_check_as_child_process to false will also help with reducing delays while editing.

If we want to optimize this even further, we probably need to do re-parsing or utilize multi-threading with some smart trickery.

[Jarred-Sumner]

• When spawning short-lived child processes, deinit can be skipped entirely because the process will free the memory on exit by the OS
• When spawning processes on macOS, it internally uses posix_spawn right? posix_spawn is much faster than exec on macOS

If you edit a file and keep it syntactically correct so that ast_check is going to be run often, setting prefer_ast_check_as_child_process to false will also help with reducing delays while editing.

I assume the "keep it syntactically correct" part is due to challenges with error handling? I lack context, but could this be run in the same process in one (in total) separate thread? Could try something like in the panic handler, reset a bump allocator to 0 and then do a long jmp back to the thread's init function

[Techatrix]

I think I kind of misdirected you with my last line about prefer_ast_check_as_child_process.

The deinit function I was referring to is this one. It only deallocates the memory that is used by a document scope. It is called whenever a file changes because we have to destroy the old document scope which is a ZLS data structure. document scopes are created and owned by the ZLS executable. No child process involved.
Getting zig ast-check errors however will use them (unless prefer_ast_check_as_child_process is false). The overhead of spawning the child processes is probably negligible because parsing and AstGen in the Zig executable (which is what zig ast-check internally does) will be taking up most of the time for large files.

With "keep it syntactically correct" is meant that zig ast-check is only requested on files that have been parsed with no errors. Therefore, if you edit a file while you have syntax errors then ast_check diagnostics will not be requested/computed. Most of the time while editing there is some syntax error because you didn't finish writing you code. In the tracy image above you will see that after refreshDocument finishes, the next zone is generateDiagnostics (text didn't fit) that finished very quickly because I was in the middle of writing a new variable declaration which produces a syntax error and therefore ast-check errors where not collected.

[Jarred-Sumner]

The overhead of spawning the child processes is probably negligible

On macOS, it looks like spawning the process is about 30% of the overhead. A persistent process which uses a bump allocator to reset the memory back to the beginning should make every call after the first run faster because in theory, it wouldn't be allocating additional memory from the operating system. If each of these takes 25ms or so and it's queuing calls to zig ast-check on each file save, that would partially explain why it takes some time to catch up.

[Jarred-Sumner]

The issue definitely gets worse the more times a file is saved until zls' next restart/crash

[Jarred-Sumner]

Using std.heap.c_allocator as the backing allocator for GeneralPurposeAllocator fixes the issue. I imagine the binned allocator will help as well.

[Jarred-Sumner]

Re-opened, as #1134 has not been merged, so this is still happening unless compiling ZLS separately in a fork

[Techatrix]

#1134 has been merged.