Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Performance] template binding if option should not cause rerender when expression re-evaluates to true #2332

Open
miellaby opened this issue Dec 28, 2017 · 8 comments

Comments

@miellaby
Copy link
Contributor

miellaby commented Dec 28, 2017

I've just realized that template: { if : expression, ... } is not equivalent to if: expression.

In the first notation, the binding definition is an object ({ "if" : expression, ... }). KO default equality comparator always returns false about objects, so the binding handler update function is reevaluated at every single dependency change, even if all included expression results are unchanged (typically "list().length > 0").

As a workaround, one may write this ugly sequence to get a reasonable behavior

template: { if : ko.pureComputed(function() { return expression; }), ... }

This is a severe performance issue IMHO. It neutralizes most of the effort made since the third KO version to reduce useless computation/rendering. According to my own experience, most of template bindings contain a if predicate.

Is the problem wider? Bindings which relay on an object as a definition might be updated way too often.

suggestion: a way to set the definition equality comparator from the handler init fn and a provided alternative comparator based on first level attributes.

@brianmhunt
Copy link
Member

This should be an easy fix, but until then a simpler workaround might be if: !!expression.

@miellaby
Copy link
Contributor Author

My translation attempt based on i18next made me changes all my "if:" and "with:" into "template:" so to have an afterRender callback to parse the data-i18n attribute. I reverted this hell of a binding. I now use a custom binding provider (thanks for the idea by the way).

I think about the "css" binding as well. The css binding handler update calls are not optimized for the same reason. So much useless DOM interaction (unless every expression is wrapped in a computed by hand).

@mbest
Copy link
Member

mbest commented Jan 11, 2018

I think about the "css" binding as well. The css binding handler update calls are not optimized for the same reason. So much useless DOM interaction (unless every expression is wrapped in a computed by hand).

That was a reason I favored the syntax supported by Knockout.Punches (and now in TKO): css.classname: value.

@miellaby
Copy link
Contributor Author

miellaby commented Jan 30, 2018

@mbest I'm not even sure that the binding expression wrapping works as I'd expect.

It looks like even a simple myBinding: bool is uselessly updated in a binding like "myBinding: functionMostlyReturningTrueWhateverItsKODependencies() ", while "myBinding: ko.pureComputed(theSameFunction)" does debounce useless updates;

Should I investigate further and set an example or is it a known behavior?

Is not identity-based debouncing the next lever for performance tuning? In my own experience, expressions have a high dependencies/actual-changes ratio (especially if one considers object reference equality that I wish KO did not ignore thanks to some global settings).

@mbest
Copy link
Member

mbest commented Jan 30, 2018

I would recommend against the myBinding: ko.pureComputed... syntax since that will involve recreating the computed each time the binding is updated. The approach I recommend is to set up those computed observables in your view model. Then rather than referencing functions in your binding, you'll just reference computed properties.

@miellaby
Copy link
Contributor Author

miellaby commented Jan 31, 2018

involve recreating the computed each time the binding is updated

Oh. That's tricky. Now I feel powerless.

Computed in view models has practical limitations IMHO. Many binding expressions are only used once, and are tiny variations around the same concept. For exemple, the lack of the "hidden" binding in previous releases makes visibility: !HiddenObservable() frequent. So are: t().length == 0, t().length > 0, for every Array. Editing the viewModel every time user interactions are tuned on the view side feels like a failure of the KO promise. I don't feel like adding fooISEmpty, fooIsNotEmpty, fooIsLessThan25, fooIsOver100, fooIsEmptyButBarIsNot, etc for every foo observable.

@mbest
Copy link
Member

mbest commented Jan 31, 2018

3.5.0 adds a hidden binding (#2103). We've tried to make bindings as lightweight as possible. Obviously the problems with the template binding still exist, but it's difficult to fix without breaking compatibility.

@miellaby
Copy link
Contributor Author

miellaby commented Feb 1, 2018

compatibility

I imagine something like the deferred updates compatibility approach.

An aggressive debouncing of indistinguishable updates would be enabled by:

ko.options.distinctUpdates = true;
ko.options.equalityComparer = ko.observable.referenceComparer;

with

  • ko.options.equalityComparer = ko.observable.primitiveComparer: KO relays on the v3.4.0 equality comparer (default)
  • ko.options.equalityComparer = ko.observable.referenceComparer: KO does reference comparison ( (a, b) => (a === b)), which is fine when viewmodel objects are immutable at the exception of observables wrapped attributes.

and

  • ko.options.distinctUpdates = false : the legacy behavior where indistinguishable updates of observable/computed are filtered out but expression-based-binding updates aren't optimized yet (default).
  • ko.options.distinctUpdates = true : KO will always tend to filter out indistinguishable updates as much as possible now and forever. In this regard, the valueAccessor within binding code works like a computed. That is, ko.options.equalityComparer is used to filter out useless valueAccessor updates.

When enabling ko.options.distinctUpdates, developers should never ever suppose observable(observable()) having any effect nor custom bindings being refreshed as long as the bound value is semantically unchanged.

To enforce ko.options.distinctUpdates for bindings, whenever they are simple (if, visible, text, html, ...) or complex (attr, style, css, template, ...), I guess the trick is to detect when the bound expression is a literal object ( { 'a': foo, 'b': bar } ). In such a case, one may wrap the expression into a utility which

  • keeps the last value of the expression
  • compare separately non-literal components of the expression (a kind of half-deep comparison),
  • returns the object unchanged (same ref, that is the last value) if NO non-literal components in the expression is actually changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants