anp / moxie

lightweight platform-agnostic tools for declarative UI
https://moxie.rs
Apache License 2.0
828 stars 27 forks source link

Fake `moxie::cache` disables gc within passed function #238

Closed zetanumbers closed 3 years ago

zetanumbers commented 3 years ago

Code to reproduce varying behavior (uncomment lines):

use moxie::{cache, once, runtime::RunLoop, Key};

fn main() {
    // let mut i = 0;
    let root = move || {
        // i += 1;
        // cache(&i, |_| {
        let (commit, key) = moxie::state(|| false);
        if *commit {
            once(|| print!("bonk"));
        }
        println!("");
        key
        // })
    };
    let mut rt = RunLoop::new(root);
    let mut i = 0;
    let mut count = move || {
        print!("{}: ", i);
        i += 1;
    };
    count();
    let key = rt.run_once();

    let mut conditional_run = move |c| {
        count();
        key.set(c);
        rt.run_once();
    };

    conditional_run(false);
    conditional_run(true);
    conditional_run(true);
    conditional_run(false);
    conditional_run(false);
    conditional_run(true);
}

This code produces:

0: 
1:
2: bonk
3: 
4:
5:
6: bonk

However if you uncomment every line of code, i.e. wrap root's body with moxie::cache, which would be recached on every run bc of argument counter i, you'll get:

0: 
1: 
2: bonk
3:
4:
5:
6:
anp commented 3 years ago

I've reproduced this and I see the problem -- the DepNode for the once() call is unconditionally inheriting the liveness from the cache() node upon which its depended, but this is obviously incorrect in cases where the cache() call executes the closure on the same revision that once()'s DepNode hasn't been called.

I think the solution is to store when a node was last actively traversed, and to not inherit liveness until it's been "dormant" for at least one revision.