JuliaGizmos / Vue.jl

a Julia wrapper for Vue.js
Other
21 stars 8 forks source link

avoid unnecessary updates #10

Closed piever closed 4 years ago

piever commented 6 years ago

This should make the hack here unnecessary: WebIO observable only updates Vue variable if that would change the value of the Vue variable and Vue only updates WebIO if the update to the Vue observable didn't come from WebIO.

JobJob commented 6 years ago

Is there not an advantage keeping those checks in the webio js? e.g. for javascript only observables?

piever commented 6 years ago

It is actually disadvantageous to keep them there, in that they interfere with libraries that work differently from Vue. I've ported knockoutjs as a Vue alternative (to solve some issues with nested scopes) in Knockout.jl. And it turns out those checks cause the WebIO observable to not update the Julia observable (when the observable is an array). I'm not sure why exactly things should be different between Vue and Knockout but as these checks are only appropriate for some javascript library, they should probably live in that library's package.

JobJob commented 6 years ago

Sounds weird. I brought it over from Vue just because I thought it's a sensible way to avoid "over-synchronisation" or constantly sending updates back and forth, in general. It's not really Vue specific, I just mentioned Vue in that code because a) that's where I got it from, and b) I assumed Vue should have fairly intelligent update/synchronisation code (the reason I went digging in the Vue source for this stuff in the first place).

If the arrays are different that check shouldn't be an issue I wouldn't have thought, and if it is it's a bug in arrays_and_equal no?

JobJob commented 6 years ago

What's the actual case/situation that is causing the problem?

piever commented 6 years ago

The part I'm unhappy about is that it's failing to update Julia. This for example would work:

        if (!(val === observable.value || arrays_and_equal(val, observable.value) ||
            (val !== val && observable.value != observable.value))) {

            observable.value = val

            // fire any handlers for the given observable in this scope
            if (typeof scope.handlers[name] !== "undefined") {
                var fs = scope.handlers[name];
                for (var i=0, l=fs.length; i<l; i++) {
                    var f = fs[i]
                    f.call(scope, val, true) // true for "client-side"
                }
            }
        }
        // sync the observable's value to julia if `sync` is true in any scope
        if (sync && observable.sync && !synced_julia) {
            WebIO.send(scope, name, val);
            synced_julia = true
        }

EDIT: to clarify what I meant, I'd also be happy to only sync Julia (but not fire javascript handlers) jf the new value is equal to the old one. This should keep the "optimization" part without causing the knockoutjs bug.

piever commented 6 years ago

What's the actual case/situation that is causing the problem?

Here's a MWE:

using Knockout
checked = Observable(String[])
checkedValue = "a"
template = Node(:input, attributes = Dict("type" => "checkbox", "data-bind" => "checkedValue : checkedValue, checked : checked"));
knockout(template, ["checked" => checked, "checkedValue" => checkedValue]) |> display

Now checked is an Observable that's supposed to keep an array of values of all checked checkboxes. Which means, String[] if the one checkbox is unchecked and ["a"] otherwise. What happens with WebIO master (but is fixed by removing those checks) is that the array checked[] only updates the first time and then stops updating. As this is fixed by the changes linked above, I believe it's an issue with WebIO, though I don't have a consistent explanation of what issue it is exactly...

JobJob commented 6 years ago

Not sure if you know this but you can add the js keyword debugger, e.g. above this line, and it will drop into into the browser devtools when it hits it (I'm doing this in Chrome on Jupyter).

The issue is that somehow the observable's value (x.value) is already updated by the time it reaches that code. So when you, e.g., check then uncheck the checkbox, x.value here is already an empty array.

I saw the same thing with Vue, if x.value = val doesn't make a copy, all the reactive watching mechanics that Vue sets up on val, are retained in x.value. This is why I did all the weird looking stuff that makes a copy here and here

Knockout is probably doing something similar.

So in order for webio to know about the value changing (so it can sync observables to julia) either: 1) call a webio.sync_obs inside the $watch to "manually" sync the observable 2) (current Vue solution) you avoid the situation by making a copy in a watch, thus isolating this issue to the (Vue/Knockout) package that causes it.

Maybe the first solution is neater?

I do believe that in general WebIO needs to keep the checks it has (to avoid some infinite sync loop scenarios)

edited: clarifications

JobJob commented 6 years ago

Made some edits above

shashi commented 6 years ago

We try to avoid loops by using a Backedge callable type as a handler in WebIO. This handler is specifically designated to update a Julia observable when JS wants that. We use setexcludinghandlers to update the observable while skipping the Backedge handler to prevent infinite loops. I think this is the right level of loop prevention WebIO should do.

In question here is what semantics for Observables we want. Does an observable

  1. Update when someone fires an update
  2. Update when the value changes

In general, the Observables package has 1). in WebIO currently we do 2). I'd prefer there be one semantic, and I tend to prefer 1) because it's more general. It feels like a leak of concern to have the checks in WebIO and we should deal with idiosyncrasies of Vue in Vue.jl.

What happens with WebIO master (but is fixed by removing those checks) is that the array checked[] only updates the first time and then stops updating.

BTW This is because I believe Knockout is updating the same Array object, hence causing === (and hence all kinds of equality) to be always true. Without the check for equality we send a current snapshot of the array by "serializing" it, so that works out correctly.

piever commented 6 years ago

I see, so the issue is that unless I always do copies on both sides (WebIO and Knockout), they start sharing the underlying array, so setting WebIO from Knockout can never trigger anything as they're equal already (Knockout updates observablearrays in place, which is the efficient thing to do).

It seems a bit of a hack to me to have to take copies on both sides to avoid this kind of bugs, and I would also be in favor of keeping a unified semantic (Observable triggers always) rather than different rules for WebIO and Julia observables.

shashi commented 6 years ago

It seems a bit of a hack to me to have to take copies

Well there's no way to avoid it anyway -- when you send something over JSON you're copying that object in a sense.