2

Please forgive me if this is answered on SO somewhere already. I've searched, and it seems as though this is a fairly specific case.

Here's an example of the JSON (NOTE: this is very stripped down - this is dynamically loaded, and currently there are 126 records):

var layout = {
"2":[{"id":"40","attribute_id":"2","option_id":null,"design_attribute_id":"4","design_option_id":"131","width":"10","height":"10",
            "repeat":"0","top":"0","left":"0","bottom":"0","right":"0","use_right":"0","use_bottom":"0","apply_to_options":"0"},
    {"id":"41","attribute_id":"2","option_id":"115","design_attribute_id":"4","design_option_id":"131","width":"2","height":"1",
            "repeat":"0","top":"0","left":"0","bottom":"4","right":"2","use_right":"0","use_bottom":"0","apply_to_options":"0"},
    {"id":"44","attribute_id":"2","option_id":"118","design_attribute_id":"4","design_option_id":"131","width":"10","height":"10",
            "repeat":"0","top":"0","left":"0","bottom":"0","right":"0","use_right":"0","use_bottom":"0","apply_to_options":"0"}],
"5":[{"id":"326","attribute_id":"5","option_id":null,"design_attribute_id":"4","design_option_id":"154","width":"5","height":"5",
            "repeat":"0","top":"0","left":"0","bottom":"0","right":"0","use_right":"0","use_bottom":"0","apply_to_options":"0"}]
};

I need to match the right combination of values. Here's the function I currently use:

function drawOption(attid, optid) {
    var attlayout = layout[attid];
    $.each(attlayout, function(k, v) {
        // d_opt_id and d_opt_id are global scoped variable set elsewhere
        if (v.design_attribute_id == d_att_id
            && v.design_option_id == d_opt_id  
            && v.attribute_id == attid 
            && ((v.apply_to_options == 1 || (v.option_id === optid)))) {
                // Do stuff here
        }
    });
}

