tc39 / proposal-signals

A proposal to add signals to JavaScript.
MIT License
2.87k stars 54 forks source link

Suggestion: don't recompute when dependent signals come back to previous values used for last computation #197

Open divdavem opened 3 weeks ago

divdavem commented 3 weeks ago

Hello, I am a maintainer of the tansu signal library. As I was exploring how to re-implement tansu with the signal-polyfill package (as I am hoping it will bring better interoperability with other signal libraries), I came across the following simple use case which does not match my expectations (and does not match either the behavior we implemented in tansu). I am opening this PR to discuss it. It does not (yet) contain any fix, just the test cases. One fails and the other one succeeds:

Here is the failing use case:

let n = 0;
const s = new Signal.State(0);
const c = new Signal.Computed(() => (n++, s.get()));
c.get(); // this triggers a computation of c (so n passes from 0 to 1)
s.set(1); // this does not trigger anything because the computation of c is lazy
s.set(0); // let's come back to the previous value of s
c.get(); // if we recompute c, there is no need to call the function as the last time c was computed was with s = 0
expect(n).toBe(1); // so I am expecting n to still be 1
// but this test fails: there is an (unneeded) re-computation

Here is a small variation of the previous case, with an extra intermediate computed signal, which prevents the re-computation of c:

let n = 0;
const s = new Signal.State(0);
const extra = new Signal.Computed(() => s.get());
const c = new Signal.Computed(() => (n++, extra.get()));
c.get(); // this triggers a computation of c (so n passes from 0 to 1)
s.set(1); // this does not trigger anything because the computation of c is lazy
s.set(0); // let's come back to the previous value of s
c.get(); // if we recompute c, there is no need to call the function as the last time c was computed was with s = 0
expect(n).toBe(1); // so I am expecting n to still be 1
// this test succeeds

I believe we should not have to add intermediate computed signals to prevent unneeded re-computations. In addition to checking the version number of dependent signals, I was expecting the algorithm to also compare the values with the previous values (using the provided equal function) before calling the re-computation function.

What do you think?

littledan commented 5 days ago

This is an interesting comparison case. Currently, comparison is done "on the way out" rather than "on the way in", so there's no way to catch such a non-change. I don't really know how to evaluate the pros and cons of these approaches. Can you tell me more about where such a case has come up for you in practice, or what motivated you to implement tansu this way?

shaylew commented 5 days ago

I think the "use an extra computed signal" trick exposes basically exactly the performance tradeoff here: if you're willing to hold onto previous values values (at a memory cost) and run equality comparisons more often (at an execution time cost) you can potentially rerun computed bodies less often.

It might not be pretty/performant but I think you can actually build this behavior into a subclass of State/Computeds by attaching a WeakMap<Computed<any>, Computed<T>> to each Signal<T>, and then override get so it uses currentComputed() to look up the right intermediary computed for the current reader.

(Though this is one thing I think is cleaner with an "intercept tracked reads" primitive. You'd then be able to make a TansuComputed that created intermediaries for anything it reads, whether or not those signals were built with tansu, rather than making specific subclasses that create intermediaries when read.)