search

julia-vscode / SymbolServer.jl

Other
22 stars 31 forks source link

Extreme inefficiency in main server loop #269

Closed thomasjm closed 9 months ago

thomasjm commented 1 year ago

I was looking at this code in server.jl, where it loads the packages one by one, and then every time does a bunch of work over the whole package environment:

https://github.com/julia-vscode/SymbolServer.jl/blob/3162b6a1b53d7036c53e24c3cc2fe21b90acf67a/src/server.jl#L95-L100

The comment alludes to how this may duplicate some work. It is a bit of an understatement. I was testing with indexing an environment containing only Plots and its dependencies, and I tried moving the end of the loop on line 95 up to line 97, so we do all the load_package calls up front.

This made the total indexing time go from 109.1 seconds to 12.2 seconds. Almost a 10X improvement from this one change!

In general, I think the perf of SymbolServer.jl has more low-hanging fruit, such as indexing and loading on-disk stores in parallel using multiple threads. I feel that the current painful performance has led to workarounds like the cloud indexing system, which I've never seen before in any other LSP ecosystem and which I really wish didn't exist. (Even more philosophically, I wonder why gobs of symbols need to be indexed and stored in memory up-front at all? Doesn't Julia have the introspection machinery to enable LanguageServer.jl to look up (and cache as needed) the necessary symbols as it runs?)

In my usage at least, SymbolServer.jl is the main contributor to slow startup times and high CPU/memory usage when using LanguageServer.jl. I think a leaner and meaner SymbolServer.jl would make life much nicer for LanguageServer.jl users.

CC @mkitti since you were so helpful with the last perf issue I reported. Also CC @davidanthoff.

mkitti commented 1 year ago

Could you send a pull request to test?

thomasjm commented 1 year ago

This may be a bit more than you asked for, but I've been meaning to open a PR with a collection of improvements I've been working on: #270. With this PR, it's more straightforward to just call SymbolServer.index_packages(...) in order to test the performance, since that logic isn't hidden in a script.

If you just want the change that this issue is about, it looks like this: https://github.com/julia-vscode/SymbolServer.jl/compare/master...codedownio:SymbolServer.jl:indexing-perf?expand=1

thomasjm commented 1 year ago

Oh and if you use Nix, I can provide a Nix-based way to test the indexing.