# Optimizing a function with nested loops and if condition

private  void find_tile(float[] x, float[] z, int nRigidBodies)  {
int serial[]=new int[nRigidBodies];
int numTiles=4;
for(int i=0;i<nRigidBodies;i++)
{
// Check for the loaction of each tile to find which one should be switched on...
//currently only supports 4
for(int j=0;j<numTiles;j++)
{
if ((x[i] > tileCoords[j][0]) && (x[i] < tileCoords[j][1]) && (z[i] > tileCoords[j][2]) && (z[i] < tileCoords[j][3]))
{
serial[i]=j+1;
break;
}
}
}


We have a optitrack system that tracks objects in a room and the number of objects is represented by nRigidBodies and their x-y cordinate by the x[] and z[] array passed here.

Also we have a fixed number of Tiles which are fixtures on the ceiling represented by numTiles and the bounds of these tiles are in tileCoords.

If an object is directly under one of these fixtures, we switch the fixture on.So I create an array serial[] which is of the same size as nRigidBodies.For every object we check if is is within the bounds of that fixture and if it we assign the fixture number to the serial array.Later we just take out all the fixture number from the array( coz these should only be lit up ) and send them a message to light up and the ones not in here are switched off.

The condition where the object is not within any of the fixtures is fine since it is checked later in the code and taken care of.

Sample Input
float[] x= {1.5, -0.5, 3.5}
float[] z= {2.5, -1.5, 3.2}
nRigidBodies= 3
numTiles=4
tileCoords={{0.5,1.5,1.5,2.5},{-0.5,0.5,0.5,1.5},{0.5,1.5,0.5,1.5},{-0.5,0.5,-0.5,0.5}}

Expected Output
serial[]={1,2,0}


I have a simple function that checks for if each object is within the bounds of the cordinates or not.If it is, we assign the number to a variable.

Currently the inner loop generally only has only 4 iterations so it doesn't matter much.However in the future it might run for upto 50 iterations with the outer loop running for 5 iterations max.The code is a time sensitive piece which controls lights in real time.Too much time in computing this function would result in lights appearing to be lagged.

It would be of great help if someone can guide me on how to optimize the function above for faster computation.

I was thinking of using threads to run all of the outer loops at once.If there is something better which can be achieved with single threaded model,I would love to stick to it as that might be making it really complicated.

• You may want to look into algorithmic improvement, though I am not familiar with any myself. Before people will say that Java is bad for real time physics, remember that you are using primitives and your code is about as fast as C++ code, the only harmful part of Java is that for every object you create, it may get garbage collected and some point and that may hurt performance. – skiwi Jul 29 '14 at 11:51
• What exactly is the point of the serial[] array? I do not see it being used or returned anywhere. – skiwi Jul 29 '14 at 11:52
• Could you please let us know the sample data contained in tileCoords? is it an instance variable initialized somewhere? Also please provide sample input data and output that you are expecting with that? – Shirishkumar Bari Jul 29 '14 at 12:54
• What else does this function do? How does it end? – 200_success Jul 29 '14 at 13:25
• @skiwi I forgot to include the rest of the function which basically is a function call to another function where we send messages to fixtures to change their state. – SteveIrwin Jul 29 '14 at 17:56

Disclaimer: I have not tested any of these optimizations. Some of these optimizations could potentially even harm the speed - or they could already be covered by the JVM. Do your own benchmarking.

Helping the Compiler/JVM
If you don't need to override this method, declare it final. The compiler might inline the method if it thinks it helps. Additionally, declare constants as final - such as numTiles.

Preventing repeated lookups
You need to access x[i], tilecoords[j], z[i] and such multiple times. Consider storing those in temporary variables. Don't forget to test to see if it is actually faster to do so. An example (with really bad variable names - you might want to pick better names than xi and zi

for(int i=0;i<nRigidBodies;i++)
{
// Check for the loaction of each tile to find which one should be switched on...
//currently only supports 4
float xi = x[i];
float zi = z[i];
for(int j=0;j<numTiles;j++)
{
if ((xi > tileCoords[j][0]) && (xi < tileCoords[j][1]) && (zi > tileCoords[j][2]) && (zi < tileCoords[j][3]))
{
serial[i]=j+1;
break;
}
}
}


Fixing bugs
int serial[]=new int[nRigidBodies]; creates a new int array with a size of nRigidBodies. int's default value is 0. Thus, the entire array will be filled with 0. Given that your innerloop starts at index 0 and sets the "found" index into the array, it's possible it's setting 0 over the default value. That's not so important. What is important is the reverse implication: If the serial array contains a 0 at a given index, you do not know whether this is because no tile-index was found, or because the tile-index that was found was the first one.

• x, z, and nRigidBodies should be final too. – rolfl Jul 29 '14 at 13:30
• @rolfl From what I found, that doesn't aid the performance. ... although it helps from a readability/maintenance perspective. – Pimgd Jul 29 '14 at 13:39
• @Pimgd thanks a lot ..I will make the necessary changes and thanks for pointing out the bug.It is a rather serious one.Also I don't really get what you mean by storing them in temporary variables.Thanks. – SteveIrwin Jul 29 '14 at 17:59
• Added explanation and fixed a typo – Pimgd Jul 29 '14 at 22:10