2

I have a page that is using jQuery to load an XML file, which I'm then outputting the contents of to the page.

Recently I added a sorting function to the output which is causing a 1+ or 2+ minute hang on Safari on an iPod Touch (depending upon how many fields I sort by) and a less than 1 minute hang on an iPad. The same sort returns within a few seconds on Firefox 4.0.1.

I'm afraid it's just a limitation of the iOS, but before I removed the sort, perhaps there's an optimization that could be made.

Before the filter there's 357 items in the XML. After the filter there's 199 items that are sorted through.

var videoGames = $($.parseXML(videoGameXml)).find("game");
videoGames = videoGames.filter(function (a) {
    return ($(this).attr('addOn') != "true" && $(this).find('own').text() == "yes");
});
videoGames.sort(function (a, b) {
    var firstTitle = $(a).find('title').text().toLowerCase();
    var secondTitle = $(b).find('title').text().toLowerCase();
    var firstSystem = ($(a).find("console").text() + " " + $(a).find("version").text()).toLowerCase();
    var secondSystem = ($(b).find("console").text() + " " + $(b).find("version").text()).toLowerCase();

    if (firstSystem != secondSystem) {
        if (firstSystem > secondSystem) {
            return 1;
        } else {
            return -1;
        }
    } else {
        if (firstTitle > secondTitle) {
            return 1;
        } else if (secondTitle < firstTitle) {
            return -1;
        }
    }
    return 0;
});
videoGames.each(function () {
    // runs quickly, so removed
});

Note that if I remove the system check as an initial 'optimization' that cuts the time about in half on the iPod Touch, but still results in the 1+ minute hang mentioned above.

So, is it an iOS device limitation, or can I optimize my sort?

ariel
  • 15,620
  • 12
  • 61
  • 73
James Skemp
  • 8,018
  • 9
  • 64
  • 107
  • It's also worth nothing that iOS Safari also seems to be taking shortcuts sorting data. While Firefox 4.0.1 displays the data in the correct order, Safari does the first sort perfectly, but starts falling apart on the titles (inconsistent - titles are getting moved around). – James Skemp May 08 '11 at 19:07

2 Answers2

2

Every time you do $(a) it will perform a very complex set of operations, so you better cache it. Also, you don't need the Title if System is different. This version should speed it up a bit:

videoGames.sort(function (a, b) {
    var first = $(a);
    var second = $(b);
    var firstSystem = (first.find("console").text() + " " + first.find("version").text()).toLowerCase();
    var secondSystem = (second.find("console").text() + " " + second.find("version").text()).toLowerCase();

    if (firstSystem != secondSystem) {
        if (firstSystem > secondSystem) {
            return 1;
        } else {
            return -1;
        }
    } else {
        var firstTitle = first.find('title').text().toLowerCase();
        var secondTitle = second.find('title').text().toLowerCase();

        if (firstTitle > secondTitle) {
            return 1;
        } else if (secondTitle < firstTitle) {
            return -1;
        }
    }
    return 0;
});

You could also cache the values in the object

Then, instead of:

var firstSystem = (first.find("console").text() + " " + first.find("version").text()).toLowerCase();

Do:

var firstSystem = first.data('system');
if (!firstSystem) {
    firstSystem = (first.find("console").text() + " " + first.find("version").text()).toLowerCase();
    first.data('system') = firstSystem;
}
ariel
  • 15,620
  • 12
  • 61
  • 73
  • I was going to answer the same. You could also possibly cache the title and system on the object and only do the find if its not already been retrieved. – John Hoven May 08 '11 at 18:19
  • Duh. Of course I shouldn't have been doing those every time. Your initial version cut the time down in half. Now I'll give your edits a try. – James Skemp May 08 '11 at 18:47
  • james, i think hash table here is not a good idea.. edited to put the text directly in the object. see if it works – ariel May 08 '11 at 18:58
  • Looks like I had to do `first.data('system', firstSystem)` instead of `first.data('system') = firstSystem` but that did knock it down to around 20 seconds. Wish I could give you another up vote, since you've given effectively two good answers. Thanks! – James Skemp May 10 '11 at 12:10
1

You should move any selectors calls like this:

var firstTitle = $(a).find('title').text().toLowerCase();

out from the comparator function. The comparator function is supposed to be lightweight.

Either use children(), next() and the like or scan you set once and create an array of keys upfront and then sort it using those keys.

The comparator function will be called 2n * ln(n) times (depends on algorithm used) where n is a number of elements in a set. So your code does the same expensive calculations twice at least.

c-smile
  • 26,734
  • 7
  • 59
  • 86
  • Using children() is a good call, but can you clarify/expand on the first part of your answer? – James Skemp May 08 '11 at 18:48
  • 1
    @James Skemp: scan the set and create `el.attr("data-key", key)` for each element. Then do `videoGames.sort()` on that key. And yet I suspect that you can just use `data-title`, `data-console`, etc. attributes on the list elements themselves. In this case for sorting you will not need selectors at all. – c-smile May 08 '11 at 20:04