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

Return a Rich object instead of Dict for show_richest #2139

Closed
wants to merge 8 commits into from

Conversation

rikhuijzer
Copy link
Collaborator

@rikhuijzer rikhuijzer commented May 29, 2022

I was trying to reduce the TTFX for SessionActions.open and ended up in PlutoRunner again.

With this PR, show_richest will now always return a Rich object instead of a Dict. This improves readability and also makes the compiler a bit happier. It reduces allocations from ~90 MB to ~80 MB on the TTFX benchmark of #2117 on @btime it reduces time by 17 μs (wow wow). Anyway, the main argument for this PR is code readability.

@rikhuijzer rikhuijzer added the backend Concerning the julia server and runtime label May 29, 2022
@rikhuijzer rikhuijzer requested a review from fonsp May 29, 2022 10:46
@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/fonsp/Pluto.jl", rev="rh/rich-struct")
julia> using Pluto

@rikhuijzer
Copy link
Collaborator Author

rikhuijzer commented May 29, 2022

@fonsp, do you know where the following error is coming from?

cell.output.body = Dict{Symbol, Any}(
  :msg => "TaskFailedException\n\n    nested task error: could not compute non-nothing type\n    Stacktrace:\n      
    [1] error(s::String)\n        @ Base ./error.jl:35\n      
    [2] nonnothingtype_checked(T::Type)\n        @ Base ./some.jl:31\n      
    [3] convert(#unused#::Type{Union{Nothing, String, Dict, Vector{UInt8}, var\"#s119\"} where var\"#s119\"<:Pluto.PlutoRunner.Rich}, x::Main.PlutoRunner.RichObject)\n        @ Base ./some.jl:36\n      
    [4] Pluto.CellOutput(body::Main.PlutoRunner.RichObject, mime::MIME{Symbol(\"application/vnd.pluto.tree+object\")}, rootassignee::Symbol, last_run_timestamp::Float64, persist_js_state::Bool, has_pluto_hook_features::Bool)\n        @ Pluto ~/git/Pluto.jl/src/notebook/Cell.jl:12\n      
    [5] Pluto.CellOutput(; body::Main.PlutoRunner.RichObject, mime::MIME{Symbol(\"application/vnd.pluto.tree+object\")}, rootassignee::Symbol, last_run_timestamp::Float64, persist_js_state::Bool, has_pluto_hook_features::Bool)\n        @ Pluto ./util.jl:493\n      
    [6] set_output!(cell::Pluto.Cell, run::NamedTuple{(:output_formatted, :errored, :interrupted, :process_exited, :runtime, :published_objects, :has_pluto_hook_features), Tuple{Union{Tuple, Main.PlutoRunner.MimedOutput}, Bool, Bool, Bool, Union{Nothing, UInt64}, Dict{String, Any}, Bool}}, expr_cache::Pluto.ExprAnalysisCache; persist_js_state::Bool)\n        @ Pluto ~/git/Pluto.jl/src/evaluation/Run.jl:312\n      
    [7] run_single!(session_notebook::Tuple{Pluto.ServerSession, Pluto.Notebook}, cell::Pluto.Cell, reactive_node::Pluto.ReactiveNode, expr_cache::Pluto.ExprAnalysisCache; user_requested_run::Bool, capture_stdout::Bool)\n        @ Pluto ~/git/Pluto.jl/src/evaluation/Run.jl:290\n      
    [8] run_reactive_core!(session::Pluto.ServerSession, notebook::Pluto.Notebook, old_topology::Pluto.NotebookTopology, new_topology::Pluto.NotebookTopology, roots::Vector{Pluto.Cell}; deletion_hook::typeof(Pluto.WorkspaceManager.move_vars), user_requested_run::Bool, already_run::Vector{Pluto.Cell}, bond_value_pairs::Base.Iterators.Zip{Tuple{Vector{Symbol}, Vector{Any}}})\n        @ Pluto ~/git/Pluto.jl/src/evaluation/Run.jl:178\n      
    [9] run_reactive_core!\n        @ ~/git/Pluto.jl/src/evaluation/Run.jl:61 [inlined]\n     
    [10] (::Pluto.var\"#255#259\"{Bool, Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, Pluto.ServerSession, Pluto.Notebook})()\n        @ Pluto ~/git/Pluto.jl/src/evaluation/Run.jl:597\n     
    [11] withtoken(f::Pluto.var\"#255#259\"{Bool, Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, Pluto.ServerSession, Pluto.Notebook}, token::Pluto.Token)\n        @ Pluto ~/git/Pluto.jl/src/evaluation/Tokens.jl:19\n     
    [12] #254\n        @ ~/git/Pluto.jl/src/evaluation/Run.jl:593 [inlined]\n     
    [13] macro expansion\n        @ ~/git/Pluto.jl/src/evaluation/Tokens.jl:58 [inlined]\n     
    [14] (::Pluto.var\"#225#226\"{Pluto.var\"#254#258\"{Bool, Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, Pluto.ServerSession, Pluto.Notebook}})()\n        @ Pluto ./task.jl:482", :stacktrace => Dict{Symbol, Any}[Dict(:call => "wait", :inlined => true, :file => "task.jl", :line => 345, :path => "./task.jl"), Dict(:call => "fetch", :inlined => true, :file => "task.jl", :line => 360, :path => "./task.jl"), Dict(:call => "maybe_async(::Pluto.var\"#254#258\"{Bool, Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, Pluto.ServerSession, Pluto.Notebook}, ::Bool)", :inlined => false, :file => "Run.jl", :line => 247, :path => "/home/rik/git/Pluto.jl/src/evaluation/Run.jl"), Dict(:call => "var\"#update_save_run!#251\"(::Bool, ::Bool, ::Bool, ::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}, ::typeof(Pluto.update_save_run!), ::Pluto.ServerSession, ::Pluto.Notebook, ::Vector{Pluto.Cell})", :inlined => false, :file => "Run.jl", :line => 592, :path => "/home/rik/git/Pluto.jl/src/evaluation/Run.jl"), Dict(:call => "var\"#open#2\"(::Bool, ::Nothing, ::Bool, ::Base.UUID, ::typeof(Pluto.SessionActions.open), ::Pluto.ServerSession, ::String)", :inlined => false, :file => "SessionActions.jl", :line => 73, :path => "/home/rik/git/Pluto.jl/src/webserver/SessionActions.jl"), Dict(:call => "top-level scope", :inlined => true, :file => "Surprising revelation 3.jl#==#83d63f92-df6f-11ec-2ffc-f55f0d0092e5", :line => 1, :path => "/home/rik/.julia/pluto_notebooks/Surprising revelation 3.jl#==#83d63f92-df6f-11ec-2ffc-f55f0d0092e5")])

EDIT: Ah nevermind. This is Main.PlutoRunner.RichObject != Pluto.PlutoRunner.RichObject because of distributed.
EDIT2: Asked question at https://discourse.julialang.org/t/pass-struct-from-module-to-distributed-worker/81905.
EDIT3: This PR could possibly be fixed by #1881 since that could solve Pluto.PlutoRunner.Rich != Main.PlutoRunner.Rich.

@fonsp
Copy link
Owner

fonsp commented Jun 1, 2022

I should have documented this somewhere 😭, but I have been purposefully restricting the communication between the Pluto runner and the Pluto server to Base Julia types only, to avoid difficult serialization problems. This will also make the transition to our own Distributed easier.

@rikhuijzer
Copy link
Collaborator Author

I should have documented this somewhere 😭

No problem. It was a good learning experience 👍

@fonsp
Copy link
Owner

fonsp commented Jun 2, 2022

997f313 :)

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants