Manual clean-up

[mindplay-dk]

Hey,

This library is interesting as as state management solution for React/Preact - but I'm really interested in using this library to create a truly reactive UI library, one that doesn't diff virtual DOM nodes, along the lines of Sinuous.

I cobbled together a working prototype last night, and this was surprisingly easy:

https://codesandbox.io/s/reactive-dom-lol-rrtd4?file=/src/index.jsx

The problem is, this leaks listeners like crazy - if you make a few changes to the input value, and then toggle the checkbox, you can see the "LOL" messages piling up in the console. Reactions aren't getting garbage-collected, presumably because they internally reference the observables they're listening to.

From the documentation, I understand I'd have to manually destroy() the reactions to free them - which I've tried to do, but suddenly everything is much more complex than it seems on the surface, as I basically have to manually manage the whole create/update/destroy life-cycle using code.

I could be wrong, but I don't think S.js has this issue? Not sure.

I wonder if there would be something we can do with WeakMap to enable garbage-collection of reactions - or maybe avoid keeping subscriber references by decoupling their relations through a message bus or something?

I honestly don't understand the theory behind it all, I just know I like it. 😄

Is this something you're interested in at all, or is this mostly a state management library to you? 🙂

[mindplay-dk]

From the S.js overview:

Automatic Disposals - S computations can themselves create more computations, with the rule that "child" computations are disposed when their "parent" updates. This simple rule allows apps to be leak-free without the need for manual disposals.

This sounds like what I'm looking for, but I'm not sure. Thoughts? 🙂

[mindplay-dk]

Hmm, so at closer inspection, I think S.js has the same problem - automatic disposal happens only in parent/child relationships, you still have to manually dispose of the parents.

So I got to thinking, can we do something with WeakMap maybe - and I noticed you're actually using that in your test-suite to track updates without preventing garbage-collection.

Of course, what we want is a reference to the value, not the key, and WeakMap does not let you enumerate those, which lead me to WeakRef, which I had not heard about before - but this is only available in the very latest browsers, so, hm.

This lead me to wonder if there was a polyfill, which there is - this relies on WeakMap, which is in browsers since almost 7 years (including IE11) and it uses a very simple technique to establish a weak reference:

    var wr = new WeakMap;
    function WeakRef(value) {
      wr.set(this, value);
    }

That is, you keep a WeakMap, and the WeakRef instances use themselves as keys. 🙂💡

I don't know if this approach actually works though - I tested it, and it didn't seem to work for me in the current version of Chrome, so I'm going to ask the author of the polyfill how he tested this. Either there's a bug or this approach might not work at all. 🤔

[zheksoon]

Hi Rasmus, thanks for your interest to the library!

Right, unlike S.js, dipole doesn't implement any parent/child relationships between reaction objects, but it's possible to do some with basic OOP patterns over Reaction class. Will try to do this in my spare time.

About WeakRef, it's not that simple :)
If there was an actually working polyfill, there would be no complex subscription/unsubscription logic in observables/computed/reactions like dipole or MobX implements: all we would need is to keep an array of weak references to objects we need to notify about change.
In case of the polyfill, if we create instance of the polyfilled WeakRef and keep it somewhere in array, the ref itself (so this in your snippet) will never get GCed, and so value will be kept by the internal WeakMap because its key is referenced by someone.

[mindplay-dk]

You're right, this is a dead end - neither of the two polyfills out there actually work. (No idea why they exist to begin with.)

[mindplay-dk]

Hmm, so I've tried to implement manual clean-up, but it seems destroy() has unexpected effects.

https://codesandbox.io/s/reactive-dom-lol-gc-v046n?file=/src/index.jsx

I added a list of reactions to a property on the DOM element - if the DOM element gets replaced during a reaction, I recursively destroy the reactions of any DOM elements being removed.

I added console output, so you can see when created reactions get tracked and released - it looks like there should still be tracked reactions that haven't been released, but for some reason, it only seems to work after the destroy/create cycle for each reaction.

Could this be because I'm creating nested reactions?

Creating new reactions during a call to a reaction, that is.

I noticed a difference from S.js, which immediately calls your reactions the first time, and your library, where I have to manually call run after creating it. I wonder if forcing immediate execution is what enables S.js to track nested reactions? Since this guarantees that nested reactions will also run immediately, enabling it to track the calling context using a stack of some sort.

Or maybe I'm doing something incredibly dumb and this approach doesn't work at all. 😄

[zheksoon]

For some reason I can't get your sandbox to output anything to result screen.
Here's my implementation of nested reactions with parent/child relationships:

import { Reaction } from 'dipole';

let gReactionContext = null;

class NestedReaction extends Reaction {
    constructor(...args) {
        super(...args);
        this.children = [];
        if (gReactionContext !== null && gReactionContext instanceof NestedReaction) {
            gReactionContext.addChild(this);
        }
    }

    addChild(child) {
        this.children.push(child);
    }

    run(...args) {
        this.destroyChildren();

        const oldReactionContext = gReactionContext;
        gReactionContext = this;
        try {
            return super.run(...args);
        } finally {
            gReactionContext = oldReactionContext;
        }
    }

    destroyChildren() {
        this.children.forEach((child) => child.destroy());
        this.children = [];
    }

    destroy() {
        this.destroyChildren();
        super.destroy();
    }
}