The issue is that I might iterate through 10-15 layouts (unique attid's), and any given layout (attid) might have as many as 50 possibilities, which means that this loop is being run A LOT.

Given the multiple criteria that have to be matched, would an AJAX call work better? (This JSON is dynamically created via PHP, so I could craft a PHP function that could possibly do this more efficently), or am I completely missing something about how to find items in a JSON object?

As always, any suggestions for improving the code are welcome!

EDIT:
I apologize for not making this clear, but the purpose of this question is to find a way to improve the performance. The page has a lot of javascript, and this is a location where I know that performance is lower than it could be.

random_user_name
  • 25,694
  • 7
  • 76
  • 115
  • 1
    You may use `jsonpath` library. hardly it will be faster, but you'll have simpler code. – kirilloid Mar 29 '12 at 19:36
  • Are you concerned about readability, performance, or some other metric? – Dan Davies Brackett Mar 29 '12 at 19:53
  • The primary concern is performance. Readability is secondary. – random_user_name Mar 29 '12 at 20:05
  • jsonpath : Data may be interactively found and extracted out of JSON structures on the client without special scripting.[link]http://goessner.net/articles/JsonPath/ – Milindu Sanoj Kumarage Mar 30 '12 at 02:49
  • 1
    If the code is working and you just want to improve this, you should post it on http://codereview.stackexchange.com. – James Montagne Mar 31 '12 at 21:19
  • What is "do stuff" here? – Raynos Mar 31 '12 at 23:11
  • Do stuff is lightweight, and is not the performance concern. The concern is finding the correct record on which to "do stuff" – random_user_name Apr 01 '12 at 02:58
  • @Montagne - thanks! That tip is excellent - I had no idea about codereview.stackexchange.com, but that IS Exactly where this belongs. Should I leave it here since I've started the bounty, or should it be moved? – random_user_name Apr 01 '12 at 03:01
  • Have you tried [HTML 5 web workers](http://www.whatwg.org/specs/web-apps/current-work/multipage/workers.html)? I was able to do a jQuery UI autocomplete implementation off of tens of thousands of rows pretty easily by taking it off the UI thread. – Paul Tyng Apr 01 '12 at 15:53
  • if you really care about performance why you don't do that server side? – Luca Filosofi Apr 01 '12 at 16:59
  • @aSeptik - thank you. I am a huge proponent of performing as much work as possible on the server side, however this is a very dynamic page with a lot of interaction that must be done via javascript. With that being the case, and as much javascript as it does, I'm trying to make it as high performance as possible. – random_user_name Apr 02 '12 at 02:07
  • @cale_b: i really think your problem is not the loop itself, i guess your are doing something wrong in the entire process; i know that the question is related to loop performance but there is nothing faster then a for() loop. so if you really want encrease performace you need to have a better process pattern. can you post more code? – Luca Filosofi Apr 02 '12 at 08:00

9 Answers9

7

First and foremost you should measure and only act if there is a real performance concern. You need exact numbers like 200ms or 80% time is spent there. "It runs a lot" doesn't mean anything. The browser can loop very fast.

You can improve constant factors as others mentioned, like using a native for loop instead of jQuery.each. Chaching global variables won't help you too much in this case.

If you really want to improve efficency you should find a better algorithm than O(n). Assuming that you only use this data for finding elements matching a certain criteria you can use JS objects as hashes to achive a O(1) performance.

Just an example for you specific case:

var layout = {
  "2": { "2,,4,131,10,0": ["40", "93"], "2,115,4,131,0": ["41"] },
  "4": { ... },
  ...
};

It's fairly easy to generate this output in php, and then you just use a lookup to find ids matching your particular criteria.

gblazex
  • 49,155
  • 12
  • 98
  • 91
  • I agree that with modern interpreters caching the global references isn't going to help much. However, it's pretty much the only thing that can be suggested given the information in the question. Other than starting with pared down data, of course. But he already figured that bit out. – James Sumners Apr 02 '12 at 22:48
  • I am inclined to accept this answer, but have a question: could you provide a simple code example of what the lookup code would look like? This JSON would be trivial to generate in php as you mention, but I would greatly appreciate an example of what the javascript would look like that seeks/finds the appropriate record based on the O(1) example you provided above. – random_user_name Apr 03 '12 at 18:52
  • 1
    So, with the help of the other answers, I stumbled upon how to use your code. It is hands-down the fastest: for 100,000 iterations, my original code took 2304ms, the answer by Colin Godsey took 120ms, and this code took only 82ms. – random_user_name Apr 05 '12 at 22:58
  • Sorry I haven't been online for a couple of days. Good to see you could work it out in the end. :) – gblazex Apr 08 '12 at 17:28
3

IMHO, a simple hashmap index will probably work best. This does require you to loop over the data ahead of time, but the index can be easily appended to and cached.

Once the index is generated, this should be O(1) for lookups, and will handle multiple entries per key.

    var layout = {
    "2":[[...], ...],
    "5":[[...], ...]
    };

    var index = {};

    function makeKey(data) {
        return data.join('_');
    }

    for(var l in layout) {
        var cur = layout[l];

        for(var i in cur) {
            var item = cur[i];
            var key = makeKey([item.p1, item.p2, ...]);

            index[key] = index[key] || [];
            index[key].push(item);
        }
    }

    function find(attid, optid) {
        var key = makeKey([attid, optid, 1, d_att_id, ...]);
        return index[key]; //this is an array of results
    }
Colin Godsey
  • 1,293
  • 9
  • 14
  • Since yours was the only one with some concrete code, I tested yours first. My original script took 2,304ms to run 100,000 times. Yours took 120ms to run (and the "loading" of the index took <= 1ms). I'm going to test jsumners suggestions next.... – random_user_name Apr 05 '12 at 22:33
  • Colin, this was a great answer, and without it, I wouldn't have been able to understand/work out @galambalazs answer. And, the reality is, his answer is the best performing: on the php side, if I set up the secondary key as a hash (like your suggestion), I'm able to access it quickly without any looping, plus I don't have to run the javascript loop to prepare an index. I've voted your answer up, because it helped so much, but because his is fastest, he will be receiving the bounty. Thank you! – random_user_name Apr 05 '12 at 23:07
  • Just remember that you will need to update your index whenever you get new data. – James Sumners Apr 05 '12 at 23:24
  • np, glad i could help. i overlooked the fact that the data was probably coming from a server (nutz!). appreciate the ups! I can up other posters now myself because of it :) – Colin Godsey Apr 05 '12 at 23:33
2

My first suggestion would be to stop using $.each if you want to squeeze out every bit of performance you can. jQuery.each does a bit more than a traditional loop. Take a look at this jsFiddle with your browser's debugger running (i.e. Safari's/Chrome's web developer tools) and step through the fiddle until execution fully returns from jQuery.

For reference, the fiddle's code is:

var myArr = [{"foo":"bar"},{"answer":42}];

debugger;

$.each(myArr, function(k, v) {
    console.log(k + ': ');
    console.dir(v);
});​

Now run through the second version:

var myArr = [{"foo":"bar"},{"answer":42}],
    i, j;

debugger;

for (i = 0, j = myArr.length; i < j; i += 1) {
    console.log('myArr[' + i + ']: ');
    console.dir(myArr[i]);
}​

Notice that there are far fewer operations being executed in the second version. So that's a tiny bit of performance gain.

Second, eliminate as many lookups outside of the local scope as you can. Even if you cache a reference to your global variables (boo!) then you can save a lot of time given that this loop will be executed possibly hundreds of times. So instead of:

function foo(a, b) {
    if (a === globalA && b === globalB) {
        // Do stuff
    }
}

You'd do:

function foo(a, b) {
    var gA = globalA,
        gB = globalB;
    if (a === gA && b === gB) {
        // Do stuff
    }
}

As for pairing down the conditional based on the object members, I'm not seeing much else that could be improved. The object properties you are checking are top level, and you're looking at local instances of each object (so the scope chain lookups are short).

Without knowing more about how this is actually supposed to work, those are the best recommendations I can make. However, I can make the guess that your idea of starting with simpler JSON data would be a big improvement. If you know what the layout, and its constraints, is, then requesting the specific details from the server would mean you don't have to check so many conditions. You could simply ask the server for the details that you actually need to implement and loop through those (to update the DOM or whatever).

James Sumners
  • 14,485
  • 10
  • 59
  • 77
  • Great answer. Can I follow up with a few questions? What do you think of the other options mentioned here - `jsonpath` and `grep`? Also, given that my variable `layout` is a globally scoped json variable, and your comments about global scope, is there a benefit to making THAT locally scoped also? If so, would you have any memory consumption concerns, or concerns over the variable getting scoped locally with each triggering of the function? – random_user_name Apr 02 '12 at 02:10
  • 1
    Giving jsonpath and $.grep a cursory look, you'd just be pushing the loop off to another bit of code again. However, jsonpath offers a PHP version that you could use server side; so that would be an advantage for it. Regarding the scope of `layout`, you cache a reference to a specific property of the object via `var attlayout = layout[attid];`, thereby reducing the cost of lookups in the loop (i.e. making them local). This is a shallow copy, not a deep copy. Therefore, very little new memory is consumed; just a new pointer is created. – James Sumners Apr 02 '12 at 04:28
  • I've tested these adjustments, and they definitely make an improvement, but not on the order I am seeing with some other options. My original code took 2304ms to run 100,000 iterations, with your suggestions it is down to 1472ms - a big improvement, but see my results utilizing Colin Godseys answer (120ms for 100,000 iterations). – random_user_name Apr 05 '12 at 22:36
1

I see that the you are searching by 5 fields: v.design_attribute_id,v.design_option_id,v.attribute_id,v.apply_to_options,v.option_id.

What you could do is add an extra field to the objects called "key" that is a composite of the values in those fields.

Here's an example

       {
            "key": "4_131_2_0_0" //i picked 0 to represent null, but you can pick any number                                    
            "id": "40",
            "attribute_id": "2",
            "option_id": null,
            "design_attribute_id": "4",
            "design_option_id": "131",
            "width": "10",
            "height": "10",
            "repeat": "0",
            "top": "0",
            "left": "0",
            "bottom": "0",
            "right": "0",
            "use_right": "0",
            "use_bottom": "0",
            "apply_to_options": "0"
        }

Note though that you must normalize the length of each value. Meaning that if one objects optionId is 1 and another object optionID is 566 you must represent the first optionId as 001 in the key string.

With this field you can then sort the array on the server side before returning it to the client.

Then you can use a binary search to find the values on the client.

Using the binary search implementation located here http://www.nczonline.net/blog/2009/09/01/computer-science-in-javascript-binary-search/

Your search function would look something like

function drawOption(attid, optid) {
    var attlayout = layout[attid];
    var needle = d_att_id + "_" + d_opt_id + "_" + attid + "_" + optid; //remember to normalize  length if you have to

    var idx = binarySearch(attlayout,needle);

    var item;
    if(idx !== -1){
        item = attlayout[idx];
        //do something 

    }
}

Another method you can try using this composite key idea is to have the server return the layout objects in one big object mapped by attid,v.design_attribute_id,v.design_option_id,v.attribute_id,v.apply_to_options,v.option_id

Then you can look up in O(1) time. It would look something like

 function drawOption(attid, optid) {

    var needle = attid + "_" + d_att_id + "_" + d_opt_id + "_" + attid + "_" + optid; //remember to normalize  length if you have to

    var item = layout[needle];
    if(typeof item !== "undefined"){
        //do something 

    }
}
KDV
  • 730
  • 1
  • 6
  • 12
0

I have experience to such issue before, my js array of objects consist of 8 thousands record and more. My experience is not about the performance, but the readability, maintainability, scalable of the codes.

hence I developed an JS Object Query Library 2 years ago: JSOQL http://code.google.com/p/jsoql/

It works like SQL to allow you query your js array of objects with syntax similar to SQL.

The example usage is something like this, I cant really remember, but you can download the example usage in the download tab.

new JSQOL().Select([field1, field2, field3 ...]).From({ ... }) .Where(fn) .Offset(int) .Limit(int) .Get();

Note: {...} is your array of objects, or an object it self.

Hope it helps, you can send me message if you need more information.

Jarod Law Ding Yong
  • 5,697
  • 2
  • 24
  • 16
0

It's not going to work everywhere but your problem sounds like something that can be done with webworkers

Another thing I would look at if you dont have webworkers is trying not to block the ui to long. If you can chunk it into bits of about 40ms and then setTimeout the next chunck for just a few ms later the user will have a more pleasant experience. This needs a little fiddling but users will start to notice stuff when something takes longer than somewhere between 50 and 100ms

Community
  • 1
  • 1
Sjuul Janssen
  • 1,772
  • 1
  • 14
  • 28
0

When trying to improve your code, it is always better to check which functions are taking time using firebug profiling. You can either profile by clicking on profile button in firebug's console panel and then run your code or using firebug's profiling commands in your code

From the code that you have given, only a few improvement points can be given.

  1. $.each is slow compared to native looping solutions. For the best looping solutions, check out this JsPref test
  2. It would be better to change the JSON to use arrays instead of object literals. It is said to be more faster to retrieve values.
sv_in
  • 13,929
  • 9
  • 34
  • 55
-1

Have you considered using the jQuery grep function?

jQuery grep

And jquery grep on json object array for an example.

Community
  • 1
  • 1
Nick Bork
  • 4,831
  • 1
  • 24
  • 25
  • thank you. This looks promising, however I'm wondering if there's a significant performance enhancement to doing this? I'm primarily concerned about performance/speed. – random_user_name Mar 29 '12 at 20:38
  • you're the only one who can put in speed tracking to exactly what you're doing. Try using a few different methods of filtering your array and see which one performs fastest for your situation. – Nick Bork Mar 29 '12 at 21:52
-1

Here is one technique that will probably yield better performance at the expense of using a bit more memory.

I'll leave my code examples simple just to illustrate the concept.

First, you'll want to pre-process your JSON data into some additional arrays that act as indexes. Here is an example of what the final arrays might look like after pre-processing:

var layouts_by_attribute = {
    // attribute_id => array(layouts)
    2: [40, 41, 44],
    5: [326]
};

var layouts_by_design_attribute_id = {
    // design_attribute_id => array(layouts)
    4: [40, 41, 44, 326]
};

Finding a layout by attribute is now very quick:

function findByAttribute(attribute_id) {
    return layouts = layouts_by_attribute[attribute_id];
}

function findByDesignAttribute(design_attribute_id) {
    return layouts = layouts_by_design_attribute[design_attribute_id];
}
Nick Clark
  • 4,439
  • 4
  • 23
  • 25