4
\$\begingroup\$

I've got a list with about 120 items in IE and I have some items I want to activate. I compare, build and randomize some Arrays on the fly. The for loop in the run() function is really slow, and i tried to add a setTimeout(), like i've read somewhere on stackoverflow.

It does not seem to improve the performance, and if the requested list of items to be activated is too large, IE starts to give the slow script error.

Can i optimize this to something that'll work?

for(index = 0; index < 120; index++){
            wrapper.tempSlot.push(index);
}
Array.prototype.shuffle = function shuffle(){
    var tempSlot;
    var randomNumber;
    for(var i =0; i != this.length; i++){
        randomNumber = Math.floor(Math.random() * this.length);
        tempSlot = this[i]; 
        this[i] = this[randomNumber]; 
        this[randomNumber] = tempSlot;
    }
}

Array.prototype.compare = function(testArr) {
    if (this.length != testArr.length) return false;

    for (var i = 0; i < testArr.length; i++) {
        if (this[i].compare) { 
            if (!this[i].compare(testArr[i])) return false;
        }
        if (this[i] !== testArr[i]) return false;
    }
    return true;
}   


function checkAgainstEl(val){
    val = $($item.select).eq(val).data('personid')
    return val in oc([array, with, to, be, activated, numbers]);
}       

var index = 0;
var length = wall.globals.highlight.selection.length;
var run = function(){
    for(;index < length; index++){

        wrapper.tempSlot.shuffle();
        var pop     = wrapper.tempSlot.pop();
        var value   = wall.globals.highlight.selection[index];
        var newItem = $('#wallItem' + value);

        wrapper.objectsStr = wrapper.objectsStr + '#wallItem' + value + ', ';   

        while(checkAgainstEl(pop)){
            wrapper.tempSlot.shuffle()  
            pop = wrapper.tempSlot.pop();
        }

        beforeItem  = $($item.select).eq(pop);

        newItem.insertBefore(beforeItem);
        $($container.select).append(beforeItem);                

        if (index + 1 < length && index % 5 == 0) {
                setTimeout(run, 25);
            }               
    }
}
run();
\$\endgroup\$
  • \$\begingroup\$ I haven't looked at it too deeply, but just on the surface it looks like you're ultimately nesting 3 loops, each with a minimum of 120 members. This becomes 1,728,000 operations. it also looks like compare calls itself in yet another nest. My suspicion is that this is where your slowness is being introduced. \$\endgroup\$ – Joel Etherton May 6 '11 at 13:57
5
\$\begingroup\$
  1. Interacting with the DOM is one of the slowest operations in JavaScript. I'd suggest selecting all your elements before the loop, then referring to the cached elements inside.

  2. On the same token, do not perform any DOM manipulation in a loop if you can help it. Since it doesn't look like you can, another approach is to detach all DOM elements you're going to rearrange before the loop, perform the manipulation inside the loop, then re-attach them to the DOM at the end.

  3. A reverse while loop would be faster than the for.

  4. If you're using a version of jQuery prior to 1.6, the .data() method carries a lot of overhead. In checkAgainstEl(), use $.data($($item.select).get(val), 'personId') instead.

  5. It looks like you're re-wrapping jQuery objects in new jQuery objects unnecessarily, and in multiple places. Like in #4. I'm assuming variables prefixed with a $ are jQuery objects; if they are, no need to wrap them in $() again.

Your biggest problem is the DOM interaction inside a loop. Cut that out any way you can and you'll be golden.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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