function reaction(...args) {
    return new NestedReaction(...args);
}

When a reaction gets created while another reaction is running, it's registered as a child of this reaction. When parent reaction runs, it destroys all child reactions first.

[mindplay-dk]

For some reason I can't get your sandbox to output anything to result screen.

Yeah, that happens. CSB is just so broken these days.

Clear your application state from dev tools and reload the whole thing - sometimes, still nothing comes up, you may have to press the refresh-button in the "browser" in CSB.

Your nested reactions looks nice. I will give that a try. 👍

[zheksoon]

Updated NestedReaction definition, forgot to add children cleanup in destroyChildren() method

[mindplay-dk]

I commented-out the track and release stuff, and now it works. 😄

I don't completely understand how. I'm doing nothing to track or release any listeners. I don't fully understand how this library works, but can it really be this easy? I would have thought I would need to do something to dispose of at least the top-level reactions?

As I understand it, a reaction is basically creates a sort of closure around the observables it depends on, right? How are they getting unsubscribed now?

It can't be this easy? What am I missing?? 😄

[mindplay-dk]

Confirmed no memory leaks - the garbage collector is doing it's job.

This is me going nuts on the input, changing it between "World" and "World2" at least 100 times, and then clicking like crazy on the checkbox. There might be circular references, and things clearly stack up for shorter periods before getting collected, but it doesn't appear to be leaking.

This library is magical. I'm still waiting for the other shoe to drop. I must be missing something. 😄

If this really "just works", is there any reason not to promote this from an extension to a standard feature in the library?

[zheksoon]

As I understand it, a reaction is basically creates a sort of closure around the observables it depends on, right? How are they getting unsubscribed now?

Do you mean with the NestedReaction implementation? It's quite easy though - when a reaction gets executed, it sets global gComputedContext variable to its instance, and then every get() method of observable or computed calls trackComputedContext() function which adds the reaction to observable._subscribers HashSet and reaction._subscriptions list (only if HashSet insertion succeed).

Logic of NestedReaction is quite evident from its code - when parent gets executed, it destroys all children reactions first, so we could get fresh list of children after run() execution. destroy() method unsubscribes the reaction from each of its subscriptions listed in reaction._subscriptions, so after that all references to the reaction are removed and it can be garbage-collected.

If this really "just works", is there any reason not to promote this from an extension to a standard feature in the library?

NestedReaction is quite specific use case, but I think it's possible to distribute it as add-on, like this:

import { NestedReaction } from 'dipole/addons`;

, so it won't be included by default in bundle that I'm trying to keep small-ish.

[mindplay-dk]

S.js highlights this as a qualitative feature - though I guess that might be a matter of opinion. It doesn't seem like this would add more than a few lines if it were built-in? Making HashSet optional would probably make a bigger difference in terms of size.

Your call of course :-)

I added stateful components and still the whole thing is just ~60 lines of code. I was very much expecting things to break because I'm creating observables inside the component "instances" now - but the garbage collector still seems to be doing it's job.

I remain suspicious because this seems waaaay too good to be true. 😄

[zheksoon]

As I understand it, a reaction is basically creates a sort of closure around the observables it depends on, right? How are they getting unsubscribed now?

Do you mean with the NestedReaction implementation? It's quite easy though - when a reaction gets executed, it sets global gComputedContext variable to its instance, and then every get() method of observable or computed calls trackComputedContext() function which adds the reaction to observable._subscribers HashSet and reaction._subscriptions list (only if HashSet insertion succeed).

Logic of NestedReaction is quite evident from its code - when parent gets executed, it destroys all children reactions first, so we could get fresh list of children after run() execution. destroy() method unsubscribes the reaction from each of its subscriptions listed in reaction._subscriptions, so after that all references to the reaction are removed and it can be garbage-collected.

If this really "just works", is there any reason not to promote this from an extension to a standard feature in the library?

NestedReaction is quite specific use case, but I think it's possible to distribute it as add-on, like this:

import { NestedReaction } from 'dipole/addons`;

, so it won't be included by default in bundle that I'm trying to keep small-ish.

[mindplay-dk]

I could not understand the NestedReaction class extension, why it had it's own reaction gReactionContext and so on, so I tried integrating it back into the main Reaction, and now I get it. 😄

But honestly, it's 10 lines of code:

This wouldn't leave more than a few bytes of footprint.

I honestly still think this is a "qualitative" feature that shouldn't be optional or left to an add-on. I wouldn't want to leave users of the library to discover on their own (the painful way) why the library leaks memory for their use-case.

(Note the missing type-check gReactionContext instanceof Reaction in the Reaction constructor - I'm experimenting with internal type-safety to help myself understand things better, and didn't discover this mistake until it became clear that gComputedContext can be either Computed or Reaction or those two other special contexts...)

[zheksoon]

So finally, you've convinced me :) I added the nested reaction behaviour as default in dipole since version 2.2.0, so no NestedReaction class is needed anymore for your experiments.

In case old behaviour is needed, it can be emulated using untracked transactions:

const free = utx(() => reaction(() => { ...reaction body... }));

[mindplay-dk]

In case old behaviour is needed, it can be emulated using untracked transactions

Nice, elegant solution 👍

[mindplay-dk]

Tested and can confirm no memory leaks or any adverse results from switching to 2.2.2

I think we can close this issue? 🙂