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

feat: push-pull-push model 🚀 #19

Merged
merged 1 commit into from
Dec 22, 2024
Merged

Conversation

johnsoncodehk
Copy link
Collaborator

@johnsoncodehk johnsoncodehk commented Dec 22, 2024

After implementing #14, I've been wondering if that could cause memory issues. I can think of some memory leaks caused by deliberate behavior, but I also think that in most cases it should not cause user-perceivable problems.

Still, I don't like that approach, saving the value on each link makes me feel insecure. Finally I discovered that we can have a
push-pull-push model solution that doesn't use version/value at all.

The new push-pull-push model is very similar to the approach adopted by vuejs/core#5912.

  • First push: signal/computed pushes dirty/toCheckDirty flag to subscribers
  • Second pull: computed/effect pull updates from toCheckDirty dependencies when executed
  • Final push: When computed is pulled, if the value is updated, pushes dirty flag to shallow subscribers

In this method, we don't need to use intermediate variables to detect changes, because all flag updates are immediate as needed.

Memory Usage improvement

Since Link does not need to store value, the memory usage is reduced by 3% (2434.72KB -> 2363.90KB) in the following test.

const start = process.memoryUsage().heapUsed;

const w = 100;
const h = 100;
const src = signal(1);

for (let i = 0; i < w; i++) {
	let last = src;
	for (let j = 0; j < h; j++) {
		const prev = last;
		last = computed(() => prev.get() + 1);
	}
	effect(() => last.get());
}

src.set(src.get() + 1);

console.log(`Memory Usage: ${((process.memoryUsage().heapUsed - start) / 1024).toFixed(2)} KB`);

Benchmark Results

main bench

benchmark avg min p75 p99 max
propagate: 1 * 1 1.43 µs/iter 1.42 µs 1.43 µs 1.45 µs 1.45 µs
propagate: 1 * 10 7.40 µs/iter 7.38 µs 7.41 µs 7.42 µs 7.42 µs
propagate: 1 * 100 65.68 µs/iter 64.79 µs 65.58 µs 69.79 µs 101.50 µs
propagate: 10 * 1 12.85 µs/iter 12.84 µs 12.85 µs 12.85 µs 12.85 µs
propagate: 10 * 10 72.23 µs/iter 71.38 µs 72.13 µs 75.88 µs 104.50 µs
propagate: 10 * 100 653.37 µs/iter 649.33 µs 654.33 µs 673.13 µs 703.71 µs
propagate: 100 * 1 126.44 µs/iter 125.21 µs 126.17 µs 134.21 µs 156.04 µs
propagate: 100 * 10 717.02 µs/iter 712.42 µs 717.71 µs 741.50 µs 765.29 µs
propagate: 100 * 100 6.51 ms/iter 6.49 ms 6.51 ms 6.60 ms 6.61 ms

This PR

benchmark avg min p75 p99 max
propagate: 1 * 1 1.43 µs/iter 1.42 µs 1.43 µs 1.44 µs 1.47 µs
propagate: 1 * 10 7.17 µs/iter 7.14 µs 7.18 µs 7.19 µs 7.20 µs
propagate: 1 * 100 63.22 µs/iter 63.15 µs 63.25 µs 63.26 µs 63.31 µs
propagate: 10 * 1 12.78 µs/iter 12.77 µs 12.79 µs 12.79 µs 12.79 µs
propagate: 10 * 10 70.18 µs/iter 69.38 µs 70.00 µs 75.04 µs 95.21 µs
propagate: 10 * 100 640.52 µs/iter 630.67 µs 644.63 µs 653.75 µs 674.08 µs
propagate: 100 * 1 125.57 µs/iter 124.13 µs 125.33 µs 136.38 µs 155.38 µs
propagate: 100 * 10 697.68 µs/iter 693.50 µs 698.25 µs 720.67 µs 755.00 µs
propagate: 100 * 100 6.32 ms/iter 6.30 ms 6.32 ms 6.38 ms 6.44 ms

@johnsoncodehk johnsoncodehk merged commit 419f855 into master Dec 22, 2024
6 checks passed
@johnsoncodehk johnsoncodehk deleted the push-pull-push-model branch December 22, 2024 20:09
medz added a commit to medz/alien-signals-dart that referenced this pull request Dec 23, 2024
@medz medz mentioned this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant