2
\$\begingroup\$

I have a script for a countdown function, which is using massive amounts of CPU power on my MBP.

The script updates the countdown display every second and is updating all piecharts as well.

    var options = {
        scaleColor: false,
        trackColor: 'rgba(255,255,255,0.3)',
        barColor: '#E7F7F5',
        lineWidth: 6,
        lineCap: 'butt',
        size: 95
    };

    $('#days').easyPieChart(options);
    $('#hours').easyPieChart(options);
    $('#minutes').easyPieChart(options);
    $('#seconds').easyPieChart(options);

function countdown(endT,callback) {
        var first_load = false;
        var days,hours,minutes,sec,timer;

        end = new Date(endT);

        end = end.getTime();                     //Get initial Date in Milliseconds
        if (isNaN(end)) {
            return;

        }
        var tot_current = new Date();
        var tot_remain = parseInt((end - tot_current.getTime())/1000);
        var tot_days = parseInt(tot_remain/86400);

        timer = setInterval(calculate,1000);
        function calculate(){
            var current = new Date();
            var remaining = parseInt((end - current.getTime())/1000); //remaining seconds 

            if (remaining <= 0){
                clearInterval(timer);
                days=0;
                hours=0;
                minutes=0;
                sec=0;
                display(days,hours,minutes,sec);
                if (typeof callback === 'function' ) {
                    callback();
                }

            }else{

                days = parseInt(remaining/86400);
                remaining = (remaining%86400);
                hours = parseInt(remaining/3600);
                remaining = (remaining%3600);
                minutes = parseInt(remaining/60);
                remaining = (remaining%60);
                sec = parseInt(remaining);
                display(days,hours,minutes,sec);

            }
        }
        function display(days,hours,minutes,sec) {
            var dl = days.toString().length;
            if (dl == "1") {
                sl = 2;
            }else{
                if (isNaN(dl)) {
                    sl = 3;
                }
                sl = dl;
            }

            days_rem = ("00"+days).slice(-sl);
            hrs_rem = ("0"+hours).slice(-2);
            min_rem = ("0"+minutes).slice(-2);
            sec_rem = ("0"+sec).slice(-2);
            $("#days span").text(days_rem);
            $("#hours span").text(hrs_rem);
            $("#minutes span").text(min_rem);
            $("#seconds span").text(sec_rem);   
            $("#days").data('easyPieChart').update((100/tot_days)*days_rem);

            // Disable animation for the first load                     
            if(hrs_rem == 23 && first_load) { $('#hours').data('easyPieChart').disableAnimation(); }
            $("#hours").data('easyPieChart').update((100/23)*hrs_rem);
            if(hrs_rem == 23 && first_load) { $('#hours').data('easyPieChart').disableAnimation(); }

            if(min_rem == 59 && first_load) { $('#minutes').data('easyPieChart').disableAnimation(); }
            $("#minutes").data('easyPieChart').update((100/59)*min_rem);
            if(min_rem == 59 && first_load) { $('#minutes').data('easyPieChart').enableAnimation(); }

            if(sec_rem == 59 && first_load) { $('#seconds').data('easyPieChart').disableAnimation(); }
            $("#seconds").data('easyPieChart').update((100/59)*sec_rem);
            if(sec_rem == 59 && first_load) { $('#seconds').data('easyPieChart').enableAnimation(); }

            first_load = true;
        }

    }

    var d = new Date(2015, 6, 10, 12, 7, 0, 0);
    countdown(d,null);

The code can be seen in action here.

\$\endgroup\$
6
  • \$\begingroup\$ Are you asking for a way to reduce the CPU performance or a review of the code? The CPU issue is fairly straight forward "fix". \$\endgroup\$ Commented Jul 7, 2015 at 18:37
  • \$\begingroup\$ @GaryStorey, both to be honest. I guess that a review would lead to the error? I cannot determine it.. \$\endgroup\$
    – JavaCake
    Commented Jul 7, 2015 at 18:48
  • 1
    \$\begingroup\$ OK. I'll add a review (including fix). \$\endgroup\$ Commented Jul 7, 2015 at 18:52
  • \$\begingroup\$ @GaryStorey, sounds good! \$\endgroup\$
    – JavaCake
    Commented Jul 8, 2015 at 4:59
  • \$\begingroup\$ @GaryStorey, if you need more info, let me know! \$\endgroup\$
    – JavaCake
    Commented Jul 8, 2015 at 14:11

