Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Can this jQuery be modified to run faster?

This following jQuery each function is causing IE8 to present a "Stop running script? .. A script on this page is causing your browser to run slowly.." message.

$('.expand-collapse-icon').each(function() {
    var dupId = $(this).parent('td').attr('duplicate-id');

    var len = $(".results-table tr")
        .filter(":not(:first)")
        .filter(":has(.hidden-row[duplicate-id='" + dupId + "'])").length;

    if (len <= 0) {
        $(this).hide();
        $(this).parent('td').css('padding-left', '15px');
    }
});

Basically, I have a number of visible rows (about 92) which have associated hidden rows. The rows are associated by duplicate-id Each visible row has an expand-collapse-icon in the first <td>. If you click the icon it will display the hidden rows. If the visible row has no associated hidden rows, I don't want to show the icon.

Ideally I could prevent the page from displaying the icon server-side if there's no associated rows, but there are dragons lurking in the code.

Is there any obvious way that I could speed this up?

like image 937
DaveDev Avatar asked Aug 30 '11 14:08

DaveDev


2 Answers

One thing for sure is to get rid of multiple $(this) - "cache" this statement value at the beginning of your function. For example:

var $this = $(this);

Now your script wraps the same object in jQuery 3 times for each iteration - messy!

Other thing that comes to my mind - in general, the .each method is slow, significantly slower than just simply iterating through result of $(".some-selector-example"). EDIT: I've found article 10 ways to instantly increase jQuery performance, while it's pretty old now (it's from 2009), most (if not all) of the tips are still recommended to use. Take especially closer look at the chart showing performance of .each method compared to regular loop over the same collection of data.

One last thing is probably the most performance-killing issue of your script - the len = $(".results-table tr").filter(":not(:first)") select. What you can do is to at least keep the result of that part of the select (keep this variable outside of function you are iterating in):

var cached_trs = $(".results-table tr").filter(":not(:first)");

Then in function you're iterating in operate with filter on you can do:

var len = cached_trs.filter(":has(.hidden-row[duplicate-id='" + dupId + "'])").length
like image 117
WTK Avatar answered Sep 23 '22 14:09

WTK


Try this. Here I have cached most of the repetitive element fetches, avoided filtering of rows by .hiddne-row class and search only once and then just filtering by attribute inside the loop which is a major improvement because attribute fetces are mostly slower comared to others. Use :gt(0) instead of `:not(:first)' that too only once outside the loop.

var $this, $parent, dupId, len ;

//This line will get all the rows from .resutls-table 
//filtered by ":not(:first):has(.hidden-row)"
//That way you dont have to search for trs and filter it everytime in the loop
var $resultsTr = $(".results-table tr.hidden-row:gt(0)");

$('.expand-collapse-icon').each(function() {
    $this = $(this);
    $parent = $this.parent('td');
    dupId = $parent.attr('duplicate-id');

    len = $resultsTr
        .filter("[duplicate-id='" + dupId + "'])").length;

    if (len <= 0) {
        $this.hide();
        $parent.css('padding-left', '15px');
    }
});
like image 41
ShankarSangoli Avatar answered Sep 21 '22 14:09

ShankarSangoli