2

Does anyone know why the performance on this page is slow when it comes to the dropdown list on the - ALL - option? I must be doing something wrong with knockout.js for this to happen. For the smaller list of games it opens up quickly.

Tournament Schedule

Javascript

(function (app, $, undefined) {

    app.viewModel = app.viewModel || {};

    function Schedule() {

        var self = this;

        self.loaded = ko.observable(false);
        self.divisionId = ko.observable();
        self.games = ko.observableArray(null);

        self.search = function(url) {
            app.call({
                type: 'POST',
                data: { divisionId: self.divisionId() },
                url: url,
                success: function (result) {
                    self.games([]);
                    self.games.push.apply(self.games, result);
                    self.loaded(true);
                }
            });
        };

        self.init = function (options) {
            app.applyBindings();
        };

    };

    app.viewModel.schedule = new Schedule();

} (window.app = window.app || {}, jQuery));

Template

     <div class="games hidden" data-bind="if: schedule.games(), css: { 'hidden': !schedule.games() }">
            <div data-bind="if: schedule.games().length > 0">
                <div data-bind="foreach: schedule.games">

                    <h2><span data-bind="html: Name"></span></h2>
                    <hr />
                    <div class="games row" data-bind="foreach: Games">
                        <div class="span4">
                            <div class="game game-box new-game-box">
                                <div class="datetime-header clearfix new-game-box">
                                    <span class="time"><span data-bind="html: DateFormatted"></span> - <span data-bind="html: TimeFormatted"></span></span>,
                                    <span class="gym" data-bind="text: Venue"></span>
                                </div>
                                <div class="team-game clearfix new-game-box" data-bind="css: { winner: AwayTeamIsWinner }">
                                    <span class="team">
                                        <a target="_blank" href="#" data-bind="html: AwayTeamName, attr: { href: AwayTeamLink }"></a>
                                    </span> <span class="score" data-bind="html: AwayTeamScoreDisplay"></span>
                                </div>
                                <div class="team-game clearfix new-game-box" data-bind="css: { winner: HomeTeamIsWinner }">
                                    <span class="team">
                                        <a href="#" target="_blank" data-bind="html: HomeTeamName, attr: { href: HomeTeamLink }"></a>
                                    </span> <span class="score" data-bind="html: HomeTeamScoreDisplay"></span>
                                </div>
                                <div class="buttons clearfix">

                                    <span class="division" data-bind="html: 'Division ' + DivisionName"></span>, 
                                    <span data-bind="text: GameTypeName"></span>
                                    <div class="btn-group">
                                        <a rel="nofollow, noindex" title="Add to calendar" href="#" class="btn btn-mini" data-bind="attr: { href: CalendarLink }"><i class="icon-calendar"></i></a>
                                        <a target="_blank" title="Gym Details" href="#" class="btn btn-mini" data-bind="attr: { href: GymLink  }"><i class="icon-map-marker"></i></a>
                                    </div>
                                </div>
                            </div>
                        </div>
                    </div>
                </div>
            </div>
        </div>
        <div class="hidden" data-bind="if: (schedule.games() && schedule.games().length == 0), css: { 'hidden': !schedule.games() }">
            No games found for this event.
            Scores will be available here the day before the event however the schedule might already be posted under <a href="@Url.Action(MVC.Event.Documents(Model.Event.Id, Model.Event.Slug))">documents</a>.

        </div>
 <script type="text/javascript">
        app.viewModel.schedule.init({});
    </script>
Michael Best
  • 16,623
  • 1
  • 37
  • 70
