Skip to content

Add PrecompileTools, precompile runserver#1218

Merged
pfitzseb merged 1 commit intojulia-vscode:masterfrom
mkitti:mkitti/precompiletools
Apr 26, 2023
Merged

Add PrecompileTools, precompile runserver#1218
pfitzseb merged 1 commit intojulia-vscode:masterfrom
mkitti:mkitti/precompiletools

Conversation

@mkitti
Copy link
Copy Markdown
Contributor

@mkitti mkitti commented Apr 26, 2023

Add PrecompileTools to and precompile runserver.

For every PR, please check the following:

@mkitti
Copy link
Copy Markdown
Contributor Author

mkitti commented Apr 26, 2023

I'm not sure why Project.toml got rearranged, but it should only be adding PrecompileTools.jl

@mkitti
Copy link
Copy Markdown
Contributor Author

mkitti commented Apr 26, 2023

julia> using PkgCacheInspector, Pkg

julia> Pkg.develop("LanguageServer")

julia> info_cachefile("LanguageServer")
Contents of /home/mkitti/.julia/compiled/v1.9/LanguageServer/ite7n_4MdHE.so:
  modules: Any[LanguageServer.URIs2, LanguageServer]
  815 external methods
  3756 new specializations of external methods (Base 72.7%, StaticLint 10.1%, JSONRPC 5.0%, ...)
  666 external methods with new roots
  15443 external targets
  11613 edges
  file size:   17084984 (16.294 MiB)
  Segment sizes (bytes):
  system:      5087440 ( 51.21%)
  isbits:      4201270 ( 42.29%)
  symbols:       59097 (  0.59%)
  tags:          73137 (  0.74%)
  relocations:  454313 (  4.57%)
  gvars:         30664 (  0.31%)
  fptrs:         28448 (  0.29%)

mkitti added a commit to mkitti/julia-vscode that referenced this pull request Apr 26, 2023
@timholy
Copy link
Copy Markdown

timholy commented Apr 26, 2023

Nice. The small amount of residual compilation described in https://discourse.julialang.org/t/precompiling-languageserver-runserver-with-precompiletools-jl/97926 might be fixable by @nospecializing(pipe) on all the IO objects in LanguageServer. (Presumably it doesn't actually run with a IOBuffer.) If you can @snoopi_deep it under "real" usage you can more directly figure out what wasn't covered by your workload.

@pfitzseb
Copy link
Copy Markdown
Member

Very cool! Did you time the difference in startup time?

And yes, we initialize the LS with Base.TTYs instead of IOBuffers.

@timholy
Copy link
Copy Markdown

timholy commented Apr 26, 2023

If printing doesn't have to be high performance, @nospecializing could be a good solution. Otherwise, there's been a request already to set expand PrecompileTools to add a harness capable of instantiating a fake TTY. If there are enough potential customers, adding that harness becomes more worth the downside of making PrecompileTools bigger.

@timholy
Copy link
Copy Markdown

timholy commented Apr 26, 2023

Very cool! Did you time the difference in startup time?

In https://discourse.julialang.org/t/precompiling-languageserver-runserver-with-precompiletools-jl/97926, he shows that @time runserver() goes from 30s to 10s. using LanguageServer goes from 20ms to 500ms, so it seems like a clear win.

Copy link
Copy Markdown
Member

@davidanthoff davidanthoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic! I'd say we can merge right away. I'll add PrecompileTools to the extension later today with our submodule way of loading things.

Also, we can probably improve a lot on this, because just running the server probably only executes a relatively small fraction of the code. But let's do that in follow up PRs.

@fredrikekre
Copy link
Copy Markdown
Member

Can

┌ LanguageServer [2b0e0bc5-e4fd-59b4-8912-456d1b03d8d7]
│  ERROR: Endpoint is not running, the current state is closed.
│  Stacktrace:
│   [1] error(s::String)
│     @ Base ./error.jl:35
│   [2] check_dead_endpoint!
│     @ ~/.julia/packages/JSONRPC/Q0FBr/src/core.jl:280 [inlined]
│   [3] get_next_message(endpoint::JSONRPC.JSONRPCEndpoint{IOBuffer, IOContext{IOStream}})
│     @ JSONRPC ~/.julia/packages/JSONRPC/Q0FBr/src/core.jl:212
│   [4] macro expansion
│     @ ~/.julia/packages/LanguageServer/bnyQN/src/languageserverinstance.jl:288 [inlined]
│   [5] (::LanguageServer.var"#113#116"{LanguageServer.LanguageServerInstance})()
│     @ LanguageServer ./task.jl:514
│  [ Info: Shutting down server instance.
└

be silenced? (I assume it comes from this PR)

@mkitti
Copy link
Copy Markdown
Contributor Author

mkitti commented May 11, 2023

I'm missing the part of the error stack trace that connects it with this pull request. Does this happen during precompilation?

@fredrikekre
Copy link
Copy Markdown
Member

Yes

@mkitti
Copy link
Copy Markdown
Contributor Author

mkitti commented May 11, 2023

One option from the code below is to set an error handler. Otherwise, we'll need to modify the else block below.

if server.err_handler !== nothing
server.err_handler(err, bt)
else
io = IOBuffer()
Base.display_error(io, err, bt)
print(stderr, String(take!(io)))
end

@mkitti
Copy link
Copy Markdown
Contributor Author

mkitti commented May 11, 2023

Is it just me or does the language server always end with an error?

while true
msg = JSONRPC.get_next_message(server.jr_endpoint)
put!(server.combined_msg_queue, (type = :clientmsg, msg = msg))
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants