Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why is this code so slow?

jsc.tools.road.correctType = function() {
    for(row = jsc.data.selection.startX - 1; row <= jsc.data.selection.endX + 1; row++) {
        for(col = jsc.data.selection.startY - 1; col <= jsc.data.selection.endY + 1; col++) {
            if(jsc.data.cells[row-1][col].type != "road" && jsc.data.cells[row+1][col].type != "road" && jsc.data.cells[row][col].type == "road") {
                jsc.ui.addClassToCell("horz", row, col);
            }
            else {
                jsc.ui.removeClassFromCell("horz", row, col);
            }
            if(jsc.data.cells[row][col-1].type != "road" && jsc.data.cells[row][col+1].type != "road" && jsc.data.cells[row][col].type == "road") {
                jsc.ui.addClassToCell("vert", row, col);
            }
            else {
                jsc.ui.removeClassFromCell("vert", row, col);
            }
        }
    }
};

// Elsewhere
jsc.ui.addClassToCell = function(class, x, y) {
    $("#" + x + "-" + y).addClass(class);
};
jsc.ui.removeClassFromCell = function(class, x, y) {
    $("#" + x + "-" + y).removeClass(class);
};

The code above runs very slowly. I can't figure out why. It's using jQuery 1.3.2. Any way to optimize it a bit?

EDIT: The code is part of a javascript game I am making as a personal project. It's basically a Simcity clone. This piece of code checks the neighbouring cells for each part of the road, and changes the class (and in turn the background image) to the correct one to make the road images line up right, e.g. horizontal, vertical and junction(no class) road images.

EDIT 2: A few more details to provide some context.

The jsc.data.cells is an 200 x 200 array. Each array element is an object with properties like so (default shown): {type: null, developed: false, powered: false, watered: false, hasTransport: false, wealth: 0, quality: 0} .

It's counterpart is in the UI, which is basically a giant table. (200 x 200 again). Each cell has a number of CSS classes added to it throughout the program to change the background image (e.g. .road to change it to road, .com.developed to make it a developed commercial zone). The table cells each have an id of the form #x-y which is what jsc.ui.addClassToCell, and jsc.ui.removeClassFromCell edit.

EDIT 3: Fixed the IDs starting with numbers. Trying out some of the optimizations now.

like image 528
Macha Avatar asked Apr 22 '26 18:04

Macha


2 Answers

A short estimate using O() notation:

for(row) ... O(N)
for(col) ... O(N)
$().addClass/removeClass ... O(N^2)

the $() is even called twice within the nested for.

so you end up with O(N^4)

You can optimize this by caching the calculated classes in the as property of jsc.data.cells[row][col], e.g.

jsc.data.cells[row][col].horz = 1; // don't set class "horz" if not present
jsc.data.cells[row][col].vert = 1;

and use the cached data when you create the cells inside the HTML table, rather than calling $() for each cell.

like image 93
devio Avatar answered Apr 24 '26 07:04

devio


Normally you can significantly optimize loops like these;

for( var x = 0; x < someMethod(); x++ ) {
  //... do stuff
}

By exchanging them out with something like this

var x = someMethod();
while( x-- ) {
  //...do stuff
}

Though it becomes slightly different semantically, it normally works quite well as long as you're not dependent upon order in your looping (order is opposite)

Even when you cannot change the order, you will also significantly improve your code by merely moving the someMethod call OUT of your actual loop, since in many JS implementations it will be called once for every iteration...

like image 38
Thomas Hansen Avatar answered Apr 24 '26 06:04

Thomas Hansen