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

Import PlutoRunner as module in Loader.jl #1881

Merged
merged 16 commits into from
Dec 4, 2023
Merged

Conversation

dralletje
Copy link
Collaborator

No idea if this is faster currently, or if it will every be...
But apart from having to do Pkg.develop() this feels like a more mature way to load PlutoRunner :P

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2022

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/fonsp/Pluto.jl", rev="PlutoRunner-as-module")
julia> using Pluto

@dralletje dralletje marked this pull request as draft February 2, 2022 02:44
@fonsp
Copy link
Owner

fonsp commented Feb 2, 2022

Nice! I ended up rewriting the whole thing:

  • Now using LOAD_PATH and Base.ACTIVE_PROJECT instead of pkg_ctx (this fixed the tests)
  • NotebookBootEnvironment.toml no longer needed, since PlutoRunner is the only "dependency"
  • Bit cleaner with try ... finally

@fonsp
Copy link
Owner

fonsp commented Feb 2, 2022

We still need to test:

  • Does this improve launch time? (@rikhuijzer can you use the new benchmark system to test this?) EDIT: looks like there is almost no difference...
  • How do different Pluto versions interfere? (@fonsp will do this) EDIT: works just like before
  • What about RelocatableFolders.jl? (Was that working before?) EDIT: very likely that it will work

@dralletje
Copy link
Collaborator Author

Nice! I ended up rewriting the whole thing:

  • Now using LOAD_PATH and Base.ACTIVE_PROJECT instead of pkg_ctx

  • NotebookBootEnvironment.toml no longer needed, since PlutoRunner is the only "dependency"

  • Bit cleaner with try ... finally

Eyeroll... 😉

@dralletje
Copy link
Collaborator Author

  • What about RelocatableFolders.jl? (Was that working before?)

I'll test it, but I'm pretty sure that still works as RelocatableFolders will "just" create the whole runner folder.

@fonsp fonsp requested a review from rikhuijzer March 18, 2022 14:43
@fonsp fonsp marked this pull request as ready for review March 18, 2022 14:44
Comment on lines 1 to 5
# The goal of this file is to import PlutoRunner into Main.
#
# This is difficult because PlutoRunner uses standard libraries and packages that are not necessarily available in the standard environment.
#
# Our solution is to create a temporary environment just for loading PlutoRunner. This environment is stored in `.julia/environments/__pluto_book_v*_*/`, and used by all notebook launches. Reusing the environment means extra speed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great comment. I was already confused about the code below this

@rikhuijzer
Copy link
Collaborator

rikhuijzer commented Mar 18, 2022

Does this improve launch time? (@rikhuijzer can you use the new benchmark system to test this?)

Hard to tell since this branch isn't up-to-date with main. I've manually added the compile tests on top of this branch and it doesn't look off. Some stuff of the PlutoRunner is covered via SessionActions.open. I should definitely add some PlutoRunner benchmarks.

@fonsp
Copy link
Owner

fonsp commented Mar 18, 2022

Let me rebase! Sorry about that

@fonsp
Copy link
Owner

fonsp commented Mar 18, 2022 via email

@fonsp
Copy link
Owner

fonsp commented Apr 20, 2022

This PR can cause problems when you use Pluto inside Pluto, at a different version than the server. But that problem might already be there because we check the existence of Main.PlutoRunner... needs more thinking

EDIT: this might only be an issue when not using distributed computing, but that might already be a problem before this PR.

@Pangoraw
Copy link
Collaborator

With 1.9 it might be interesting to revisit this PR. Here are some unscientific measurements:

On main (julia 1.9-rc2):

julia> @time let
           nb = Pluto.Notebook([Pluto.Cell("1 + 1")])
           sess = Pluto.ServerSession()
           Pluto.update_run!(sess, nb, nb.cells)
           nb.cells[1].output.body
       end
  7.291519 seconds (164.88 k allocations: 12.471 MiB, 0.29% compilation time)
"2"

With the loader as a package, this pr (julia 1.9-rc2):

julia> @time let
           nb = Pluto.Notebook([Pluto.Cell("1 + 1")])
           sess = Pluto.ServerSession()
           Pluto.update_run!(sess, nb, nb.cells)
           nb.cells[1].output.body
       end
  5.720203 seconds (164.12 k allocations: 12.435 MiB, 0.36% compilation time)
"2"

Note that there are no precompile statements in PlutoRunner itself so it can probably be improved more?

This PR can cause problems when you use Pluto inside Pluto, at a different version than the server. But that problem might already be there because we check the existence of Main.PlutoRunner... needs more thinking

This problem will be solved when we enable Malt so that each Pluto instance can drive its own notebook processes independently.

@fonsp fonsp added backend Concerning the julia server and runtime display & PlutoRunner & AbstractPlutoDingetjes.jl performance labels Apr 26, 2023
@disberd
Copy link
Contributor

disberd commented Aug 23, 2023

I'd like to add that apart from precompilation time this PR (making PlutoRunner its own package with UUID) will open up a lot of possibilities regarding building package extensions to do Pluto specific stuff.

At the moment one has to rely on AbstractPlutoDingetjes but having a Pluto-functionality extensions for packages that are loaded when PlutoRunner is loaded (so inside Pluto) would be super nice!

@fonsp
Copy link
Owner

fonsp commented Dec 2, 2023

Woohooo almost ready!

We thought hard about interference between Pluto versions and nested Pluto and it will be fine!

TODO:

  • Wait for CI
  • Rerun benchmark from Paul's previous commet

@Pangoraw
Copy link
Collaborator

Pangoraw commented Dec 3, 2023

Benchmark results

On main (b15d420)

julia> import Pluto
       @time let
           nb = Pluto.Notebook([Pluto.Cell("1 + 1")])
           sess = Pluto.ServerSession()
           Pluto.update_run!(sess, nb, nb.cells)
           nb.cells[1].output.body
       end
8.171571 seconds (190.23 k allocations: 14.144 MiB, 0.54% compilation time)
"2"

This PR

julia> import Pluto
       @time let
           nb = Pluto.Notebook([Pluto.Cell("1 + 1")])
           sess = Pluto.ServerSession()
           Pluto.update_run!(sess, nb, nb.cells)
           nb.cells[1].output.body
       end
6.713073 seconds (178.20 k allocations: 13.227 MiB, 0.33% compilation time)
"2"

@fonsp
Copy link
Owner

fonsp commented Dec 4, 2023

lets merge!

@Pangoraw Pangoraw merged commit 5a41179 into main Dec 4, 2023
15 checks passed
@Pangoraw Pangoraw deleted the PlutoRunner-as-module branch December 4, 2023 19:48
@fonsp
Copy link
Owner

fonsp commented Dec 6, 2023

On Julia 1.9, launching the second notebook in a session with Malt went from 4 seconds to 3 seconds :)

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 display & PlutoRunner & AbstractPlutoDingetjes.jl performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants