help optimizing code please :)

hiberphoptik

Hello, I have a short php script that is heavy on the workload, I am looking for help on ways to speed things up without changing the functionallity of the script. Any ideas would be much appreciated. below is the code (with unimportant proprietary info changed to generic text)

as you will see, its not a large script but sometimes it takes about 60-90 seconds to complete and draw the results in the browser and we are only talking about 200 results. I THINK the slowdown is happening because I am selecing a status from the mysql server inside the initial looped results from the mssql server results

in english.. what this does is: grab records, loop through records and grab statuses from another DB/table, if the status allows it then show the record else dont show the record
---START CODE---

$cur=odbc_exec($odbccnx, "SELECT buncha stuff from mssql2000 server");

while(odbc_fetch_row($cur)) {
$number = odbc_result($cur, 1);
$phone = odbc_result($cur, 2);
$type = odbc_result($cur, 3);
$name = odbc_result($cur, 4);
$state = odbc_result($cur, 5);
$shipdate = substr(odbc_result($cur, 6),0,11);

		$statuscur=mysql_db_query(dbname, "select HIGH_PRIORITY status, statusdate, cbdate from tablename where number = '$number' order by attempts desc limit 1");
			$row = mysql_fetch_array($statuscur);
				$stat = $row[0];
				$statdate = $row[1];
				$cbdate = substr($row[2],0,10);

		$lockcur=mysql_db_query(dbname, "select locked from lock_table where number = '$number'");
			$rowlocked = mysql_fetch_array($lockcur);
				$islocked = $rowlocked[0];

	if ($stat=="Finish" || $stat=="Return" || $stat=="used") {
		$show="0";
	}

	if ($stat=="Pended" && $statdate==$today) {
		list($year,$month,$day) = split('-',$statdate);
		$year=$year;
		$month=$month;
		$day=$day;

		if ( alarmDaysDiff($year,$month,$day) == 0 )
			$alarmmessage .= "today";
		else if ( alarmDaysDiff($year,$month,$day) < 0 )
			$alarmmessage .= "past";
		else if ( alarmDaysDiff($year,$month,$day) >= 1 )
			$alarmmessage .= "future";

		if ($alarmmessage=="past") { $show="1"; }
		if ($alarmmessage=="today") { $show="0"; }
		if ($alarmmessage=="future") { $show="0"; }
	}

	if ($stat=="Hold") {
		$show="0";
	}

	if ($stat=="CB" && $cbdate!=$today) {
		list($year,$month,$day) = split('-',$cbdate);
		$year=$year;
		$month=$month;
		$day=$day;

		if ( alarmDaysDiff($year,$month,$day) == 0 )
				$alarmmessage .= "today";
		else if ( alarmDaysDiff($year,$month,$day) < 0 )
				$alarmmessage .= "past";
		else if ( alarmDaysDiff($year,$month,$day) >= 1 )
				$alarmmessage .= "future";

		if ($alarmmessage=="past") { $show="1"; }
		if ($alarmmessage=="today") { $show="1"; }
		if ($alarmmessage=="future") { $show="0"; }
	}

	if (($stat!="CB" && $selected=="cbvendor") || ($stat!="CB" && $selected=="cbnonvendor")) { 
		$show="0"; 
	}

	if(!$show) {
	$count++;		

echo"
<tr>
	<td></td>
	<td>$number</td>
	<td>$name</td>
	<td>$shipdate</td>
	<td>$type</td>
	<td>$state</td>
</tr>";
	}
	$show="";
	$alarmmessage="";
}

odbc_close($odbccnx);
echo "</table><br>Total: $count";

----END CODE---

justsomeone

One thing I notice is that you do a couple of selects while looping through the resultset of another select.

Would it be possible to replace these nested selects by using a joined query in the original select? Eg something like...


$cur=odbc_exec($odbccnx, "SELECT a.something, a.somethingelse, b.status, b.statusdate, c.locked from originaltable a, tablename b, lock_table c where originalwheres AND b.number = a.number AND c.number = a.number ");

This would mean that (assuming 200 rows in the orignal query), you would only be executing one query instead of 401.

That should make a big difference.

Other than that, you might want to check the indexing on your database tables. Does each table have a suitable index?

Hope this helps.

hiberphoptik

justsomeone wrote:
One thing I notice is that you do a couple of selects while looping through the resultset of another select.

Would it be possible to replace these nested selects by using a joined query in the original select? Eg something like...
$cur=odbc_exec($odbccnx, "SELECT a.something, a.somethingelse, b.status, b.statusdate, c.locked from originaltable a, tablename b, lock_table c where originalwheres AND b.number = a.number AND c.number = a.number ");
This would mean that (assuming 200 rows in the orignal query), you would only be executing one query instead of 401.

That should make a big difference.

Other than that, you might want to check the indexing on your database tables. Does each table have a suitable index?

Hope this helps.

thanks for the reply 🙂

Im kinda stuck doing the selects inside the loop of the first query because we are using two differant databases like so:

MSSQL2000 (loop)
MySQL (inside the loop)

currently the mysql table is indexed on the number so hopefully that is helping out a bit.

justsomeone

ack! Sorry, I didn't spot that 🙂

You could still combine the two SQL queries into one, that would bring you down from 401 queries to 201.

Another tweak would be exiting out of the loop iteration when you're sure you don't want to display. so instead of setting $show to zero, just exit from the iteration and proceed to the next iteration.

A more generic tip might be to echo the current time to a logfile at key points in the program. This will help analyse what's taking the time. Is it the firs sql server query that takes 60 seconds? or maybe one of the mysql queries takes 3 seconds?

hiberphoptik

justsomeone wrote:
ack! Sorry, I didn't spot that 🙂

You could still combine the two SQL queries into one, that would bring you down from 401 queries to 201.

Another tweak would be exiting out of the loop iteration when you're sure you don't want to display. so instead of setting $show to zero, just exit from the iteration and proceed to the next iteration.

A more generic tip might be to echo the current time to a logfile at key points in the program. This will help analyse what's taking the time. Is it the firs sql server query that takes 60 seconds? or maybe one of the mysql queries takes 3 seconds?

ooh interesting, i like the sound of your suggestion of exiting the loop if the record/status in question should not be drawn. however im not sure how to tell a loop to stop and continue where it left off

hiberphoptik

I have combined the two mysql queries in the loop as one left join query, i dont see much of a speed improvement but im still tinkering with ways to cut overhead

herve

I think too that the best part to optimize is the SQL queries, but anyway, some ideas :

$cur=odbc_exec($odbccnx, "SELECT buncha stuff from mssql2000 server");

while(odbc_fetch_row($cur)) 
{
	$number = odbc_result($cur, 1);
	$phone = odbc_result($cur, 2);
	$type = odbc_result($cur, 3);
	$name = odbc_result($cur, 4);
	$state = odbc_result($cur, 5);
	$shipdate = substr(odbc_result($cur, 6),0,11);

$statuscur = mysql_db_query(dbname, "select HIGH_PRIORITY status, statusdate, cbdate from tablename where number = '$number' order by attempts desc limit 1");

$row = mysql_fetch_array($statuscur);
$stat = $row[0];
$statdate = $row[1];
$cbdate = substr($row[2],0,10);

$lockcur = mysql_db_query(dbname, "select locked from lock_table where number = '$number'");

$rowlocked = mysql_fetch_array($lockcur);
$islocked = $rowlocked[0];

switch ($stat)
{
	case 'Finish':
	case 'Return':
	case 'used':
	case 'Hold':
			$show="0";
			break;
	case 'Pended':
			if ($statdate == $today)
			{
				list($year,$month,$day) = split('-',$statdate);

				$show="0";
				$n = alarmDaysDiff($year,$month,$day);

				if (  $n == 0 )
				{	$alarmmessage .= "today";
				}
				else if ( $n < 0 )
				{	$alarmmessage .= "past";
					$show="1";
				}
				else if ( $n >= 1 )
				{	$alarmmessage .= "future";
				}
			}
			break;
	case "CB":
			if ($cbdate != $today)
			{
				list($year,$month,$day) = split('-',$cbdate);

				$show="1";
				$n = alarmDaysDiff($year,$month,$day);

				if ( $n == 0 )
				{	$alarmmessage .= "today";
				}
				else if ( $n < 0 )
				{	$alarmmessage .= "past";
				}
				else if ( $n >= 1 )
				{	$alarmmessage .= "future";
					$show="0";
				}
			}				
			break;

}

if ($stat != "CB")
{
	if ( ($selected == "cbvendor") || ($selected == "cbnonvendor") )
	{
		$show="0";
	}
}

if (!$show) 
{
	$count++;

	echo"
	<tr>
	<td></td>
	<td>$number</td>
	<td>$name</td>
	<td>$shipdate</td>
	<td>$type</td>
	<td>$state</td>
	</tr>";
}

$show="";
$alarmmessage="";
}

odbc_close($odbccnx);
echo "</table><br>Total: $count";

Mainly replacing many "if()" by a "switch()" reduce the number of tests to do.

justsomeone

hiberphoptik wrote:
ooh interesting, i like the sound of your suggestion of exiting the loop if the record/status in question should not be drawn. however im not sure how to tell a loop to stop and continue where it left off

Sorry, I wasn't being clear.

You would have to restructure your if's.

so instead of...

 if ($stat=="Finish" || $stat=="Return" || $stat=="used") {
$show="0";
}
//continue doing stuff

try

if ($stat != "Finish" && $stat !="Return" && $stat !="used") {
//continue doing stuff
}

This means that if $stat is any of "Finish", "Return" or "used", it will skip past any further actions and just skip on to the end of the loop.

Can you see what I mean?

I also think you should look at displaying or logging the time before and after each of the database calls. This might help identify if one call in particular is slowing down the system.