CODEWITHSUNDEEP
javascriptjqueryperformance
DisgruntledGoat
DisgruntledGoat

Reputation: 72510

Why is this Javascript much *slower* than its jQuery equivalent?

I have a HTML list of about 500 items and a "filter" box above it. I started by using jQuery to filter the list when I typed a letter (timing code added later):

$('#filter').keyup( function() {
    var jqStart = (new Date).getTime();

    var search = $(this).val().toLowerCase();
    var $list = $('ul.ablist > li');

    $list.each( function() {
        if ( $(this).text().toLowerCase().indexOf(search) === -1 )
            $(this).hide();
        else
            $(this).show();
    } );

    console.log('Time: ' + ((new Date).getTime() - jqStart));
} );

However, there was a couple of seconds delay after typing each letter (particularly the first letter). So I thought it may be slightly quicker if I used plain Javascript (I read recently that jQuery's each function is particularly slow). Here's my JS equivalent:

document.getElementById('filter').addEventListener( 'keyup', function () {
    var jsStart = (new Date).getTime();

    var search = this.value.toLowerCase();
    var list = document.querySelectorAll('ul.ablist > li');
    for ( var i = 0; i < list.length; i++ )
    {
        if ( list[i].innerText.toLowerCase().indexOf(search) === -1 )
            list[i].style.display = 'none';
        else
            list[i].style.display = 'block';
    }

    console.log('Time: ' + ((new Date).getTime() - jsStart));
}, false );

To my surprise however, the plain Javascript is up to 10 times slower than the jQuery equivalent. The jQuery version takes around 2-3 seconds to filter on each letter, while the Javascript version takes 17+ seconds! I'm using Google Chrome on Ubuntu Linux.

This isn't for anything really important so it doesn't need to be super efficient. But am I doing something really dumb with my Javascript here?

Upvotes: 14

Views: 926

Answers (4)

alonisser
alonisser

Reputation: 12068

Another best practice for javascript speed is caching the list.length in a variable and calling the variable like:

l = list.length;
for (var i=0;i<l;i++):{ code here}

And maybe timing with jsperf would be better.

Upvotes: 4

Šime Vidas
Šime Vidas

Reputation: 185873

Here, I've refactored your code a bit:

var filter = document.getElementById( 'filter' ),
    ablist = document.querySelector( '.ablist' );

filter.addEventListener( 'keyup', function () {
    var re, elems, i, len, elem;

    re = RegExp( this.value, 'i' );
    elems = ablist.children;

    for ( i = 0, len = elems.length; i < len; i += 1 ) {
        elem = elems[i];       
        elem.style.display = 
                elem.textContent.search( re ) > -1 ? 'list-item' : 'none';
    }
}, false );

Live demo: http://jsfiddle.net/MVFxn/

Changes:

  • with a regular expression and an i flag, there's no need for toLowerCase,
  • if there is only one '.ablist' element on the page, querySelector should be the fastest way to grab it (since it aborts the query once it finds the first such element),
  • there's no query for the LI elements since the children property already references them conveniently.

I'd love to know how this code performs on your page...

Upvotes: 2

Emre Erkan
Emre Erkan

Reputation: 8482

I used while instead of for and did some minor improvements. Here is the final code.

var list = list = document.querySelectorAll('ul.ablist > li');
document.getElementById('javascriptFilter').addEventListener( 'keyup', function () {
    var jsStart = (new Date).getTime(),
        search = this.value.toLowerCase(),
        i = list.length - 1,
        listItem,
        result;
    while( i >= 0 )
    {
        listItem = list[i];
        if ( listItem.textContent.toLowerCase().indexOf(search) === -1 )
            listItem.style.display = 'none';
        else
            listItem.style.display = 'block';
        i--;
    }

    result = ((new Date).getTime() - jsStart);
    console.log(['Time: ', result, '<br />'].join(''));
}, false );

Upvotes: 0

Lycha
Lycha

Reputation: 10177

You could try using textContent instead of innerText , I think it should be faster. Also timing the list-generation and loop separately would tell if there is problem in list-generation.

Upvotes: 29

Related Questions