Skip to content

chore: tune signals for better runtime perf #9529

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 1 commit into from
Nov 18, 2023
Merged

chore: tune signals for better runtime perf #9529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions .changeset/thirty-ghosts-fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

chore: tweak signals for better runtime perf
27 changes: 7 additions & 20 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 @@ -55,9 +55,6 @@ export let current_effect = null;
/** @type {null | import('./types.js').Signal[]} */
let current_dependencies = null;
let current_dependencies_index = 0;
// Used to prevent over-subscribing dependencies on a consumer
let current_consumer_read_clock = 1;
let current_read_clock = 1;
// Handling capturing of signals from object property getters
let current_should_capture_signal = false;
/** If `true`, `get`ting the signal should not register it as a dependency */
Expand Down Expand Up @@ -153,7 +150,6 @@ function create_signal_object(flags, value, block) {
equals: null,
flags,
init: null,
read: 0,
references: null,
value
};
Expand Down Expand Up @@ -190,13 +186,15 @@ function is_signal_dirty(signal) {
let i;
for (i = 0; i < length; i++) {
const dependency = dependencies[i];
const dep_flags = dependency.flags;

if ((dependency.flags & MAYBE_DIRTY) !== 0 && !is_signal_dirty(dependency)) {
if ((dep_flags & MAYBE_DIRTY) !== 0 && !is_signal_dirty(dependency)) {
set_signal_status(dependency, CLEAN);
continue;
}
if ((dependency.flags & DIRTY) !== 0 || dependency.value === UNINITIALIZED) {
if ((dependency.flags & DERIVED) !== 0) {
// The flags can be marked as dirty from the above is_signal_dirty call.
if ((dependency.flags & DIRTY) !== 0) {
if ((dep_flags & DERIVED) !== 0) {
update_derived(dependency, true);
// Might have been mutated from above get.
if ((signal.flags & DIRTY) !== 0) {
Expand All @@ -221,7 +219,6 @@ function execute_signal_fn(signal) {
const init = signal.init;
const previous_dependencies = current_dependencies;
const previous_dependencies_index = current_dependencies_index;
const previous_consumer_read_clock = current_consumer_read_clock;
const previous_consumer = current_consumer;
const previous_block = current_block;
const previous_component_context = current_component_context;
Expand All @@ -230,12 +227,6 @@ function execute_signal_fn(signal) {
const previous_untracking = current_untracking;
current_dependencies = /** @type {null | import('./types.js').Signal[]} */ (null);
current_dependencies_index = 0;
if (current_read_clock === MAX_SAFE_INT) {
current_read_clock = 1;
} else {
current_read_clock++;
}
current_consumer_read_clock = current_read_clock;
current_consumer = signal;
current_block = signal.block;
current_component_context = signal.context;
Expand Down Expand Up @@ -290,7 +281,6 @@ function execute_signal_fn(signal) {
} finally {
current_dependencies = previous_dependencies;
current_dependencies_index = previous_dependencies_index;
current_consumer_read_clock = previous_consumer_read_clock;
current_consumer = previous_consumer;
current_block = previous_block;
current_component_context = previous_component_context;
Expand Down Expand Up @@ -427,7 +417,7 @@ function flush_queued_effects(effects) {
for (i = 0; i < length; i++) {
const signal = effects[i];
const flags = signal.flags;
if ((flags & DESTROYED) === 0 && (flags & INERT) === 0) {
if ((flags & (DESTROYED | INERT)) === 0) {
if (is_signal_dirty(signal)) {
set_signal_status(signal, CLEAN);
execute_effect(signal);
Expand Down Expand Up @@ -744,12 +734,9 @@ export function get(signal) {
current_dependencies_index++;
} else if (current_dependencies === null) {
current_dependencies = [signal];
} else if (signal.read !== current_consumer_read_clock) {
} else if (signal !== current_dependencies.at(-1)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't at like really slow compared to [.length - 1]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was faster?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.measurethat.net/Benchmarks/Show/17464/0/arrayarraylength---1-vs-arrayat-1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, .at is faster on that site, but I got 2.5s for .at and 670ms for .length - 1, when running this snippet in my console,

{
	let N = 1000000;
	const arr = Array.from({length: 100}).map(Math.random);
	let acc= 0;
	console.log(arr.at(-1), arr[arr.length - 1])

	for (var j = 1000; j--;) acc += arr.at(-1);
	for (var j = 1000; j--;) acc += arr[arr.length - 1];
	console.time();
	for (var i = N; i--;) { acc = 0; for (var j = 1000; j--;) acc += arr.at(-1); }
	console.timeEnd();

	console.time();
	for (var i = N; i--;) { acc = 0; for (var j = 1000; j--;) acc += arr[arr.length - 1]; }
	console.timeEnd();

    console.log(acc);
}

I read about it being slower somewhere because the method had to also consider ArrayLike objects

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESBench results with the same code from measurethat
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll put that change in the other PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

On the left side is at() function, and on the right side is length() function.

function at(i) {
  return i.at(-1);
}
function length(i) {
  return i[i.length - 1]
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the left side is slightly better.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that length() function should be faster, it will execute 0-6e instructions if nothing goes wrong like OOB, wrong object shape, etc. And at() function will execute 0-5d, jmp to 84-8c, jmp to 6a-78. But I am not 100% sure about it, just quickly looked at it :)

current_dependencies.push(signal);
}
if (!unowned) {
signal.read = current_consumer_read_clock;
}
}

if ((flags & DERIVED) !== 0 && is_signal_dirty(signal)) {
Expand Down
2 changes: 0 additions & 2 deletions packages/svelte/src/internal/client/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ export type Signal<V = unknown> = {
flags: SignalFlags;
/** The function that we invoke for effects and computeds */
init: null | (() => V) | (() => void | (() => void)) | ((b: Block) => void | (() => void));
/** The read clock from the given context the signal was read/written to */
read: number;
/** Anything that a signal owns */
references: null | Signal[];
/** The latest value for this signal, doubles as the teardown for effects */
Expand Down