Use precompile
for SessionActions.open
#1934
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is another shot at precompilation, see also #1392. I propose here to use
precompile
directly instead of running a minimal code example. This one line of code reduces the time forSessionActions.open
by 3 seconds on my system while only introducing 0.2 seconds atusing Pluto
(details below)For most packages, adding
precompile
manually doesn't really work, because for many packages, the inference looks something like this:(Source: SciML/ModelingToolkit.jl#1215 (comment)).
However, for
SessionActions.open
, it looks more like this:What these pictures tell is that for
SessionActions.open
, inference is pretty straightforward. OnceSessionActions.open
is inferred, all methods that it calls can be inferred too. For the ModelingToolkit on the other hand, "many calls are being made by runtime dispatch: each separate flame is a fresh entrance into inference." (SnoopCompile docs).This makes sense. Pluto is an application where many methods were written with specific types in mind. There is no point in dispatching on multiple types of notebooks or whatever like what numerical packages do with dispatching on different number types. After adding
the call graph looks as follows:
And the time is reduced by 3 seconds (see below for details). With this PR, I would suggest adding a few of these
precompile
calls at strategic places. Note that theprecompile
function compiles the given method but does not execute it which explains why this PR has less negative impact on@time using Pluto
than earlier attempts.Okay, then a few last things which might come up if you read this.
Firstly, one could wonder "why can't I just make the type signature contain only concrete types? Then there is only one possible method instance, so Julia can precompile that automatically." Well, that was not implemented because it increased the likelihood that people would put concrete types all over the place and that would be causing less generic Julia code (lower composability of packages), see JuliaLang/julia#12897 (comment).
Secondly, one could wonder "why not use SnoopCompile to generate a bunch of precompile directives?". That would indeed also be possible, but would be harder to maintain. The SnoopCompile directives are normally put in
src/precompile.jl
. Now, when updating the signature of a method, package developers have to remember to checksrc/precompile.jl
whether the precompile directives have to be updated as well. In the approach that I propose in this PR, the precompile directive is directly below the method definition so it should be easier to keep the two in-sync.Before
After