BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
856 stars 130 forks source link

performance improvment template expressions evaluated multiple times #421

Closed johan-ohrn closed 5 years ago

johan-ohrn commented 5 years ago

Please see this demo on codepen that shows how template expressions are evaluated multiple times. Run it with the console open as it prints out how many times the ~log() helper was executed.

The first thing is that a linked template expression such as {^{:~log('x') && what && ever }} is evaluated two times. Remove the linking (^) and it's evaluated only one time.

The second thing is that the whole expression is re-evaluated whenever any of the linked properties change which is correct on it's own. But there should be room for improvments here. Consider the following observable update: $.observable(viewModel).setProperty({ever:2, what: 2}); These updates are made as a batch and so it would be nice if the jsrender/jsviews side of things could "collect" these two observable property change events and evaluate the template expression only at the last of the property change events.

The third thing has been remedied by 417 but I mention it anyway since you may or may not be aware of it. This is regarding intermediate renders. Consider the following template expression and the following observable update:

{^{if ~log('y') && (what==1 || ever==2) }}
  {{:~log('z_true')}}
{{else}}
  {{:~log('z_false')}}
{{/if}}

$.observable(viewModel).setProperty({what: 0, ever:2});

What will happen in the current version of jsviews is that the "what" property is updated to 0 which leads to the false part of the template to render. Immediatelly after the "ever" property changes to 2 which cause the true part of the template to render.

BorisMoore commented 5 years ago

Thanks Johan. Generally of course it is assumed rendering (including function calls, converters etc) is without side effects, and so can be called more than once. From a perf point of view, the rendering of a {^{:}} tag is typically a small perf hit compared to things like disposing of bindings. And running code that attempts to cache rendered content, or collect prop change events in a smart way may end up costing as much in perf as the original redundant operations. (And adding also in complexity and code size.)

Even leveraging the batched behavior of $.observable(viewModel).setProperty({ever:2, what: 2}); may be complex, since there may be multiple template expressions depend on different combinations of ever, what etc. : {^{:ever}} {^{:what}} {^{:what && ever}}. It may be possible at the level of individual bindings, but for the moment I'll put as a lower priority than some other work in hand...

So let's keep this issue open for now, and we'll see...

BorisMoore commented 5 years ago

Closing for now. (Labelled for future reference as 'Optimization')

renaudham commented 4 years ago

Hi

What is the status on this? On trying building big complexe pages, I see on console log that many data binded element are rended or called (?) twice when data have not changed yet. Is there anything to do? I started a complex app with a lot of data on a SPA, and now I' not feeling safe for the performances. I've read other tickets concerning perf, but this one seems not solved yet.

Thanks for the great job anyway. Regards

johan-ohrn commented 4 years ago

Here are a few pointers that might help you out. I've used these with pretty good success:

1) data-linking is expensive so try to move them outside loops. I.e.

<div>
{{for somearray}}
  <li data-link="{on 'click' doSomething}">...</li>
{{/for}}
</div>

can be written as

<div data-link="{on 'click' 'li' doSomething}">
{{for somearray}}
  <li>...</li>
{{/for}}
</div>

This result in one data-link for all li-elements as oppsed to one data-link per li-element.

2) Avoid re-rendering data-links (same line of though as 1)

<div>
{^{if someCondition}}
  <button data-link="{on 'click' doSomething}">
{{/if}}
<div>

can be written as

<div data-link="{on 'click' 'button' doSomething}">
{^{if someCondition}}
  <button>
{{/if}}
<div>

If someCondition toggles a lot then the first version would cause data-linking to take place after each render. In the second version it takes place one time on initial render.

3) Avoid complex conditional expressions

{^{if something && somethingElse && yetAnotherThing}}
   some really complex expensive stuff here...
{{/if}}

If your javascript code does something like this

$.observable(viewmodel).setProperty("something", ...);
$.observable(viewmodel).setProperty("somethingElse", ...);
$.observable(viewmodel).setProperty("yetAnotherThing", ...);

This will cause the body inside the if expression to re-render and data-link 3 times.

Instead you want to combine the individual properties into a composite one such as

{^{if allConditionsSatisfied}}
   some really complex expensive stuff here...
{{/if}}

and in the viewmodel $.observable(viewmodel).setProperty("allConditionsSatisfied", ...);

4) Split your updates if possible.

{^{if property1}}
...
{{/if}}
{^{if property2}}
...
{{/if}}
{^{if property3}}
...
{{/if}}

from viewmodel

$.observable(viewModel).setProperty({
  property1: ...
  property2: ...
  property3: ...
});

This updates the view with respect to all three properties in one go. Instead you could split the updates into different setProperty calls separated by a call to requestAnimationFrame or setTimeout such as

$.observable(viewModel).setProperty("property1", ...);
requestAnimationFrame(() =>{
  $.observable(viewModel).setProperty("property2", ...);
  requestAnimationFrame(() =>{
    $.observable(viewModel).setProperty("property3", ...);
  });
});

Depending on your specific case this may or may not work. Idea is to not hog the main thread for to long and allow the browser to perform other rendering tasks and user input between view renders.

Again these are general guidelines that work pretty good for us. If you show some actual example I may be able to give you more pointers.

johan-ohrn commented 4 years ago

@BorisMoore

One area that I think could be improved relatively easy is cleanup. When a view is re-rendered the cleanup (whatever jsviews does internally) could perhaps be done async after rendering/linking the new content. Calling life cycle functions such as a tags dispose method still needs to be done sync as to not introduce breaking changes.

An other area where I think I've seen hints in the code is defered linking. Because linking seem to be very expensive compared to the overall rendering it would be greate if this part could be done async after the view has rendered.

To give some context why I think rendering performance is critical... I have this search page that updates itself as you type. From a usability perspective it's important that the page does not constantly freeze up during rendering new content. Both cleanup and linking as mentioned above are performance thieves.

As for cleanup performance cost. I've managed to improve rendering performance by manually cloning the linked DOM elements and then observably refreshing the data with empty arrays so that jsviews removes the old content from the DOM, then manually attaching the cloned elements. I can do this while waiting for a request to the server. It's not ideal and it's heavy lifting on our part to get rid of the cost of cleanup during the next re-render. It would be nice if jsviews instead performed the internal cleanup after the new content has been rendered to the DOM and thus eliminate the perceived performance cost of cleaning up the old content.

As for defered linking... The search page contains loads of filters (checkboxes). Ideally I would like to data-link them but the rendering cost (well linking really) is to great so I can only render them in correct checked/unchecked state and then have to resort to jQuery to update them from the view model. If linking could be made async after the content has been rendered to the DOM then I would be able to link these checkboxes and still have stellar rendering performance.

BorisMoore commented 4 years ago

Thanks for your input @johan-ohrn . I am not currently able to work on these scenarios. I'm not sure yet when/whether I will be able to re-invest in this area. But if so, I will look at this in more detail, and re-discuss. There are actually some nascent async features already in the code, but not yet ready for public documentation... Hopefully at some point :)

@renaudham : I am not planning optimization work in the near future. But the current implementation is certainly working well for most scenarios. The comments from Johan may be helpful to you though, and certainly give some ideas as to where you need to look if you are worried about perf.

Do give feedback if you do encounter perf problems in specific scenarios....