Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use precompile for SessionActions.open #1934

Merged
merged 1 commit into from
Feb 22, 2022
Merged

Use precompile for SessionActions.open #1934

merged 1 commit into from
Feb 22, 2022

Conversation

rikhuijzer
Copy link
Collaborator

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 for SessionActions.open by 3 seconds on my system while only introducing 0.2 seconds at using Pluto (details below)

For most packages, adding precompile manually doesn't really work, because for many packages, the inference looks something like this:

image

(Source: SciML/ModelingToolkit.jl#1215 (comment)).

However, for SessionActions.open, it looks more like this:

image

What these pictures tell is that for SessionActions.open, inference is pretty straightforward. Once SessionActions.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

precompile(SessionActions.open, (ServerSession, String))

the call graph looks as follows:

image

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 the precompile 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 check src/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

$ julia --project

pkg> precompile
[...]

julia> @time using Pluto
  0.705556 seconds (1.01 M allocations: 71.043 MiB, 76.06% compilation time)

julia> session = Pluto.ServerSession();

julia> path = "/home/rik/Downloads/tmp_simple_notebook.jl";

julia> @time Pluto.SessionActions.open(session, path);
11.303100 seconds (23.56 M allocations: 1.242 GiB, 5.09% gc time, 99.91% compilation time)

After

$ julia --project

pkg> precompile
[...]

julia> @time using Pluto
  0.946054 seconds (1.45 M allocations: 98.672 MiB, 2.80% gc time, 55.25% compilation time)

julia> session = Pluto.ServerSession();

julia> path = "/home/rik/Downloads/tmp_simple_notebook.jl";

julia> @time Pluto.SessionActions.open(session, path);
  8.472655 seconds (16.53 M allocations: 906.598 MiB, 3.31% gc time, 96.59% compilation time)

@github-actions
Copy link
Contributor

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/rikhuijzer/Pluto.jl", rev="rh/precompile")
julia> using Pluto

@rikhuijzer
Copy link
Collaborator Author

rikhuijzer commented Feb 20, 2022

To reproduce the graph, open a notebook with

begin
    using Pkg
    
    Pkg.activate(; temp=true)
    Pkg.add([
        "SnoopCompile",
        "ProfileSVG"
    ])
    Pkg.develop(; path="/home/rik/git/Pluto.jl")
end
begin
    using SnoopCompile
    using ProfileSVG
    using Pluto
end
session = Pluto.ServerSession();
path = "/home/rik/Downloads/tmp_simple_notebook.jl";
tinf = @snoopi_deep Pluto.SessionActions.open(session, path; run_async=true);
let
     g = flamegraph(tinf)
     ProfileSVG.view(g)
end

where tmp_simple_notebook.jl is a notebook containing something basic like 1 + 1.

@fonsp
Copy link
Owner

fonsp commented Feb 22, 2022

Wow, what an awesome find!!

@rikhuijzer rikhuijzer marked this pull request as draft February 22, 2022 13:09
@fonsp
Copy link
Owner

fonsp commented Feb 22, 2022

Let's merge this, unless you can find some other good improvements! It would be fantastic if we could precompile our HTTP server: Pluto.run. The easiest way to measure might be to use a stopwatch on your phone, and count how long between Pluto.run(notebook=asdf) and seeing the first Markdown text.

We are doing a release later this afternoon, let's get this PR in, and work on further improvements if you have time

@fonsp fonsp changed the title Use precompile Use precompile for SessionActions.open Feb 22, 2022
@rikhuijzer rikhuijzer marked this pull request as ready for review February 22, 2022 13:50
@fonsp fonsp merged commit f633e5b into fonsp:main Feb 22, 2022
@rikhuijzer rikhuijzer deleted the rh/precompile branch February 23, 2022 05:18
@fonsp fonsp added the backend Concerning the julia server and runtime label Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Concerning the julia server and runtime performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants