msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

chore: refactor internal signal dependency heuristic (#12881)

Browse Source 
* chore: remove redundant signal logic

* more tweaks

* more tweaks

* refactor

* tweak
pull/12889/head
Dominic Gannaway 1 year ago committed by GitHub
parent 4512462b66
commit 817558828e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 42 additions and 60 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/hot-kangaroos-invite.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
chore: refactor internal signal dependency heuristic

2
packages/svelte/src/internal/client/dom/template.js
Unescape View File

@ -39,7 +39,7 @@ export function template(content, flags) {
return hydrate_node;
}
if (!node) {
if (node === undefined) {
node = create_fragment_from_html(has_start ? content : '<!>' + content);
if (!is_fragment) node = /** @type {Node} */ (get_first_child(node));
}

6
packages/svelte/src/internal/client/reactivity/effects.js vendored
Unescape View File

@ -375,8 +375,10 @@ export function destroy_effect(effect, remove_dom = true) {
remove_reactions(effect, 0);
set_signal_status(effect, DESTROYED);
if (effect.transitions) {
for (const transition of effect.transitions) {
var transitions = effect.transitions;
if (transitions !== null) {
for (const transition of transitions) {
transition.stop();
}
}

4
packages/svelte/src/internal/client/reactivity/sources.js
Unescape View File

@ -139,8 +139,10 @@ function mark_reactions(signal, status) {
if (reactions === null) return;
var runes = is_runes();
var length = reactions.length;
for (var reaction of reactions) {
for (var i = 0; i < length; i++) {
var reaction = reactions[i];
var flags = reaction.f;
// Skip any effects that are already dirty

2
packages/svelte/src/internal/client/reactivity/types.d.ts vendored
Unescape View File

@ -9,7 +9,7 @@ export interface Signal {
export interface Value<V = unknown> extends Signal {
/** Signals that read from this signal */
reactions: null | Set<Reaction>;
reactions: null | Reaction[];
/** Equality function */
equals: Equals;
/** The latest value for this signal */

67
packages/svelte/src/internal/client/runtime.js
Unescape View File

@ -169,7 +169,7 @@ export function check_dirtiness(reaction) {
if ((flags & DISCONNECTED) !== 0) {
for (i = 0; i < dependencies.length; i++) {
(dependencies[i].reactions ??= new Set()).add(reaction);
(dependencies[i].reactions ??= []).push(reaction);
}
reaction.f ^= DISCONNECTED;
@ -188,11 +188,11 @@ export function check_dirtiness(reaction) {
if (is_unowned) {
// TODO is there a more logical place to do this work?
if (!current_skip_reaction && !dependency?.reactions?.has(reaction)) {
if (!current_skip_reaction && !dependency?.reactions?.includes(reaction)) {
// If we are working with an unowned signal as part of an effect (due to !current_skip_reaction)
// and the version hasn't changed, we still need to check that this reaction
// if linked to the dependency source – otherwise future updates will not be caught.
(dependency.reactions ??= new Set()).add(reaction);
(dependency.reactions ??= []).push(reaction);
}
}
}
@ -291,26 +291,9 @@ export function update_reaction(reaction) {
var deps = reaction.deps;
if (new_deps !== null) {
var dependency;
var i;
if (deps !== null) {
/** All dependencies of the reaction, including those tracked on the previous run */
var array = skipped_deps === 0 ? new_deps : deps.slice(0, skipped_deps).concat(new_deps);
// If we have more than 16 elements in the array then use a Set for faster performance
// TODO: evaluate if we should always just use a Set or not here?
var set = array.length > 16 ? new Set(array) : null;
// Remove dependencies that should no longer be tracked
for (i = skipped_deps; i < deps.length; i++) {
dependency = deps[i];
if (set !== null ? !set.has(dependency) : !array.includes(dependency)) {
remove_reaction(reaction, dependency);
}
}
}
remove_reactions(reaction, skipped_deps);
if (deps !== null && skipped_deps > 0) {
deps.length = skipped_deps + new_deps.length;
@ -323,7 +306,7 @@ export function update_reaction(reaction) {
if (!current_skip_reaction) {
for (i = skipped_deps; i < deps.length; i++) {
(deps[i].reactions ??= new Set()).add(reaction);
(deps[i].reactions ??= []).push(reaction);
}
}
} else if (deps !== null && skipped_deps < deps.length) {
@ -350,9 +333,16 @@ export function update_reaction(reaction) {
function remove_reaction(signal, dependency) {
let reactions = dependency.reactions;
if (reactions !== null) {
reactions.delete(signal);
if (reactions.size === 0) {
reactions = dependency.reactions = null;
var index = reactions.indexOf(signal);
if (index !== -1) {
var new_length = reactions.length - 1;
if (new_length === 0) {
reactions = dependency.reactions = null;
} else {
// Swap with last element and then remove.
reactions[index] = reactions[new_length];
reactions.pop();
}
}
}
// If the derived has no reactions, then we can disconnect it from the graph,
@ -377,19 +367,8 @@ export function remove_reactions(signal, start_index) {
var dependencies = signal.deps;
if (dependencies === null) return;
var active_dependencies = start_index === 0 ? null : dependencies.slice(0, start_index);
var seen = new Set();
for (var i = start_index; i < dependencies.length; i++) {
var dependency = dependencies[i];
if (seen.has(dependency)) continue;
seen.add(dependency);
// Avoid removing a reaction if we know that it is active (start_index will not be 0)
if (active_dependencies === null || !active_dependencies.includes(dependency)) {
remove_reaction(signal, dependency);
}
remove_reaction(signal, dependencies[i]);
}
}
@ -726,16 +705,10 @@ export function get(signal) {
// rather than updating `new_deps`, which creates GC cost
if (new_deps === null && deps !== null && deps[skipped_deps] === signal) {
skipped_deps++;
}
// Otherwise, create or push to `new_deps`, but only if this
// dependency wasn't the last one that was accessed
else if (deps === null || skipped_deps === 0 || deps[skipped_deps - 1] !== signal) {
if (new_deps === null) {
new_deps = [signal];
} else if (new_deps[new_deps.length - 1] !== signal) {
new_deps.push(signal);
}
} else if (new_deps === null) {
new_deps = [signal];
} else {
new_deps.push(signal);
}
if (

16
packages/svelte/tests/signals/test.ts
Unescape View File

@ -236,13 +236,13 @@ describe('signals', () => {
return () => {
flushSync(() => set(count, 1));
// Ensure we're not leaking consumers
assert.deepEqual(count.reactions?.size, 1);
assert.deepEqual(count.reactions?.length, 1);
flushSync(() => set(count, 2));
// Ensure we're not leaking consumers
assert.deepEqual(count.reactions?.size, 1);
assert.deepEqual(count.reactions?.length, 1);
flushSync(() => set(count, 3));
// Ensure we're not leaking consumers
assert.deepEqual(count.reactions?.size, 1);
assert.deepEqual(count.reactions?.length, 1);
assert.deepEqual(log, [0, 1, 2, 3]);
};
});
@ -304,8 +304,8 @@ describe('signals', () => {
flushSync(() => set(count, 4));
flushSync(() => set(count, 0));
// Ensure we're not leaking consumers
assert.deepEqual(count.reactions?.size, 1);
assert.deepEqual(calc.reactions?.size, 1);
assert.deepEqual(count.reactions?.length, 2);
assert.deepEqual(calc.reactions?.length, 1);
assert.deepEqual(log, [0, 2, 'limit', 0]);
destroy();
// Ensure we're not leaking consumers
@ -484,7 +484,7 @@ describe('signals', () => {
return () => {
flushSync();
assert.equal(a?.deps?.length, 1);
assert.equal(state?.reactions?.size, 1);
assert.equal(state?.reactions?.length, 1);
destroy();
assert.equal(a?.deps?.length, 1);
assert.equal(state?.reactions, null);
@ -658,12 +658,12 @@ describe('signals', () => {
});
assert.equal($.get(d), 0);
assert.equal(count.reactions?.size, 1);
assert.equal(count.reactions?.length, 1);
assert.equal(d.deps?.length, 1);
set(count, 1);
assert.equal($.get(d), 2);
assert.equal(count.reactions?.size, 1);
assert.equal(count.reactions?.length, 1);
assert.equal(d.deps?.length, 1);
destroy();

Write Preview
Loading…
Cancel
Save