Open ffevotte opened 4 years ago
Could you rebase on master, and I'll try to review in the next few days?
OK, the PR is now rebased on the current master, and everything seems to work on my system.
There were several conflicts though; I'll try and test more in the coming days, and report back if anything goes wrong.
As always, thanks for your insightful comments, that are thought-provoking.
To sum everything up, my current position would be along the lines of:
eglot-jl-enable-sysimage
cusomization variable, that is set to nil by default so that users who don't opt-in would have absolutely no overhead. And users who opt in wouldn't have to manually call eglot-jl-compile-sysimage
: if no suitable sysimage is found when eglot
starts, they would be prompted to launch the compilation of one in the background.What would you think about that?
That sounds like a good path forward to me. :) I will note that if using the custom sysimage requires spawning multiple julia processes, it should definitely be opt-in rather than opt-out, but that's the plan anyway.
As an alternative, maybe eglot-jl-enable-sysimage
could cause the sysimage to be compiled on eglot-jl-init
so that compiling the sysimage doesn't run afoul of eglot-connect-timeout
? Then invoking eglot
either errors or shows a warning if the sysimage isn't available? We would want to make sure this is happening asynchronously of course.
I'v finally gotten around to working on this again. The latest commit should implement the strategy discussed above:
eglot-jl-enable-sysimage
is nil by default. No additional latency is introduced unless this setting is manually changed.eglot-jl-enable-sysimage
automatically triggers sysimage compilation on eglot-jl-init
when no suitable sysimage is found.eglot-jl-enable-sysimage
is set and a suitable system image is found, it is automatically used.${DIR}/eglot-jl_${VERSION}_${SLUG}.{EXT}
${DIR}
: path to the active project (should be the one defined by eglot-jl-language-server-project
). Defaults to eglot-jl-base
for Julia versions <1.4 where Pkg.project()
is not defined (more on that hereafter).${VERSION}
: Julia version string. This should allow avoiding problems with the sysimage becoming incorrect when Julia gets updated.${SLUG}
: sha1 checksum of the contents of ${DIR}/Manifest.toml
. This should allow avoiding problems with the sysimage becoming out-of-date when eglot-jl and its Julia dependencies get updated.${EXT}
: suitable extension depending on the OS.sysimage-path.jl
. When needed in Emacs, a Julia process is spawned for this (never happens unless eglot-jl-enable-sysimage
is set).@non-Jedi When you have some time to look at this (no rush!), I'd love to hear what you think.
There is a small gotcha regarding the use of system images in conjunction with customized eglot-jl-language-server-project
: although the Emacs side of the process knows which LanguageServer project should be activated, we currently don't forward that explicitly to Julia. And introspection capabilities have only been recently introduced in Pkg, so that for older Julia versions, there is no way (that I know of) of retrieving the currently active project in order to correctly encode the SHA1 of its Manifest in the sysimage name.
There are several ways of dealing with this that I can think of:
eglot-jl-enable-sysimage
and eglot-jl-language-server-project
only work well together for Julia 1.4 and aboveeglot-jl-language-server-project
to the Julia process; this would probably be the easiest solution to implement, requiring minimal changes to the current code. But "hiding" this in an environment variable does not look too clean to me.eglot-jl-language-server-project
as an additional (optional) command-line argument to eglot-jl.jl
. But I fear this will make calling eglot-jl.jl
more complex, for maybe not too much of a benefit. EDIT: come to think of it, eglot-jl.jl
does not have to know anything about the system image name; only compile.jl
. So I definitely prefer this option!I think my preference goes to this last option, but I'd love to hear your thoughts about it if you have some...
I went ahead and fixed the issues with eglot-jl-language-server-project
and eglot-jl-enable-sysimage
being used together in the latest commit. I'm happy with this solution, since it does not affect eglot-jl.jl
in any way for users not opting in the sysimage feature.
I haven't had an opportunity to use Julia for a while to test-drive this, but at this point if you've been using this without issue, I'm fine with you merging it. Sorry for the extremely long review delay.
This new version is rebased on the current master. It also incorporates (minor) modifications related to the need to resolve environments when switching Julia version.
I'm planning to test this in daily life for a few days in order to check that nothing went wrong with those last changes.
This would still be nice to have IMO, even with the improvements in 1.9.
The following PR introduces features helping compile and use custom system images for LanguageServer and SymbolServer. The expected benefits are reduced start-up times for the language server. On my (old-ish) machine, start-up times are reduced from 20 seconds to less than 2. On a newer machine where I also tested this, times went from 15 seconds down to less than 1.
The idea is the following:
eglot-jl.jl
looks at theEGLOT_JL_TEST
environment variable; when it is set, the server is configured to read from an IOBuffer that sends it anexit
instruction as soon as it is able to process requests. This allowseglot-jl.jl
to be used as aprecompile_execution_file
that exercises the (language) server before gracefully exiting.compile.jl
script automates the generation of a system image, named after the julia version (and having the relevant extension depending on the OS).This is what the first commit in this PR introduces, alongside with instructions in
README.org
explaining how to compile the system image, and seteglot-jl-julia-args
to use it.The second commit goes a step further: it automates the search for a suitable system image at server startup, in order to use it if one is found.
--sysimage
command-line switch is generated. If not, nothing happens; no system image is used, so users who have not generated any system image will not be impacted by this.eglot-jl-enable-sysimage
customizable option. This allows users to opt out of the system, even if they have previously generated a system image (for example in case it would become stale for some reason). It also completely removes the julia version overhead.Additionally, a new autoloaded command allows running the system image generation script from within emacs. The same command could be used to re-generate a system image if the previous one caused problems for any reason.
This is documented in
README.org
. I've tested this with Julia 1.3 and 1.4, on Windows and Linux, without encountering any problem (I don't have access to a Mac).I know you were reluctant to handle system images in
eglot-jl
, and were (rightfully) especially concerned about possible ways that a system image would become obsolete. I tried making this implementation as robust as possible, but I might very well have overlooked something. So please do not hesitate to tell me if something bothers you with this proposal.