Page runs slow

[arenaninja]

Hello all. I took over a website recently from someone who left, but I'm not yet familiarized with PHP (though I know other languages) and I've hardly ever done web programming. The site is simple enough -- it's a signup for people who need to use our equipment.

 

The problem that I have is that one of the pages, when it queries a particular table, runs slow. On most tables the page loads in less than 2 secs. On the very slow one (the one that is used the most) it takes over 30 secs. I believe the code may be sloppy and this may be part of the reason why. Unfortunately I don't know enough about the mySQL/PHP combo just yet, and I need results stat on this issue, so your help is greatly appreciated. I'm attaching the code for your consideration.

 

The server runs mySQL v.3.23.58. I tried passing the userid to a variable instead of searching for it from the array at the time we're creating a query, but there was no improvement.

 

The way this code works is that it shows seven days and breaks each day by half-hour segments, displayed in a table. Each table segment has a 'Sign Up' link unless there's another reservation for that time in which case it displays the contact info for the person who reserved it. I believe the latter fact is the source of my problem, but I'm attaching more code since I believe the nested loops may also be causing issues.

 

	
		// Loop through the week we're displaying
		for( $j=0; $j<$dayPerPage; $j++ )
		{
			// Calculate the date here...
			$dateArray = getDate( mktime( 0, 0, 0, date("m"), date("d")+$j+$dateOffset, date("Y") ) );
			$date = $dateArray["year"]."-".$dateArray["mon"]."-".$dateArray["mday"];

			$oldSMin = $startMin - $row["time"];
			$oldSHr = $startHr;
			if( $oldSMin < 0 )
			{	
				$oldSMin += 60;
				$oldSHr--;
			}
			$sql = "SELECT * FROM ".$row["table_name"]." WHERE date='".$date."' AND timestart='".$oldSHr.":".$oldSMin.":00'";
			$res = mysql_query( $sql );
			$dates = @mysql_fetch_array( $res );
			//echo $sql;

			// We have a sign up here already
			if($dates)
			{
                                    // pass the userid... should be much faster
                                    $userid = $dates["userid"];
				$sql2 = "SELECT * FROM `".$GLOBALS["userTable"]."` WHERE id=".$userid;
				$r = mysql_query( $sql2 );
				$person = mysql_fetch_array( $r );
				$timestart = explode( ":", $dates["timestart"] );
				$timeend = explode( ":", $dates["timeend"] );
				$timeSMin = $timestart[1] + 60*$timestart[0];
				$timeEMin = $timeend[1] + 60*$timeend[0];
				echo "<td rowspan=\"". (($timeEMin-$timeSMin)/$row["time"]) . "\"><strong>Name: </strong>".
					$person["first_name"]." ".$person["last_name"]."<br><strong>Email:</strong>
					<a href=\"mailto:".$person["email"]."\">".$person["email"]."</a>";

				// Check to see if there is a 'comments' section to display
				if( $dates["comments"] )
				{
					// Yes, so give a link to show the comment
					$page = "showcomment.php?id=".$dates["id"]."&table=".$row["table_name"];
					echo "<br><a href=\"javascript:popUp('$page')\"><font color=\"red\">Comment</font></a>";
				}

				// If this is our OR we are admin sign up we want a cancel button
				if( $userid == $_SESSION["userid"] || $_SESSION["userlevel"] == 0 )
				{
					// Cancel button
					echo "<br><a href=\"cancelreg.php?equip=".$row["id"]."&user=".$person["id"]."&date=".$dates["id"]."\">Cancel Registration</a>";
				}
				// Otherwise, we want an email option button
				if( $userid != $_SESSION["userid"])
				echo "<br><a href=\"emailoption.php?equip=".$row["id"]."&date=".$dates["id"]."\"><font color=\"purple\">Email Option</font></a>";

				echo "</td>";
				// Remember... 
				$span[$j] = ($timeend[0]-$timestart[0])/$row["time"];
			}
			else
			{
				// No sign up, so put a link to sign up here.
				$span[$j]--;
				if( $span[$j] <= 0 )
					echo"<td><a href=\"signupnow.php?date=".$date."&time=".$oldSHr.":".$oldSMin."&id=".$equipid."\">Sign up</a></td>";
			}
		}

[PFMaBiSmAd]

In general, here is how you should handle this -

 

1) Determine the overall start and end dates/times of the data you want.

 

2) Execute ONE query that gets all the data you want and join this with the userTable to get the corresponding user information. This will give you one row per date/time/user. Pre-process this data and store it into a php array with the array index being the date/time value, so that you can easily find and retrieve data as you iterate over the overall date/time range (see step 3.)

 

3) Iterate over the dates/times (essentially your for() loop and the code that produces the $date, $oldSHr, and $oldSMin) and form a date/time key of the same format that you used for the array index values in step 2. Simply test if there is data (or not) in the php array for the date/time slot and produce the output that you want.

[arenaninja]

Thanks for the help. The more I look at this db the less I like it. Apparently there's one table per piece of equipment, even though they're all identical except in name. I've begun to do what you've suggested already, but as I said I'm still unfamiliar to some of this syntax and wet behind the ears, so I have a few questions.

 

On point two, I keep getting the error "Unknown table 'db.usertable' in on clause". Here's a sample of the code:

SELECT * FROM db.EQUIP X, db.usertable UT
LEFT JOIN db.EQUIP ON db.EQUIP.userid = db.usertable.id
WHERE db.EQUIP.date BETWEEN '2011-9-01' AND '2011-10-01'

For reference, I've tried switching the LEFT JOIN and WHERE clauses, but I still get an error.

 