1 Answer 1

3
\$\begingroup\$

First, I'll get to the fix for your problem. The problem is more in the CSS than in your JavaScript. Because you are updating the graph every second, that area of the page has to be repainted each time. Due to the way browsers handles these things by default, it has to also check the surrounding area to see if it now needs to be changed as well. So the idea is to minimize this. And the simplest way to do this is to add the following lines into your CSS:

.chart {
  transform: translateZ(0);
  z-index:1;
  /* future proofing */
  will-change: transform;
}

What this code does is move the charts onto there own rendering layer. This prevents the browser from having to check the surrounding areas because each chart is now separated from the rest of the content. In Chrome, you can see the performance difference this makes by watching the Timeline in the Dev tools. In the original, it spends ~180 ms repainting the screen and ~100 ms performing calculations. In the updated version, it spends ~20 ms for each. Add this over four graphs and you can see why the cpu is pegged. The will-change is a new CSS property that will basically do the same thing as the two lines above in the somewhat-near-future. I have updated the fiddle with just this change and you can see the difference.

Now, the code review:

As always, the first thing I recommend is to remove your code from the global scope. The easiest way to do this is to use an IIFE. This will basically create a private scope for all your code which reduces the amount of code you place in the global scope, which in turn, reduces the chance of collisions with other peoples code. Since you are using jQuery, this also provides a way to make sure $ always points to it.

(function( $, window, document, undefined ) {
  //your code here
})( jQuery, window, document );

So in the above statement, we are passing jQuery as $, as well as the window and document objects (for a slight performance boost) and, to deal with older browsers, making sure undefined is actually undefined.

The next thing you could do is cache your jQuery selectors. This will improve the overall performance because selecting an element in the DOM is one of jQuery's slowest performing functions. Your are using ID's which help but caching them is even quicker. We want to do this as soon as the DOM is ready. I usually create a single function for everything that has to happen there and then call the single function. Also, since we will be referring to these selectors we will declare them immediately inside of our IIFE. I have also included the reference to create the piecharts:

(function( $, window, document, undefined ) {

  var $days, $daysText, $hours, $hoursText,
      $minutes, $minutesText, $seconds, $secondsText;

  function init() {

    var options = {
      scaleColor: false,
      trackColor: 'rgba(255,255,255,0.3)',
      barColor: '#E7F7F5',
      lineWidth: 6,
      lineCap: 'butt',
      size: 95
    };

    getSelections();
    
    $days.easyPieChart( options );
    $hours.easyPieChart( options );
    $minutes.easyPieChart( options );
    $seconds.easyPieChart( options );

  }

  function getSelections() {

    $days = $('#days'),
    $daysText = $days.find('span'),
    $hours = $('#hours'),
    $hoursText = $hours.find('span'),
    $minutes = $('#minutes'),
    $minutesText = $minutes.find('span'),
    $seconds= $('#seconds'),
    $secondsText = $seconds.find('span');

  }


  $(function() {   //shortcut for document.ready event
    init();
  });

})( jQuery, window, document );

One other big change you need to make is to not update all four pie charts every second. There is only one chart that needs updating that often (seconds obviously). You should only update the minutes chart every minute, the hour every hour and the days every day. That way, on almost every interaction you are only updating the one chart.

Another way you could improve this code is to DRY it out a bit. One place that would greatly benefit from this is the display function. We need to make changes here anyway to help out with the problem mentioned above.

Let's take this code:

