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

[divdavem]

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]

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]

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.)

[divdavem]

@littledan @shaylew Thank you for your answers.

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 have noticed that. I think it is good (of course!) to do the comparison "on the way out", as you say, because if the value did not change, there is no reason to do anything at all.
But I think it should also be done "on the way in" to avoid useless re-computations.

Of course this has a memory cost, as commented by @shaylew:

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

That's what we decided for tansu.
In tansu, we also cache calls to equal to avoid calling equal more than once with each previous value (cf here). Each time the value of a signal changes, we increment a valueIndex counter and we clear the cache of calls to equal. For each listener of a signal, we store both the valueIndex and the value it last received. To know whether the value changed, we look at the current cache with the valueIndex of the listener and call equal only if it was not already called for this valueIndex.

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.

I agree this is not pretty/performant.

(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.)

Having a way to intercept tracked reads is an interesting primitive to add. I don't think it is available in the current spec, is it?
But this is still heavy, and for this solution to work, it would also be needed to have access to the equal function of the signal we read, which is not (yet?) available either in the spec, right?

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?

We included the feature of storing the value received by each listener as part of a big refactoring that fixed a number of bugs. It was included especially because tansu implements the svelte store contract, which means a subscribe function allows to register a listener that is supposed to be called each time the value changes. It would be a bug to call the listener if the value actually did not change and is the same as before.

I think people would have the same expectation with native signals: signals should not be recomputed if the value of all dependent signals is the same as the last computation. I think it should work out of the box without having to add an extra layer of signals. Having a different behavior when adding or not adding a layer of computed signals feels buggy in my opinion.