xilem: Add Memoization views (Memoize and Arc<impl View>)

[Philipp-M]

This ports the Memoize view from old xilem, slightly enhances it, by checking whether the given view callback is a non-capturing closure and not a function pointer (by asserting std::mem::size_of::<F>() == 0)

It also ports the Arc<impl View> and Arc<dyn AnyMasonryView> from #164 including the example there to show how these two forms of memoization can be used.

[jplatte]

I think this could be turned into a compile-time check by moving it to the inherent impl above (the one containing pub fn new) as

const _ASSERT_CONTEXTLESS_FN: () = {
    assert!(
        std::mem::size_of::<F>() == 0,
        "The callback is not allowed to be a function pointer or a closure capturing context"
    );
};

[Philipp-M]

Oh pretty cool idea, I had compile-time check in mind, but thought it would be more complicated and requires unstable Rust, turns out, not the case, thanks!

I had to add ASSERT_CONTEXTLESS_FN as statement into the new function though, otherwise the compiler would just optimize it away (not sure whether that's a compiler bug, as I think it should also work without, because const needs to be evaluated anyways?

[DJMcNab]

We should explain why here

[Philipp-M]

Is the content of the assert in the last commit better?

[DJMcNab]

Is it worth making a generic type which encapsulates this?
I.e. which owns the count and the Option<Arc<V>>

And so when the data is updated, it invalidates the view.

Because it seems incredibly easy to forget otherwise. IMO, that seems like a better fit for the Xilem threading model, although it's complicated

[Philipp-M]

Yeah agree. But I'm not sure if I understand you completely, do you mean something like in the last commit?

[DJMcNab]

No, I mean as a library type

It isn't quite a View, but it's a utility on top of views

It also should be able to better encapsulate the behaviour

[Philipp-M]

So basically Memoize but with an Arc instead? I'm still not quite sure what you mean TBH.

So something like

struct MemoizedArcView<F, D, State> {
    data: D,
    // When TAITs are stabilized this can be a non-erased concrete type
    view: Option<Arc<dyn AnyMasonryView<State>>>,
    view_factory: F,
}

impl<State, D: PartialEq, F: Fn(&D) -> Arc<dyn AnyMasonryView<State>>>
    MemoizedArcView<F, D, State>
{
    fn view(&mut self) -> Arc<dyn AnyMasonryView<State>> {
        if let Some(view) = &self.view {
            view.clone()
        } else {
            let view = (self.view_factory)(&self.data);
            self.view = Some(view.clone());
            view
        }
    }

    fn set_data(&mut self, data: D) {
        if self.data != data {
            self.data = data;
            self.view = None;
        }
    }
}

fn memoized_arc_view<State, D, F>(data: D, view_factory: F) -> MemoizedArcView<F, D, State> {
    MemoizedArcView {
        data,
        view: None,
        view_factory,
    }
}

?

Though, I think having more direct control over invalidation with an Arc is an advantage over Memoize (not that this utility you're talking about is mutually exclusive though).

[DJMcNab]

This isn't quite what I was trying to describe, but that's probably better as a follow-up (probably related to #235)

[Philipp-M]

Yes might be, alright, keeping it in the example for now, we can still refactor later...

[DJMcNab]

I think requiring Sync here makes sense, although we don't require it for anything other than Arcs

[Philipp-M]

Yeah or views derived from Arc as well or anything, that actually is shared between the runtime and the AppState, but that'll likely be mostly Arcs

[DJMcNab]

As discussed in office hours, I'm personally much more comfortable with the Arc impls, and I find the normal memoisation view a bit concerning from a conceptual threading point of view (this is also important for hot reloading, if the view creation and rebuilding happen in different processes)

But I don't think that opinion is shared by others, and so we agreed to land this.

[Philipp-M]

(Quoting as well from zulip)

this is also important for hot reloading
... it could just not memoise in hot reload mode, which would work I suppose

I may not yet get all the details of hot-reloading (have not dug into that yet), but maybe we could just check if the TypeId of the callback F changes, and add the TypeId of F to the (serializable) ViewState maybe (not sure how robust that is though)?

I think all of this is still kinda fluid, so in case Arc (with extensions?) proves more useful or Memoize limits us somewhat, we may also get rid of Memoize later I guess.