As for the iteration, how do I refer to specific items in the array? For example, say I'm cycling through the reservations, and I want to (1) get the row number that I'm on and (2) get a value from a specific column. To make it more concrete, how do I reference the 'email' column for the fifth row? (Assume my variable is $result), I imagine $result["email"] would get me the email (?), but then where/how do I specify the record # (since I'm looping through it). Note that matching userid would not be good, since a user could make multiple reservations in any given date range.

[PFMaBiSmAd]

I guess that explains the $row["table_name"] variable? I was hoping that it was 'configuration' data and not a dynamically changing table name.  It holds the current table name and the code you first posted is inside of a query/loop itself.

 

The query statement, based on the two queries in the code you did post, would look like this (add other columns to the SELECT term as needed) -

 

$query = "SELECT d.date,d.timestart,u.first_name,u.last_name,u.email
FROM {$row["table_name"]} d JOIN {$GLOBALS["userTable"]} u ON d.userid=u.id
WHERE d.date BETWEEN '$start_date' AND '$end_date'";

 

Here is some sample code I came up with -

<?php
$step_size = 30; // minutes (size of each time slot)
$end_time = "17:00"; // HH:MM (make time slots up to, not including, this end time)

// build array of date/time slots for the range of dates
for( $j=0; $j<$dayPerPage; $j++ ){
$dateArray = getDate( mktime( 0, 0, 0, date("m"), date("d")+$j+$dateOffset, date("Y") ) );
$date = sprintf('%04d-%02d-%02d',$dateArray["year"],$dateArray["mon"],$dateArray["mday"]);
$hr = $startHr; $min = $startMin;
$time = sprintf('%02d:%02d:00',$hr,$min);
while($time < $end_time){
	$datetime[] = "$date|$time";
	$min += $step_size;
	if($min >= 60){
		$min = 0;
		$hr++;
	}
	$time = sprintf('%02d:%02d:00',$hr,$min);
}
}

// get start and end dates
list($start_date) = explode('|',$datetime[0]);
list($end_date) = explode('|',end($datetime));

// query for the matching data
$query = "SELECT d.date,d.timestart,u.first_name,u.last_name,u.email FROM {$row["table_name"]} d JOIN {$GLOBALS["userTable"]} u ON d.userid=u.id WHERE d.date BETWEEN '$start_date' AND '$end_date'";
if(!$result = mysql_query($query)){
trigger_error("Query failed: $query<br />Error: " . mysql_error());
} else {
if(!mysql_num_rows($result)){
	echo "Query matched zero rows!";
} else {
	// one or more rows
	while($row = mysql_fetch_assoc($result)){
		$data["{$row['date']}|{$row['timestart']}"] = $row;
	}
}
}

// iterate over the date/time slots and display any data
foreach($datetime as $dt){
if(isset($data[$dt])){
	// data exists for this date/time slot
	echo "Date: $dt, Data: ",'<pre>',print_r($data[$dt],true),'</pre>';
} else {
	// no data for this date/time slot
	echo "Date: $dt - no data<br />";
}
}

 

[arenaninja]

Thanks a lot for the help. With it, I'm almost done!

 

An issue I've come across is that in your code, the scope of the array $datetime[] is limited. I scratched my head for a while to no avail. How do I populate it with the necessary number of records so that the code runs? I don't want to take that while loop and apply it to the entire thing, it'd be too much looping (3 nested loops). I tried switching out the while() for a for loop but found that it took more time and didn't resolve the issue of scope.

[PFMaBiSmAd]

I have no idea what your last post means. The limited information you have supplied in the thread doesn't indicate what the overall code on the page is doing or what the overall result on the page is, so statements like 'necessary number of records', 'entire thing', or even mentioning scope doesn't mean anything to anyone reading this thread.

 

The code I posted is only a sample showing how the general steps I suggested could be accomplished, relative to the code you did post. You would need to adapt the general concepts to what you are doing or you would need to provide enough information so that someone here could help.

 

However, until you consolidate the tables for each piece of equipment in to one table, nothing you do will have much affect on performance. Putting queries inside of loops is a performance killer due to the 'cost' of forming each query statement, sending that query statement from php to the database server, and executing that query, repeated for every pass through the overall outer loop(s).

[arenaninja]

Apologies for the vagueness. I actually am adjusting the code that you posted to my needs, or at least so I think. Allow me to step through it.

 

You posted the following (relevant to an array you called datetime[])

/ build array of date/time slots for the range of dates
for( $j=0; $j<$dayPerPage; $j++ ){
while($time < $end_time){
	$datetime[] = "$date|$time";
	$min += $step_size;
	if($min >= 60){
		$min = 0;
		$hr++;
	}
	$time = sprintf('%02d:%02d:00',$hr,$min);
}
}

// iterate over the date/time slots and display any data
foreach($datetime as $dt){
if(isset($data[$dt])){
	// data exists for this date/time slot
	echo "Date: $dt, Data: ",'<pre>',print_r($data[$dt],true),'</pre>';
} else {
	// no data for this date/time slot
	echo "Date: $dt - no data<br />";
}
} 

I've adapted this piece with a ton of modifications, but the skeleton of the code remains the same. The datetime[] array was declared and populated inside of the first for loop. However the foreach loop tries to call on it to step through each $datettime element, but (I've checked this) it's blank at that point.

 

I understand the website has bad design, and it's not the only thing it has. It has outdated software (MySQL 3.23! I couldn't even find MySQL 4.0 to upgrade to) and re-designing the table design means overhauling the website. Which I will try, but I'd like to patch this first.

[fenway]

I don't see any queries in that code.