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

Unregister Bonds at the start of the reactive run. #2467

Merged
merged 3 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/evaluation/RunBonds.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
function set_bond_values_reactive(;
session::ServerSession, notebook::Notebook,
bound_sym_names::AbstractVector{Symbol},
is_first_values::AbstractVector{Bool}=[false for x in bound_sym_names],
function set_bond_values_reactive(;
session::ServerSession, notebook::Notebook,
bound_sym_names::AbstractVector{Symbol},
is_first_values::AbstractVector{Bool}=[false for x in bound_sym_names],
initiator=nothing,
kwargs...
)::Union{Task,TopologicalOrder}
Expand Down Expand Up @@ -39,13 +39,23 @@ function set_bond_values_reactive(;

new_values = Any[notebook.bonds[bound_sym].value for bound_sym in syms_to_set]
bond_value_pairs = zip(syms_to_set, new_values)


syms_to_set_set = Set{Symbol}(syms_to_set)
function custom_deletion_hook((session, notebook)::Tuple{ServerSession,Notebook}, old_workspace_name, new_workspace_name, to_delete_vars::Set{Symbol}, methods_to_delete::Set{Tuple{UUID,FunctionName}}, to_reimport::Set{Expr}, invalidated_cell_uuids::Set{UUID}; to_run::AbstractVector{Cell})
to_delete_vars = Set([to_delete_vars..., syms_to_set...]) # also delete the bound symbols
WorkspaceManager.move_vars((session, notebook), old_workspace_name, new_workspace_name, to_delete_vars, methods_to_delete, to_reimport, invalidated_cell_uuids)
to_delete_vars = union(to_delete_vars, syms_to_set_set) # also delete the bound symbols
WorkspaceManager.move_vars(
(session, notebook),
old_workspace_name,
new_workspace_name,
to_delete_vars,
methods_to_delete,
to_reimport,
invalidated_cell_uuids,
syms_to_set_set,
)
set_bond_value_pairs!(session, notebook, zip(syms_to_set, new_values))
end
to_reeval = where_referenced(notebook, notebook.topology, Set{Symbol}(syms_to_set))
to_reeval = where_referenced(notebook, notebook.topology, syms_to_set_set)

run_reactive_async!(session, notebook, to_reeval; deletion_hook=custom_deletion_hook, save=false, user_requested_run=false, run_async=false, bond_value_pairs, kwargs...)
end
Expand Down
4 changes: 3 additions & 1 deletion src/evaluation/WorkspaceManager.jl
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,8 @@ function move_vars(
to_delete::Set{Symbol},
methods_to_delete::Set{Tuple{UUID,FunctionName}},
module_imports_to_move::Set{Expr},
invalidated_cell_uuids::Set{UUID};
invalidated_cell_uuids::Set{UUID},
keep_registered::Set{Symbol}=Set{Symbol}();
kwargs...
)

Expand All @@ -611,6 +612,7 @@ function move_vars(
$methods_to_delete,
$module_imports_to_move,
$invalidated_cell_uuids,
$keep_registered,
)
end)
end
Expand Down
18 changes: 10 additions & 8 deletions src/runner/PlutoRunner.jl
Original file line number Diff line number Diff line change
Expand Up @@ -529,9 +529,6 @@ function run_expression(
cell_published_objects[cell_id] = Dict{String,Any}()

# reset registered bonds
for s in get(cell_registered_bond_names, cell_id, Set{Symbol}())
delete!(registered_bond_elements, s)
end
cell_registered_bond_names[cell_id] = Set{Symbol}()

# If the cell contains macro calls, we want those macro calls to preserve their identity,
Expand Down Expand Up @@ -683,6 +680,7 @@ function move_vars(
methods_to_delete::Set{Tuple{UUID,Vector{Symbol}}},
module_imports_to_move::Set{Expr},
invalidated_cell_uuids::Set{UUID},
keep_registered::Set{Symbol},
)
old_workspace = getfield(Main, old_workspace_name)
new_workspace = getfield(Main, new_workspace_name)
Expand All @@ -706,6 +704,10 @@ function move_vars(
if (symbol ∈ vars_to_delete) || (symbol ∈ name_symbols_of_funcs_with_no_methods_left)
# var will be redefined - unreference the value so that GC can snoop it

if haskey(registered_bond_elements, symbol) && symbol ∉ keep_registered
delete!(registered_bond_elements, symbol)
end

# free memory for other variables
# & delete methods created in the old module:
# for example, the old module might extend an imported function:
Expand Down Expand Up @@ -2012,8 +2014,8 @@ struct Bond
Bond(element, defines::Symbol) = showable(MIME"text/html"(), element) ? new(element, defines, Base64.base64encode(rand(UInt8,9))) : error("""Can only bind to html-showable objects, ie types T for which show(io, ::MIME"text/html", x::T) is defined.""")
end

function create_bond(element, defines::Symbol)
push!(cell_registered_bond_names[currently_running_cell_id[]], defines)
function create_bond(element, defines::Symbol, cell_id::UUID)
push!(cell_registered_bond_names[cell_id], defines)
registered_bond_elements[defines] = element
Bond(element, defines)
end
Expand Down Expand Up @@ -2051,10 +2053,10 @@ The second cell will show the square of `x`, and is updated in real-time as the
macro bind(def, element)
if def isa Symbol
quote
$(load_integrations_if_needed)()
$(load_integrations_if_needed)()
local el = $(esc(element))
global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : $(initial_value_getter_ref)[](el)
PlutoRunner.create_bond(el, $(Meta.quot(def)))
global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : $(initial_value_getter_ref)[](el)
PlutoRunner.create_bond(el, $(Meta.quot(def)), $(GiveMeCellID()))
end
else
:(throw(ArgumentError("""\nMacro example usage: \n\n\t@bind my_number html"<input type='range'>"\n\n""")))
Expand Down
42 changes: 33 additions & 9 deletions test/Bonds.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ import Distributed
# 1
Cell("""
begin
import AbstractPlutoDingetjes
const APD = AbstractPlutoDingetjes
import AbstractPlutoDingetjes as APD
import AbstractPlutoDingetjes.Bonds
end
"""),
Expand Down Expand Up @@ -205,9 +204,13 @@ import Distributed
Cell("@bind pv4 PossibleValuesTest((x+1 for x in 1:10))"),
# 34
Cell("@bind pv5 PossibleValuesTest(1:10)"),

# 35 - https://github.com/fonsp/Pluto.jl/issues/2465
Cell(""),
Cell("@bind ts2465 TransformSlider()"),
Cell("ts2465"),
])



function set_bond_value(name, value, is_first_value=false)
notebook.bonds[name] = Dict("value" => value)
Pluto.set_bond_values_reactive(; session=🍭, notebook, bound_sym_names=[name],
Expand All @@ -225,8 +228,7 @@ import Distributed
@test notebook.cells[10].output.body == "missing"
set_bond_value(:x_simple, 1, true)
@test notebook.cells[10].output.body == "1"



update_run!(🍭, notebook, notebook.cells)

@test noerror(notebook.cells[1])
Expand Down Expand Up @@ -271,7 +273,7 @@ import Distributed
@test noerror(notebook.cells[32])
@test noerror(notebook.cells[33])
@test noerror(notebook.cells[34])
@test length(notebook.cells) == 34
@test length(notebook.cells) == 37


@test Pluto.possible_bond_values(🍭, notebook, :x_new) == [1,2,3]
Expand Down Expand Up @@ -326,8 +328,30 @@ import Distributed
@test notebook.cells[25].output.body == "1"
set_bond_value(:x_counter, 7, false)
@test notebook.cells[25].output.body == "2"



# https://github.com/fonsp/Pluto.jl/issues/2465
update_run!(🍭, notebook, notebook.cells[35:37])

@test noerror(notebook.cells[35])
@test noerror(notebook.cells[36])
@test noerror(notebook.cells[37])
@test notebook.cells[37].output.body == "\"x\""
@test isempty(notebook.cells[35].code)

# this should not deregister the TransformSlider
setcode!(notebook.cells[35], notebook.cells[36].code)
setcode!(notebook.cells[36], "")

update_run!(🍭, notebook, notebook.cells[35:36])
@test noerror(notebook.cells[35])
@test noerror(notebook.cells[36])
@test notebook.cells[37].output.body == "\"x\""

set_bond_value(:ts2465, 2, false)
@test noerror(notebook.cells[35])
@test noerror(notebook.cells[36])
@test notebook.cells[37].output.body == "\"xx\""

WorkspaceManager.unmake_workspace((🍭, notebook))
🍭.options.evaluation.workspace_use_distributed = false

Expand Down