knockout / knockout

Knockout makes it easier to create rich, responsive UIs with JavaScript
http://knockoutjs.com/
Other
10.47k stars 1.51k forks source link

Major Browser Lag when assigning new value to observable that component renders #2357

Closed chrisknoll closed 6 years ago

chrisknoll commented 6 years ago

I'm experiencing a bit of a show-stopper in my knockout app where a seemingly simple 'click' operation which assigns a new value to an observable causes a 8 second hang in the UI to re-render the component that is bound to the observable. Here's a screenshot of the UI to demonstrate the complexity:

uiview

There's a bit more surrounding this part of the UI, but my problem is limited to a very specific function: when you click an item on the left, the clicked item is bound to a 'selectedItem' observable which the component on the right renders. Here's a wireframe of the structure:

wireframe

The IndexRule param is a top level paramater that references the overal view model, and the InclusionRule parameter is the specific item that is selected. (selectedInclusionRule is an observable).

The problem is: when clicking between one of the items on the left and the other. a huge lag spike occurs (about 8-10 seconds) which should amount to just a small DOM replacement of the rendered component (which should be sub-second). The overall model is actually quite large: about 2.3MB of JSON text. And the UI is responsive when the overall view model is small (I have another example of a 28kb JSON object where the UI is responsive). One source of the lag could be the overall number of observables alive at the time. However, for this specific use-case, clicking on a selected item and rendering the item in a component shouldn't lead to any additional notifications to any subscribed observables, so I'm very confused as to what is happening here.

I'd like to know what people would do to diagnose this type of issue. I attempted to crack open the Chrome DevTools and take a profile after loading the model, and then profiled clicking one item, 2 second break, and then clicking the second item. This is the performance report I got: profileview

As you can see, there's a very large rise in listeners, JS heap and nodes that rises very fast, and then suddenly drops off. About 1/3 of the total time here is spent in a GC collection and DOM collection. But, what I can't understand is: the 'item renderer component' that renders the selected item is just a very small box on the screen: maybe 20 UI elements, maybe a hundered DOM elements under the covers? But the profile is showing a huge amount of allocations and then cleanup.

I can only imagine I'm doing something wrong with components or there is some other problem in my approach. For full disclosure: the UI depends on bootstrap functionality, I am leveraging some custom data bindings from jqueryui and knockout.sortable. I am showing you screenshots running off of the 3.5 beta, but I get the exact same result on 3.4.2 (which is what our internal production environment is..I was hoping something in 3.5 would optimize this like component HTML based on a static string doesn't get reparsed every time, but it didn't help).

@SteveSanderson , @mbest , @rniemeyer , @brianmhunt : if you have any techniques for tracing through the code or setting up certain watches to understand how components might be destroyed/reloaded, I'd very much appreciate your insight.

Also: I'm working on setting up a 'live' demo of this behavior once I get a website provisioned and configured, so I will post a link to a real-world demonstration of the issue once it becomes available.

brianmhunt commented 6 years ago

@chrisknoll It's a very good question, and I've encountered this exact problem from time-to-time.

