Snap.SnapshotOn when both snaps change at the same time

[Bananas-Are-Yellow]

I'm trying to control update propagation of views so that update only occurs when the value has changed.

Here is an example. I am dragging the mouse and I want to convert a view of XY positions into a view of distances from the mouse-down position, where these distances are snapped to the nearest value of 10.

let snap (dist: float) x = dist * Math.Round (x / dist)
let inline private sq x = x * x
let dist (stX, stY) (enX, enY) =
    sq (stX - enX) + sq (stY - enY) |> float |> sqrt |> snap 10

// set the value only if it has changed
let inline assign (var: Var<_>) value = if value <> var.Value then var.Value <- value

let distView down pos = // result is View<float>
    let state = Var.Create 0.
    pos // input is View<int * int>
    |> View.Bind (fun pos ->
        assign state (dist down pos)
        state.View)
    |> View.SnapshotOn state.Value state.View

As you drag away from the mouse-down position, many XY positions will produce the first snapped distance, 0, and then many more XY positions will produce the next distance, 10, and then 20, and so on.

The usual behavior of computed views (View.Map, View.Bind, etc.) is that whenever input views are updated (their Snap becomes obsolete), this marks the computed view snap as obsolete, and so on, all the way up the dependency tree. A new snap value will be recalculated if and when it is next demanded.

This is even true for View.UpdateWhile. When the first view is false, the computed view will nevertheless update each time the second view is updated, and return the last captured value of the second view over and over.

This is fine in that the values are correct, but if the calculation done inside a View.Map function is expensive, we don't want to repeat the calculation when the value hasn't changed. In my case, the calculation done with the distance (not shown here) is expensive, but this needs to be efficient since it is going on while the user is dragging the mouse.

In the code above, at first glance it looks like View.Bind returns state.View, which doesn't change all the time, but in fact View.Bind creates a new computed view, which just happens to have the value of state.View in this example. So even if state.View hasn't changed, View.Bind will create a new snap for each new XY position, copying the value for this snap out of state.View.

So my idea was to use View.SnapshotOn at the end, so that the resulting view is only updated when state.View has actually changed value. But sadly it doesn't work, and I only get the default value.

I believe the problem might be in Snap.SnapshotOn. Here's the code:

let SnapshotOn sn1 sn2 =
    match sn1.State, sn2.State with
    | _, Forever y -> CreateForever y
    | _ ->
        let res = Create ()
        let v = ref None
        let triggered = ref false

        let obs () =
            v := None
            MarkObsolete res

        let cont () =
            if !triggered then
                match !v with
                | Some y when IsForever sn2 -> MarkForever res y
                | Some y -> MarkReady res y
                | _ -> ()
        When sn1 (fun x -> triggered := true; cont ()) obs
        When sn2 (fun y -> v := Some y; cont ()) ignore
        res

I'm not sure what this triggered test is all about, but it might be the problem. As I see it, we want the value from sn2 to be applied to res, and we want res to be made obsolete when sn1 becomes obsolete.

Perhaps I don't understand the code. Am I misusing View.SnapshotOn or is there a bug?

[Bananas-Are-Yellow]

As a workaround, I'm taking a different approach. Instead of preventing update, I have a View<'t * int>, where the second item is a repeat count. So each time the value is unchanged, I increment the count. The consumer of the view can ignore values with a count other than 1 if repeated values are to be skipped.

Initially it makes the code a bit ugly, but I think I can make it tidier.

I'd still like to know about View.SnapshotOn though.

[Bananas-Are-Yellow]

If anyone is interested, here is the cleaned up repeat count code.

/// Converts a View<'a> into a View<'b * int>, containing a repeat count.
/// The mapping function f is 'a -> 'b option, where None means there is
/// no change, so the last value should be repeated.
module View =
    let Repeat def f input =
        let state = Var.Create (def, 1)
        let update value =
            state.Value <-
                match state.Value, value with
                | (v, n), None -> v, n + 1
                | (v, n), Some value when v = value -> v, n + 1
                | _, Some v -> v, 1
        input
        |> View.Bind (fun input ->
            update (f input)
            state.View)

Notice that the state var is encapsulated.