days_rem = ("00"+days).slice(-sl);
hrs_rem = ("0"+hours).slice(-2);
min_rem = ("0"+minutes).slice(-2);
sec_rem = ("0"+sec).slice(-2);
$("#days span").text(days_rem);
$("#hours span").text(hrs_rem);
$("#minutes span").text(min_rem);
$("#seconds span").text(sec_rem);   

Notice how that is pretty much the same thing repeated over and over? That means it needs to be it's own function:

updateText( $daysText, days );
updateText( $hoursText, hours );
updateText( $minutesText, minutes );
updateText( $secondsText, seconds );

function updateText( $el, val) {
  var len = val.length;
  val = ( len === 1 ) ? '0'+val : val;
  $el.text( val );
}

With this function, val will always be at least 2 characters long.

Now before updating the charts, check to see what are current value is. If it's the same as the new value (which will be typical for minutes, hours, days), then there is no reason to update them (or there text for that matter). Adding a data element with jQuery is a great way to keep track of this.

if ( $days.data('remaining') !== days_rem) {
  // update it
  updateText( $daysText, days );
  $days.data('remaining', days_rem);  
  $days.data('easyPieChart').update( ( 100 / tot_days) * days_rem );     
}

The first time through, the remaining data element is not set so it will update the chart. Each other time through, it will check that existing value and only update the chart when the value actually changes. Once the code is added for each chart, it becomes obvious that this can be DRY-ed out as well. Maybe with something like this:

updateChart( $days, $daysText, days_rem, ( 100 / tot_days ) * days_rem );
function updateChart( $chart, $chartText, newValue, formula ) {
  //code here
}

I will leave that code up to you.

At this point, you should see a noticeable increase in the performance of the page.

Some other things to consider:

In your calculate function you have a bunch of Magic Numbers such as 86400. Although this is fairly straight forward when you know what you are looking at, it might be easier to set up some variables to hold those that provide some explanation for them:

var SECONDS_IN_A_DAY = 86400;
days = parseInt( remaining / SECONDS_IN_A_DAY , 10 );

Also notice, the radix parameter for the parseInt function. It isn't required but it's one of those things you should use anyway. That way the parser doesn't take the extra time to figure this out on it's own.

I hope that helps! Feel free to post any comments or questions.

Updates based on comments/fiddle

I have created a fiddle with all of the updated code, including some of the things you were trying to do on your own fiddle. To show you the performance improvements, I created a couple of screen captures. Note these are both showing when the seconds chart goes from 1 to 59.

This first one is your original fiddle linked in your post. Performance Before

This is the updated fiddle I linked to above. Performance After

By reducing the number of times each of the charts have to update and using the CSS above, you can see the execution time has improved significantly. These samples were taken with Chrome dev tools. If you are seeing significant CPU thrashing, try running in an incognito window to see if it is something else (tab or extention) interfering with your performance.

\$\endgroup\$
6
  • \$\begingroup\$ Very nice description. I tried to do all the fixes as good as i could, except for the updateChart function you suggested. Although i am still experiencing that Chrome is going a bit crazy when i open the webpage. The JS code is here jsfiddle.net/qghqdm5b and i have added the additional CSS code as you described. \$\endgroup\$
    – JavaCake
    Commented Jul 8, 2015 at 20:11
  • \$\begingroup\$ @JavaCake I will check it out tomorrow. Leaving for the day. \$\endgroup\$ Commented Jul 8, 2015 at 21:18
  • \$\begingroup\$ Ok, so i took the time to implement the updateChart. jsfiddle.net/qghqdm5b/4 \$\endgroup\$
    – JavaCake
    Commented Jul 9, 2015 at 6:24
  • \$\begingroup\$ @JavaCake See updates in answer. \$\endgroup\$ Commented Jul 9, 2015 at 16:17
  • 1
    \$\begingroup\$ @JavaCake Using the timeline feature in Chrome Dev tools. Go to that tab, click on the record button for however long you want to test, then click stop. It will show you overall performance or you can select a specific timeframe from the recording. \$\endgroup\$ Commented Jul 9, 2015 at 16:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.