Skip to content

Commit

Permalink
New heuristic to skip intermediate bond values (#1892)
Browse files Browse the repository at this point in the history
  • Loading branch information
fonsp authored Feb 4, 2022
1 parent e2f955e commit e73c11a
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 12 deletions.
59 changes: 49 additions & 10 deletions frontend/components/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -651,15 +651,36 @@ patch: ${JSON.stringify(
const message = update.message
switch (update.type) {
case "notebook_diff":
let apply_promise = Promise.resolve()
if (message?.response?.from_reset) {
console.log("Trying to reset state after failure")
apply_notebook_patches(message.patches, initial_notebook()).catch((e) => {
apply_promise = apply_notebook_patches(message.patches, initial_notebook()).catch((e) => {
alert("Oopsie!! please refresh your browser and everything will be alright!")
throw e
})
} else if (message.patches.length !== 0) {
apply_notebook_patches(message.patches)
apply_promise = apply_notebook_patches(message.patches)
}

const set_waiting = () => {
let from_update = message?.response?.update_went_well != null
let is_just_acknowledgement = from_update && message.patches.length === 0
// console.log("Received patches!", message.patches, message.response, is_just_acknowledgement)

if (!is_just_acknowledgement) {
this.waiting_for_bond_to_trigger_execution = false
}
}
apply_promise
.then(set_waiting)
.catch((e) => {
set_waiting()
throw e
})
.then(() => {
this.send_queued_bond_changes()
})

break
default:
console.error("Received unknown update type!", update)
Expand Down Expand Up @@ -788,6 +809,8 @@ patch: ${JSON.stringify(
})
}
}
/** Whether we just set a bond value which will trigger a cell to run, but we are still waiting for the server to process the bond value (and run the cell). See https://github.com/fonsp/Pluto.jl/issues/1891 for more info. */
this.waiting_for_bond_to_trigger_execution = false
/** Number of local updates that have not yet been applied to the server's state. */
this.pending_local_updates = 0
this.js_init_set = new SetWithEmptyCallback(() => {
Expand All @@ -797,6 +820,7 @@ patch: ${JSON.stringify(
/** Is the notebook ready to execute code right now? (i.e. are no cells queued or running?) */
this.notebook_is_idle = () => {
return !(
this.waiting_for_bond_to_trigger_execution ||
this.pending_local_updates > 0 ||
// a cell is running:
Object.values(this.state.notebook.cell_results).some((cell) => cell.running || cell.queued) ||
Expand All @@ -808,6 +832,15 @@ patch: ${JSON.stringify(
this.is_process_ready = () =>
this.state.notebook.process_status === ProcessStatus.starting || this.state.notebook.process_status === ProcessStatus.ready

let bond_will_trigger_evaluation = (/** @type {string|PropertyKey} */ sym) =>
Object.entries(this.state.notebook.cell_dependencies).some(([cell_id, deps]) => {
// if the other cell depends on the variable `sym`...
if (deps.upstream_cells_map.hasOwnProperty(sym)) {
// and the cell is not disabled
return !(this.state.notebook.cell_inputs[cell_id]?.running_disabled ?? true)
}
})

let last_update_notebook_task = Promise.resolve()
/** @param {(notebook: NotebookData) => void} mutate_fn */
let update_notebook = (mutate_fn) => {
Expand All @@ -821,13 +854,13 @@ patch: ${JSON.stringify(
mutate_fn(notebook)
})

// If "notebook is not idle" I seperate and store the bonds updates,
// If "notebook is not idle" we seperate and store the bonds updates,
// to send when the notebook is idle. This delays the updating of the bond for performance,
// but when the server can discard bond updates itself (now it executes them one by one, even if there is a newer update ready)
// this will no longer be necessary
// console.log(`this.notebook_is_idle():`, this.notebook_is_idle())
if (!this.notebook_is_idle()) {
let changes_involving_bonds = changes.filter((x) => x.path[0] === "bonds")
let is_idle = this.notebook_is_idle()
let changes_involving_bonds = changes.filter((x) => x.path[0] === "bonds")
if (!is_idle) {
this.bonds_changes_to_apply_when_done = [...this.bonds_changes_to_apply_when_done, ...changes_involving_bonds]
changes = changes.filter((x) => x.path[0] !== "bonds")
}
Expand All @@ -838,18 +871,24 @@ patch: ${JSON.stringify(
console.log(`Changes to send to server from "${previous_function_name}":`, changes)
} catch (error) {}
}
if (changes.length === 0) {
return
}

for (let change of changes) {
if (change.path.some((x) => typeof x === "number")) {
throw new Error("This sounds like it is editing an array...")
}
}

if (changes.length === 0) {
return
}
if (is_idle) {
this.waiting_for_bond_to_trigger_execution ||= changes_involving_bonds.some(
(x) => x.path.length >= 1 && bond_will_trigger_evaluation(x.path[1])
)
}
this.pending_local_updates++
this.on_patches_hook(changes)
try {
// console.log("Sending changes to server:", changes)
await Promise.all([
this.client.send("update_notebook", { updates: changes }, { notebook_id: this.state.notebook.notebook_id }, false).then((response) => {
if (response.message?.response?.update_went_well === "πŸ‘Ž") {
Expand Down
9 changes: 8 additions & 1 deletion src/evaluation/RunBonds.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
function set_bond_values_reactive(; session::ServerSession, notebook::Notebook, bound_sym_names::AbstractVector{Symbol}, is_first_values::AbstractVector{Bool}=[true for x in bound_sym_names], kwargs...)::Union{Task,TopologicalOrder}
function set_bond_values_reactive(;
session::ServerSession, notebook::Notebook,
bound_sym_names::AbstractVector{Symbol},
is_first_values::AbstractVector{Bool}=[true for x in bound_sym_names],
initiator=nothing,
kwargs...
)::Union{Task,TopologicalOrder}
# filter out the bonds that don't need to be set
to_set = filter(zip(bound_sym_names, is_first_values) |> collect) do (bound_sym, is_first_value)
new_value = notebook.bonds[bound_sym].value
Expand All @@ -25,6 +31,7 @@ function set_bond_values_reactive(; session::ServerSession, notebook::Notebook,
end .|> first

if isempty(to_set) || !will_run_code(notebook)
send_notebook_changes!(ClientRequest(; session, notebook, initiator))
return TopologicalOrder(notebook.topology, Cell[], Dict{Cell, ReactivityError}())
end

Expand Down
3 changes: 2 additions & 1 deletion src/webserver/Dynamic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -327,10 +327,11 @@ responses[:update_notebook] = function response_update_notebook(πŸ™‹::ClientRequ
bound_sym_names=bound_sym_names,
is_first_values=is_first_values,
run_async=true,
initiator=πŸ™‹.initiator,
)
end

send_notebook_changes!(πŸ™‹; commentary=Dict(:update_went_well => :πŸ‘))
send_notebook_changes!(πŸ™‹; commentary=Dict(:update_went_well => :πŸ‘))
catch ex
@error "Update notebook failed" πŸ™‹.body["updates"] exception=(ex, stacktrace(catch_backtrace()))
response = Dict(
Expand Down

0 comments on commit e73c11a

Please sign in to comment.