Here are some tricks I've used to debug:

  1. Make sure no observables for "outer" if, with and template bindings are being changed (using x.subscribe() to see what's changing it); It can help to add a debug binding that prints every time it's bound to the dom, or other feedback e.g.
<div data-bind='template: x'>
   <!-- ko debug --><!-- /ko -->
</div>
  1. See if any computes are going crazy by overloading ko.computed.fn.updateVersion to both update the version but also print out any variables that have unrealistically high recounts
  2. Look through the GC while adding/removing pieces of the DOM that might be problematic

Huge allocations and GC are strong indications that there's something "exponential" happening.

Simultaneously, there are a few performance buffs that can assist (though not as ideal as rooting out crazy things):

  1. Use deferred and rateLimit'ed observables / computeds
  2. Have the UI indicate that it's updating e.g. with a spinner, while the DOM is being bound (while hidden)

That's what comes to mind. I'll pop back and add anything else that might assist.

krnlde commented 6 years ago

I faced the same problem with many paints (>2000) on the DOM. See the issue here. The only solution that helped was to defer the painting to many event loops. Drawing just a bunch each loop and then deferring it again until it's finished. @cervengoc created a patch that allows fast-foreach to paint in different loops. Have a look over here: cervengoc/knockout-fast-foreach. Unfortunately it never made it into the master, because there is some polishing left to do. I forked it to have some consistency in my dependencies krnlde/knockout-fast-foreach

Try it out and let me know if it helped. Would be awesome to get back some traction on this issue.

chrisknoll commented 6 years ago

Thank you! I'll look into these approaches and let you know what I uncover. Especially the consideration about the outer-scoped observable changing.

fastfasterfastest commented 6 years ago

what people would do to diagnose this type of issue

Maybe obvious, but one initial thing is to replace the inclusion-rule-editor component with a very simple component (both the view model it may create, and the way it is rendered e.g. only render some text). And then progressively add more functionality to both the view model of the component and the rendering of the component to pinpoint the bottleneck and issue.

chrisknoll commented 6 years ago

Ok, so funny story...but the short version is 'my fault'.

I made a component that was designed to show a little 'preview' of a subset of the model, and i wanted this preview to popup in the context of a bootstrap dropdown menu. Well, after a lot of churn, I had to make it so that the 'preview' appeared along side the dropdown menu using an absolute positioned div that I could control through some observables.

Well, I left the component inside the dropdown menu that was spewing out some dom elements. Well, the way bootstrap dropdown menus work, you put some li/ul items in the list and if it changes, then the dom gets reshaped so that they look like dropdown menu items. The fix was to remove the component that I accidently left behind, and now the dom wasn't being constructed in the dropdown menu via my custom component.

Why didn't I notice this before? One of the PITA parts of the bootstrap dropdown menu is how it uses elements with overflow:none so even tho my component was physically there, it was not visible on the screen!

So, thank you all for your help, you all gave me some great tips about debugging. I have to give the gold star, tho, to @fastfasterfastest , since his suggestion led me to systematically down the chain of components and rip stuff out until it was responding normally (fast), and then putting stuff back in until it broke it. I was in the process of weeding out the next function when I noticed the duplicate component under the dropdown menu when I removed that, it went back to normal.

mbest commented 6 years ago

@chrisknoll I'm glad you figured it out how to fix it. Were you able to figure out why that component caused such a huge slowdown in your app?

chrisknoll commented 6 years ago

I can't be absolutely sure because it was an intersection with dom elemetns being created by the component in knockout and bootstrap dropdown menus using the generated dom to create the dropdown menu. I think there was some crosstalk there. Also, in the final version (where I got the pop-up to be shown in the right place), I wrapped it in a if: binding so that the component was only rendered when needed (vs. I think the original was rendering all the dom elements in the forEach while now it will only show the component when you hover over the item in the dropdown).

If you are curious I have pushed the broken and fixed version of the component with 2 demo .txt files (that the view models serialize from) here: http://ec2-34-227-195-147.compute-1.amazonaws.com/circe_slow/ http://ec2-34-227-195-147.compute-1.amazonaws.com/circe_fast/

This is just temporarily hosted, but it'll be around for a little while just for example purposes. This has been a long process of learning for me, covering things like AMD, knockout (before and after components) and I just want to say that it's been quite a joy to work with the capabilities in knockout, i'm doing some pretty complicated maneuvers in this app and the observable technology and computed observables at the core made building this feel very natural to implement. Naturally there's been bumps in the road (such as the mistake I made related to this issue) but in the end it was always doing what it was expected to do.

Cheers!

chrisknoll commented 6 years ago

@krnlde : I didn't have a chance to use the fastForEach extension because I wasn't looping through a large number of items anywhere in my app (we resort to paging and filtering down to the 'current page view' when dealing with long lists). I'm sorry to hear that the progress stalled on getting that as the default implementation, but at least it's easy to add extensions into default knockout bindings (admittedly, you need to know that these extensions exist to take advantage of them).