The processing function f returns an optional value. Returning None means the current value should be repeated. This is more convenient than looking up the current value, and is more natural in code that is considering state transitions (the example here is not like that, but I have other code that is).

The XY position mouse-dragging code is now:

let snap (dist: float) x = dist * Math.Round (x / dist)
let inline private sq x = x * x
let dist (stX, stY) (enX, enY) =
    sq (stX - enX) + sq (stY - enY) |> float |> sqrt |> snap 10

let distView down pos = // converts View<int * int> to View<float * int>, i.e. (dist, repeat)
    pos |> View.Repeat 0. (fun pos -> Some (dist down pos))

The consuming code simply matches on (dist, 1) to skip duplicates.

Given that this processing function always returns Some, it can be written as:

let AddRepeat def input = Repeat def Some input

let distView down pos =
    pos |> View.Map (dist down) |> View.AddRepeat 0.

With a bit more work, I should be able to remove the def argument from View.AddRepeat. It is needed for View.Repeat in case the processing function returns None the very first time.

[Bananas-Are-Yellow]

Back to the original problem, I tried two workaround, using View.Map followed by View.Map2, instead of using View.Bind, but neither of them worked:

First attempt:

let distView down pos = // result is View<float>
    let state = Var.Create 0.
    pos // input is View<int * int>
    |> View.Map (fun pos -> assign state (dist down pos))
    |> View.Map2 (fun a b -> a) state.View
    |> View.SnapshotOn d.Value state.View

Second attempt:

let distView down pos = // result is View<float>
    let state = Var.Create 0.
    pos // input is View<int * int>
    |> View.Map (fun pos -> assign state (dist down pos))
    |> View.SnapshotOn () state.View
    |> View.Map2 (fun a b -> a) state.View

[Tarmil]

For the use case of avoiding the recomputation of a mapped value, I just PR'd a MapCached function #39. Can you check if this does what you need?

[Bananas-Are-Yellow]

View.MapCache does not help me, but it might be useful in its own right.

Although not detailed in my original post, the work done inside the 'expensive' View.Map function does not produce the resulting value for the View.Map. The resulting value is unit and the work is a side effect, which is a call to the server which does the expensive work.

I appreciate that View.Map can't avoid propagating the change, but surely View.SnapshotOn can?

[Tarmil]

I don't really understand why MapCached doesn't do the job. It compares the input value to the saved input, and if it's equal, doesn't call the (potentially expensive) mapping function and instead returns the saved output (which would always be () in your case). I understand that the actual output is not important, but nevertheless the function only gets called when the input changes. Where exactly does this differ from what you need?

[Bananas-Are-Yellow]

I think you are right, View.MapCached should work for me. When are you making the next version available?

I'm still curious to know what's wrong with the View.SnapshotOn approach. The internal Snap.ShapshotOn is different from the other functions in that it only obsoletes the resulting snap when sn1 is updated, and not when sn2 is updated.

Also curious to know what the triggered test is for.

[Bananas-Are-Yellow]

It occurs to me that I can try this out myself.

static member MapCached f input =
    let mutable prev = None
    input
    |> View.Map (fun value ->
        match prev with
        | Some (arg, res) when arg = value -> res
        | _ ->
            let res = f value
            prev <- Some (value, res)
            res)

The caching logic does not need to be done down at the Snap level.

[Tarmil]

I think the reason why SnapshotOn does not do what you expect is that it does nothing when sn2 becomes obsolete. It waits until sn1 becomes obsolete before retrieving the new sn2; but the problem is that it's the action of retrieving the new sn2 that causes sn1 to be obsoleted, because it's only then that the mapping function is called. In the end it's kind of a deadlock.

[Bananas-Are-Yellow]

But isn't that a bug? I mean, what I was doing with ShapshotOn is reasonable, isn't it?

[Bananas-Are-Yellow]

MapCached seems to do the trick (my version at least). Thanks.

[Tarmil]

It's really more "undefined behavior" than a bug. In the semantics of View, there is no guarantee that intermediate functions are called for all upstream updates. If there was such a guarantee then it would be easy to introduce memory leaks with Bind. Only the final value received by Sink has such guarantees. So any side-effects inside the dataflow graph should be purely for optimization, and should not change the semantics of the program.