Mike Flynn
  • 22,342
  • 54
  • 182
  • 341
  • Does everything speed up if you comment out `self.games.push.apply(self.games, result);` ? – Jamie Dixon Nov 09 '13 at 22:15
  • If that turns out to be the case you might consider using `concat` rather than the cached array apply method. This test suite here shows it to be 14% faster than cached apply: http://jsperf.com/array-prototype-push-apply-vs-concat/4 – Jamie Dixon Nov 09 '13 at 22:17
  • I mean if you choose division 14 on the dropdown list that contains way less items for that array, it does get alot faster, so yea that is the issue, but if knockout.js performs like this I might was well use pure javascript with jquery and build my elements. – Mike Flynn Nov 09 '13 at 22:21
  • To be honest though, your entire page is slow. I've got 8GB Ram here and your page is crashing Chrome for me. Scrolling on the page is virtually impossible. – Jamie Dixon Nov 09 '13 at 22:22
  • I dont know about the concat, but I know the push.apply is a best practice for knockout.js and performance, http://www.knockmeout.net/2012/04/knockoutjs-performance-gotcha.html. – Mike Flynn Nov 09 '13 at 22:22
  • Yes, calling knockouts push method is the preferred way. I think there's more going on with your page though. I suspect this isn't a knockout issue per se. – Jamie Dixon Nov 09 '13 at 22:25
  • I disabled the .push and it works like normal with the dropdown speed. I am thinking maybe it has to do with the large template it reuses, not sure. – Mike Flynn Nov 09 '13 at 22:27
  • When I remove the template inside my knockout.js foreach loop it also works fine. – Mike Flynn Nov 09 '13 at 22:30
  • Try to remove all the whitespace and indentation from your tempalte. So your code should be one long line `
    ...
    `
    – nemesv Nov 13 '13 at 06:09
  • Are you serious? That cant be the issue.... – Mike Flynn Nov 13 '13 at 16:11
  • Can you work with self.games as an observable instead of an observable array? I think part of the issue is also the sheer number of items in the DOM, which is why scrolling is incredibly slow. I'm not sure how useful it is to even have all that data show at once. – cjd82187 Nov 13 '13 at 17:53
  • I just removed the CSS file with Chrome's inspector, and it scrolls and performs much better. Leading me to believe that its an issue with the number of items in the DOM. – cjd82187 Nov 13 '13 at 17:57
  • I made it an observable and its still the same issue. Yes removing the stylesheet made it exactly what I want. Why is this and what is a fix for this???? – Mike Flynn Nov 13 '13 at 23:04
  • I removed the inner template inside the games foreach, and its really fast. Should I be using a different knockout.js template engine or doing some other technique? – Mike Flynn Nov 13 '13 at 23:18
  • observations: 1. in fact, the push.apply method is not recommended here, as said by Niemeyer and the end of his post. For replacing the content of the obsArray, replace `self.games([]); self.games.push.apply(self.games, result);` with `self.games(result)` 2. the if-binding on line one of your HTML can be removed. It is always true 3. the second if-binding is essentially caught by your foreach binding. That whole line of code (div and binding) can be removed. 4. As was said, a lot of elements and styling will indeed slow things down. I don't see the inner template, where are you using it? – Hans Roerdinkholder Nov 14 '13 at 14:26
  • Its pasted above in the html. I think my best bet is to add another filter, like maybe on days.... – Mike Flynn Nov 14 '13 at 21:15
  • The problem doesn't appear to have anything to do with Knockout. – Michael Best Nov 14 '13 at 21:59
  • Agreed. I think you're just showing too much at a time, or at least the markup to do so is too 'expensive'. All benefits you can get from optimizing your knockout-code will not give significant performance improvements. Another filter would help, but what about paging the results? You can't view thousands of records on one screen anyway. – Hans Roerdinkholder Nov 14 '13 at 22:17
  • You should think about add paging with infinite-scrolling! that would speed up as fck – john Smith Nov 15 '13 at 00:45

3 Answers3

5

I downloaded your HTML and CSS and did some testing. I was able to fix the problem by removing the following CSS:

.ui-widget :active {
    outline: none
}

To test this on the current page, execute document.styleSheets[0].deleteRule(23) in the console.

Some more testing showed that the drop-down is only slow in Chrome (30). Firefox (23) and IE (10) don't have the problem.

Michael Best
  • 16,623
  • 1
  • 37
  • 70
  • 3
    Pretty cool of Michael to spend the time clearing this up for you when it seems to be 100% not KO related. – PW Kad Nov 15 '13 at 04:22
  • I got a bit lucky trying to find the problem. I first used a CSS formatter that messed up some the CSS (unknown to me). When I noticed that the page no longer had the problem, I found and used a better CSS formatter and then compared the two versions. The bad formatter has changed the above to `.ui-widget:active`, effectively negating the performance issue. – Michael Best Nov 15 '13 at 08:54
  • Wow, just wow. Why would this cause the issue? I would of never figured this out. Thank goodness it wasnt knockout.js, and other large datasets on my pages could be caused by this too. Do you know why this is the case? – Mike Flynn Nov 15 '13 at 19:02
  • Selectors are calculated from right to left. In your case, first there will be a search for all active elements in the DOM tree. Then for each one it will be checked that his parents have the right class. The ":active" selector is not a simple selector for which browsers are optimized. So it will be a slow complete enumeration of all elements. Therefore, this rule is very slow down. – Enyby Mar 16 '20 at 17:06
1

You may suffer from performance problems when manipulating large or rich (containing complex objects) observable arrays. Any time you perform any operation on such array, all the subscribers get notified.

Imagine you are inserting 100 items into an observable array. More often than not, you don’t need each subscriber to recalculate it’s dependencies 100 items, and UI to be reacting 100 times. Instead, once should just fine.

To do this, you can always modify the underlying array instead of the observableArray directly, since observableArray concept is just a function wrapper around the traditional JS array. After you are done with the array manipulation, you can then notify all the subscribers that the array has changed its state with .valueHasMutaded()

. See the simple example:

success: function (result) {
    ko.utils.arrayPushAll(self.games, result);
    self.games.valueHasMutated();
    ....

cheers

john Smith
  • 17,409
  • 11
  • 76
  • 117
0
  1. There are too many dom element at the page, it will be hard to select element for jquery.
  2. If you need to handle big data bound after ajax, you'd better add a new thread to do it. in ajax success function:

    setTimeout(function(){ // your code }, 100);

for No.1, why not add a pager? Long long scroll bar is very terrible.

Teddy
  • 787
  • 5
  • 13
  • JavaScript is single threaded, and always is. The setTimeout(...) simply forces a pause (guaranteed minimum length of delay, but could be longer depending on queue) and queue of the work in question. – pim Dec 24 '15 at 19:06