Skip to content

chore: improve internal performance of effect runtime #10999

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 31, 2024
Merged

chore: improve internal performance of effect runtime #10999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
61e4c19
chore: improve internal performance of effect runtime
trueadm Mar 30, 2024
04a84ba
add TODOs
trueadm Mar 30, 2024
09703ae
add TODOs
trueadm Mar 30, 2024
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
2 changes: 2 additions & 0 deletions packages/svelte/src/internal/client/dom/blocks/each.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,8 @@ function reconcile_tracked_array(array, state, anchor, render_fn, flags, keys) {
});
}

// TODO: would be good to avoid this closure in the case where we have no
// transitions at all. It would make it far more JIT friendly in the hot cases.
pause_effects(to_destroy, () => {
state.items = b_items;
});
Expand Down
20 changes: 13 additions & 7 deletions packages/svelte/src/internal/client/reactivity/effects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,8 @@ export function destroy_effect(effect) {
}
}

effect.first =
effect.last =
effect.next =
// `first` and `child` are nulled out in destroy_effect_children
effect.next =
effect.prev =
effect.teardown =
effect.ctx =
Expand Down Expand Up @@ -318,14 +317,17 @@ export function pause_effect(effect, callback = noop) {
export function pause_effects(effects, callback = noop) {
/** @type {import('#client').TransitionManager[]} */
var transitions = [];
var length = effects.length;

for (var effect of effects) {
pause_children(effect, transitions, true);
for (var i = 0; i < length; i++) {
pause_children(effects[i], transitions, true);
}

// TODO: would be good to avoid this closure in the case where we have no
// transitions at all. It would make it far more JIT friendly in the hot cases.
out(transitions, () => {
for (var effect of effects) {
destroy_effect(effect);
for (var i = 0; i < length; i++) {
destroy_effect(effects[i]);
}
callback();
});
Expand Down Expand Up @@ -370,6 +372,8 @@ function pause_children(effect, transitions, local) {
var sibling = child.next;
var transparent = (child.f & IS_ELSEIF) !== 0 || (child.f & BRANCH_EFFECT) !== 0;
// TODO we don't need to call pause_children recursively with a linked list in place
// it's slightly more involved though as we have to account for `transparent` changing
// through the tree.
pause_children(child, transitions, transparent ? local : false);
child = sibling;
}
Expand Down Expand Up @@ -404,6 +408,8 @@ function resume_children(effect, local) {
var sibling = child.next;
var transparent = (child.f & IS_ELSEIF) !== 0 || (child.f & BRANCH_EFFECT) !== 0;
// TODO we don't need to call resume_children recursively with a linked list in place
// it's slightly more involved though as we have to account for `transparent` changing
// through the tree.
resume_children(child, transparent ? local : false);
child = sibling;
}
Expand Down
118 changes: 69 additions & 49 deletions packages/svelte/src/internal/client/runtime.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,72 +484,92 @@ export function schedule_effect(signal) {

/**
*
* This function recursively collects effects in topological order from the starting effect passed in.
* Effects will be collected when they match the filtered bitwise flag passed in only. The collected
* array will be populated with all the effects.
*
* In an ideal world, we could just execute effects as we encounter them using this approach. However,
* this isn't possible due to how effects in Svelte are modelled to be possibly side-effectful. Thus,
* executing an effect might invalidate other parts of the tree, which means this this tree walking function
* will possibly pick up effects that are dirty too soon.
* This function both runs render effects and collects user effects in topological order
* from the starting effect passed in. Effects will be collected when they match the filtered
* bitwise flag passed in only. The collected effects array will be populated with all the user
* effects to be flushed.
*
* @param {import('./types.js').Effect} effect
* @param {number} filter_flags
* @param {boolean} shallow
* @param {import('./types.js').Effect[]} collected_user
* @param {import('./types.js').Effect[]} collected_effects
* @returns {void}
*/
function recursively_process_effects(effect, filter_flags, shallow, collected_user) {
var current_child = effect.first;
var user = [];

while (current_child !== null) {
var child = current_child;
current_child = child.next;
var flags = child.f;
var is_inactive = (flags & (DESTROYED | INERT)) !== 0;
if (is_inactive) continue;
function process_effects(effect, filter_flags, shallow, collected_effects) {
var current_effect = effect.first;
var effects = [];

main_loop: while (current_effect !== null) {
var flags = current_effect.f;
// TODO: we probably don't need to check for destroyed as it shouldn't be encountered?
var is_active = (flags & (DESTROYED | INERT)) === 0;
var is_branch = flags & BRANCH_EFFECT;
var is_clean = (flags & CLEAN) !== 0;
var child = current_effect.first;

if (is_branch) {
// Skip this branch if it's clean
if (is_clean) continue;
set_signal_status(child, CLEAN);
}

if ((flags & RENDER_EFFECT) !== 0) {
// Skip this branch if it's clean
if (is_active && (!is_branch || !is_clean)) {
if (is_branch) {
if (shallow) continue;
// TODO we don't need to call recursively_process_effects recursively with a linked list in place
recursively_process_effects(child, filter_flags, false, collected_user);
} else {
if (check_dirtiness(child)) {
execute_effect(child);
set_signal_status(current_effect, CLEAN);
}

if ((flags & RENDER_EFFECT) !== 0) {
if (is_branch) {
if (!shallow && child !== null) {
current_effect = child;
continue;
}
} else {
if (check_dirtiness(current_effect)) {
execute_effect(current_effect);
// Child might have been mutated since running the effect
child = current_effect.first;
}
if (!shallow && child !== null) {
current_effect = child;
continue;
}
}
} else if ((flags & EFFECT) !== 0) {
if (is_branch || is_clean) {
if (!shallow && child !== null) {
current_effect = child;
continue;
}
} else {
effects.push(current_effect);
}
// TODO we don't need to call recursively_process_effects recursively with a linked list in place
recursively_process_effects(child, filter_flags, false, collected_user);
}
} else if ((flags & EFFECT) !== 0) {
if (is_branch || is_clean) {
if (shallow) continue;
// TODO we don't need to call recursively_process_effects recursively with a linked list in place
recursively_process_effects(child, filter_flags, false, collected_user);
} else {
user.push(child);
}
var sibling = current_effect.next;

if (sibling === null) {
let parent = current_effect.parent;

while (parent !== null) {
if (effect === parent) {
break main_loop;
}
var parent_sibling = parent.next;
if (parent_sibling !== null) {
current_effect = parent_sibling;
continue main_loop;
}
parent = parent.parent;
}
}

current_effect = sibling;
}

if (user.length > 0) {
if (effects.length > 0) {
if ((filter_flags & EFFECT) !== 0) {
collected_user.push(...user);
collected_effects.push(...effects);
}

if (!shallow) {
for (var i = 0; i < user.length; i++) {
// TODO we don't need to call recursively_process_effects recursively with a linked list in place
recursively_process_effects(user[i], filter_flags, false, collected_user);
for (var i = 0; i < effects.length; i++) {
process_effects(effects[i], filter_flags, false, collected_effects);
}
}
}
Expand All @@ -568,7 +588,7 @@ function recursively_process_effects(effect, filter_flags, shallow, collected_us
*/
function flush_nested_effects(effect, filter_flags, shallow = false) {
/** @type {import('#client').Effect[]} */
var user_effects = [];
var collected_effects = [];

var previously_flushing_effect = is_flushing_effect;
is_flushing_effect = true;
Expand All @@ -578,8 +598,8 @@ function flush_nested_effects(effect, filter_flags, shallow = false) {
if (effect.first === null && (effect.f & BRANCH_EFFECT) === 0) {
flush_queued_effects([effect]);
} else {
recursively_process_effects(effect, filter_flags, shallow, user_effects);
flush_queued_effects(user_effects);
process_effects(effect, filter_flags, shallow, collected_effects);
flush_queued_effects(collected_effects);
}
} finally {
is_flushing_effect = previously_flushing_effect;
Expand Down