CODEWITHSUNDEEP
jqueryperformancehtml-table
Steven
Steven

Reputation: 439

Very slow function, is there a faster way? (using jQuery 1.4.2)

The following function needs at least 3 seconds to run (on 500 table rows). Is it possible to make this function faster?

function prepareTable() {
    var groupIndex = 0;
    $("#row tbody tr").each(function(index) {
    // each row gets a unique id
    // remove default css styles for table rows
    // read out hidden value, that stores if row is a group
    var group = $(this).attr('id', 'node-'+index).removeClass("odd event").find('td :hidden').attr('value');
    // if it is a group, add special styles to row and remember row index
    if (group == 'true') {
        groupIndex = index;
        $(this).addClass('odd').find("td:first")
            .mouseenter(function() {
                $(this).parent().addClass("swGroupLink");
            })
            .mouseleave(function() {
                $(this).parent().removeClass("swGroupLink");
        });
    } else {
        // make all following rows to children of the previous group found
        $(this).addClass('even child-of-node-' + groupIndex);
    }   
    });
}

Upvotes: 3

Views: 305

Answers (3)

Ayaz Alavi
Ayaz Alavi

Reputation: 4835

you can take mouseenter and mouseleave outside with live event, so that it will not execute with prepareTable function and you can put it in document ready function.

 $("#row tbody tr td.trueInPrepareTable")
  .live("mouseenter", function(event){      
               $(this).parent().addClass("swGroupLink");    
    }).live("mouseleave", function(event){      
               $(this).parent().removeClass("swGroupLink");    
    });

instead of fetching group value from hidden field, put this value in the rel,rev or title attribute.

    function prepareTableEdit() {
                var groupIndex = 0;
                $("#row tbody tr").each(function(index) {
                     groupIndex = index;
                     $(this).attr('id', 'node-'+ groupIndex ).removeClass("odd even");
                    if($(this).attr('rel') == 'true')
                    {                           
                        $(this).addClass('odd').find("td:first").addClass('trueInPrepareTable');                      }
                    else
                    {
                         $(this).addClass('even child-of-node-' + groupIndex).find("td:first").removeClass('trueInPrepareTable');  
                    }
                });

 }

check out http://www.jsfiddle.net/raBGq/

Upvotes: 1

jAndy
jAndy

Reputation: 236012

I suggest two improvements:

  • Cache DOM References
  • Work at your table offline

Example

function prepareTable() {
   var groupIndex = 0;
   var $mytable = $('#row'),
       $parent  = $mytable.parent();

   $mytable = $mytable.detach();

   $mytable.find('tr').each(function(index) {
      var $this = $(this);
      var group = $this.attr('id', 'node-'+index).removeClass("odd event").find('td :hidden').attr('value');
// if it is a group, add special styles to row and remember row index
   if (group == 'true') {
       groupIndex = index;
       $this.addClass('odd').find("td:first")
           .mouseenter(function() {
               $this.parent().addClass("swGroupLink");
           })
           .mouseleave(function() {
               $this.parent().removeClass("swGroupLink");
       });
   } else {
       // make all following rows to children of the previous group found
       $this.addClass('even child-of-node-' + groupIndex);
   }   
   });

   $parent.append($mytable);
}

I added a variable $this which caches $(this) in your .each() loop. I also added $mytable and $parent. $mytable stores the #row element and $parent stores the parent-node from #row. That is because I remove the whole element from the DOM, do the work and re-attach it to the parent.

Test environment: http://www.jsfiddle.net/2C6fB/4/

If that is still too slow, you have other options here. First, look if you can split the loop into smaller pieces. You can optimize that like a lot by using asychronous callbacks, for instance, setTimeout. That can be a tricky business and I would need to know your code in more detail, but in general you might just want to wrap your whole loop into single setTimeout() functions. Example -> http://www.jsfiddle.net/2C6fB/5/

This ensures that the browser won't "hang" while operating. But of course this took a little bit longer to complete the whole task.

Upvotes: 6

Stefanvds
Stefanvds

Reputation: 5916

i guess the find is the one where all the times goes lost.

can you not make a selector of it in stead of a find. what's the HTML?

Upvotes: -1

Related Questions