Speed up search results/queries

[Mal1]

Complete beginner here so just really looking for pointers on where to start.

 

Been doing a bit of bug fixing on one of our sites because the past 3 programmers we've hired have disappeared on us. The big issue I'm looking to solve is the load time of a search. Our website (Love-Rugs) is taking about 6 or 7 seconds to perform a blank search (basically a quick browse) whereas out other site (Little-Persia) takes about a second. It wouldn't be so bad if it was just the initial search but going from one page (only 10 products listed per page) to the next takes the same amount of time.

 

There seems to be an awful lot of queries (around 130-150) on the searches - however - when using some of the search options e.g. type and fabric to refine the search the queries actually stay high but the time to process the results is reduced significantly. I don't really understand why if the queries are still high the search time is much lower unless it's to do with the number of results returned. However this does not explain why Little-Persia (which has far more products on it) takes less time as there would obviously be more results.

 

I realize that without code this isn't easy to answer so I'm just looking to see if someone can point me in the right direction to look at just now.

[scootstah]

Sorry, but we can't help you if you don't post code. 130-150 queries for a simple search is pretty much insane. I'm 99% sure you could do whatever you're doing in a single query.

[Mal1]

Sorry, but we can't help you if you don't post code. 130-150 queries for a simple search is pretty much insane. I'm 99% sure you could do whatever you're doing in a single query.

 

Will try to post tomorrow... will need to find the actual pages that are sending the queries - there are probably several interlinked.

 

Thanks.

[PFMaBiSmAd]

Also post the table definition(s) and indicate the approximate number of rows in each table.

[samshel]

Agree with scootstah. Basic search can be done in a simple query with joins if necessary instead of 120-130 queries.

 

The difference in page load times if some filter is applied may be because of indexes defined on your tables. Correctly defined Indexes play a huge part on the query execution time.

 

 

[leitning]

I realize I'm reiterating what's already been said but I am super curious to see how in the heck your previous programmers managed to get 130-150 queries into your search.

[scootstah]

I realize I'm reiterating what's already been said but I am super curious to see how in the heck your previous programmers managed to get 130-150 queries into your search.

 

My guess is they are selecting something, then looping through it and selecting something else for each result.

[Psycho]

I realize I'm reiterating what's already been said but I am super curious to see how in the heck your previous programmers managed to get 130-150 queries into your search.

 

My guess is they are selecting something, then looping through it and selecting something else for each result.

 

Zactly! I've seen this before where the output needed to have several sets of subheadings by company, vendors and purchases. The developer did a query to get the list of companies and then did a nested loop running a new query each time to get the vendors for that company. Then did another nested loop to do queries to get the data for those vendors for that company. So the page needed to run the number of queries based on: # companies X # vendors X # purchases.

 

After a couple of months in use the amount of data was causing the report to time out. I fixed it by simply creating a single query that joined the records and used "flag" variables while processing the data to determine when to display new headers.

 

BAD Example (Many queries based upon number of records)

//Run query to get types
$query_types = "SELECT type_id, type_name FROM carpet_types";
$result_types = mysql_query($query_types);

while($row1 = mysql_fetch_assoc($result_types))
{
    //Display type name
    echo "<h1>Type: {$row1['type_name']}</h1>";
    //Run query to get carpets matching this type
    $query_carpets = "SELECT carpet_name, price FROM carpets WHERE type_id = {$row1['type_id']}";
    $result_carpets = mysql_query($query_carpets);

    while($row2 = mysql_fetch_assoc($result_carpets))
    {
        //Display carpet info
        echo "Name: {$row2['carpet_name']}, Price: {$row2['price']}<br>";
    }
}

 

GOOD Example (One Query)

//Run ONE query to get ALL carpet information - including the type info
//Ensure records are sorted appropriately
$query = "SELECT c.carpet_name, c.price, t.type_id, t.type_name
          FROM carpets AS c
          LEFT JOIN carpet_types AS t ON c.type_id = t.type_id
          ORDER BY t.type_id";
$result = mysql_query($query);

$current_type = false; //Flag to detect change in type
while($row = mysql_fetch_assoc($result))
{
    //Display type if different than last
    if($current_type != $row['type_id'])
    {
        echo "<h1>Type: {$row1['type_name']}</h1>";
    }
    //Display carpet info
    echo "Name: {$row2['carpet_name']}, Price: {$row2['price']}<br>";
}

[Mal1]

Zactly! I've seen this before where the output needed to have several sets of subheadings by company, vendors and purchases. The developer did a query to get the list of companies and then did a nested loop running a new query each time to get the vendors for that company. Then did another nested loop to do queries to get the data for those vendors for that company. So the page needed to run the number of queries based on: # companies X # vendors X # purchases.

 

I think something like this is happening. It retrieves the results but I think it does a lot of checks (to see if a promotion or discount is active and if it is a valid result etc.) maybe it's checking this each time rather than just once.

 

I'm not 100% sure if this is the code, there's pages of code that it could be so I've posted what I think to be the most likely segment and there's code within other files as well. Will maybe have a better idea when the boss comes in, he's from a software design background so might be able to read the code better (although not php/sql). If it's not someone please let me know so I can get it removed, it's not something I want up online:

 

<?php
//echo "rug_count: ".count($rugs_all)."<br />";
//echo "<br />---FULL---".$sql;
$page_count = $this->query($count_sql);
$pages = ceil(array_shift(array_shift($page_count)) / $search_results_per_page);
$search_results_per_page = 10;
if(isset($_GET['page'])) {
	$cur_page = intval($_GET['page'])-1;
	if($cur_page==0) {
		$sql .= " LIMIT 0, $search_results_per_page";
	} else {			
		$sql .= " LIMIT ".($search_results_per_page * $cur_page).", $search_results_per_page";
	}
} else {
	$sql .= " LIMIT 0, $search_results_per_page";
}
//echo $sql;
// page the result set			
$rugs_search = $this->query($sql);
$rugs = array();    
foreach ( $rugs_search as $rug ) {
	$tmp = new Rug($rug['id']);
	// apply promotions
	$tmp->applyPromotion();
	if(!isset($_GET['admin-search']) OR (isset($_GET['admin-search']) && $_GET['admin-search'] == 0)){
		// eliminate rugs without any stock       
		if ( $tmp->useQuantity() ) {
			$stock = 0;
			// look for active stock        
			foreach ( $tmp->data['stock']->data as $stock => $values ) {
				if ( $values['active'] == '1' && $values['stock'] > 0 ){
					$stock += $values['stock'];
				}
			}
			if ( $stock > 0 ){
				$rugs[] = $tmp->data;
			}
		}else{
			$rugs[] = $tmp->data;
		}
	}else{
		$rugs[] = $tmp->data;
	}

}
return array( 'this' => $rugs, 'all' => $rugs_all, 'sql' => $sql_all) ;
}

[Psycho]

I don't see anything in that code that would necessarily cause a significant performance problem.

 

It does not show how the entire query is created. It only shows where some additional clauses (e.g. LIMIT) are added. And, it seems like the query would only be run one (unless that file is included multiple times). There does seem to be some 'potentially' unnecessary post-processing of the code. For example, there is code to eliminate rugs without any stock from the results. That would be better handled in the WHERE clause of the query. But, that again does not "appear" to be something that would cause significant performance problems.

 

If you did not write this code and do not understand it and this is for a production site, I would really advise you bring someone in to fix this for you. We can only point out some suggestions on particular pieces of code we see. Without a full understanding of how the whole thing works there is the very real possibility that suggested changes could create other, more serious problems. Some may become readily apparent while others may not be easily exposed but are nonetheless significant.

 

This forum is for people to get help with code they have written. That's not because we don't want to help, it's because the person writing the code should have enough knowledge to post the relevant section of code they are having difficulty with and will understand how any changes would impact other areas of the application.

[Mal1]

As I said, there's a whole lot of code which all seems interlinked, it checks to see if items are on sale, or specific sizes within an ID are on sale, it checks for site-wide promotions etc etc. I'll get someone to have a look at it later and see if I can get it posted up but I don't want to and it wouldn't be fair to post up a thousand lines of code.

[samshel]

You cannot add / remove records after you retrieve them from the result set, that will screw up you paging.

Example:

Total 1000 records in table.

First page fetches 0 to 25 irrespective of stock.

After fetching from database, you hide 5 rugs which don't have stock. So you would be showing only 20 rugs on the page.

This will differ with each page and the count of the records on one page will not be consistent.

 

If you need to hide rugs with no stock, it is better to be done in the query.

 

however as Psycho pointed out, if this is production code, then you better get someone to look at the entire code then fix it.

 

[batwimp]

We also don't know if the applyPromotion() or useQuantity() methods in the Rug class are running queries of their own.

[Mal1]

Here's more of the code, this is the preceding part of that which was posted earlier. There is a lot of inter-twined functions but as far as I can see this is how the code fetches the results (the first part is how the code opens up, the second where I believe the search results are generated (all this is in the one file - I've added and closed the php tags just so it displays properly in code view:

 

Opening code (important but not the code that displays/fetches search results as far as I'm aware)

<?php
class Database {
var $pages, $error, $query_count;

function Database($db_user, $db_pass, $db_name, $db_host) {
	$this->query_count = 0;
	$this->error = 0;

	if (!mysql_connect($db_host, $db_user, $db_pass) || !mysql_select_db($db_name))
		$this->error = 1;
}

function query($sql_query) {
	//echo $sql_query.'<br />';
	$this->query_count++;
	$result = mysql_query($sql_query);

	$rows = array();
	if(@mysql_num_rows($result) > 0){
		while ($row = mysql_fetch_assoc($result))
			$rows[] = $row;
	}
	return $rows;
}

function escape($escape_string) {
	if (get_magic_quotes_gpc())
		return mysql_escape_string($escape_string);
	else
		return $escape_string;
}

function execute($sql_query) {
	mysql_query($sql_query) or die("EXECUTE: ".mysql_error()." SQL: ".$sql_query);
//	echo "TSDXX: $sql_query";
//	die();

	return mysql_insert_id();
}

function save($table, $fields) {
	$pairs = array();

	foreach ($fields as $field => $value)
      // do not quote NULLs
      if ( $value == "NULL" )
        $pairs[] = "`$field`=".$this->escape($value);
      else
    		$pairs[] = "`$field`='".$this->escape($value)."'";

	$sql = "UPDATE `$table` SET ".join(",", $pairs)." WHERE `id`='{$fields['id']}'";
    
//    echo $sql;
//    die();
//		print_r($fields);
//		die();
	$this->execute($sql);
}

function randomRugs($page=1) {
	global $search_results_per_page;
echo "Test";
	if (!isset($_SESSION['random_rugs']) || !is_array($_SESSION['random_rugs']))
		$_SESSION['random_rugs'] = $this->query("SELECT `id` FROM `rugs` WHERE `active`='1' AND `deleted`='0' ORDER BY RAND()");

	$this->pages = ceil(count($_SESSION['random_rugs'])/$search_results_per_page);

	$page = intval($page);
	if ($page > $this->pages)
		$page = $this->pages;
	if ($page < 1)
		$page = 1;

	$current_ids = array_slice($_SESSION['random_rugs'], ($page-1)*$search_results_per_page, $search_results_per_page);

	$ids = array();
	foreach ($current_ids as $current_id)
		$ids[] = "`id`='".$current_id['id']."'";

	//return $this->query("SELECT * FROM `rugs` WHERE ".join(" OR ", $ids));
	$rugs=array();
	foreach($ids as $id) {
		$rugs=array_merge($rugs, $this->query("SELECT * FROM `rugs` WHERE ".$id.";"));

	}
	return $rugs;
}
?>

 

 

 

 

 

Here is where it's getting the search results.

<?php
function findRugs($conditions, $page=1, $sort="") {
global $search_results_per_page, $current_user, $pages;

if ($current_user != NULL){
	$search_fields = array("id", "active", "type", "mod_trad", "shape", "rug_type", "fabric", "design", "make", "age", "origin", "name", "pattern", "colour");
}else{
	$search_fields = array("id", "active", "type", "mod_trad",  "shape", "rug_type", "fabric", "design", "make", "age", "origin", "name", "pattern", "colour");
}
$pairs = array();
$ignore_rs = $this->query("SELECT LOWER(name) as name FROM rug_search_ignore_words WHERE id <> 0 AND active = 1");
$ignore_list = array();
foreach ( $ignore_rs as $ignore ){
	$ignore_list[] = $ignore['name'];
}

require("inc_search.php");
// collect entire result set for refine area 
//$rugs_all = $this->query($sql);

//$rugs_search_all = $this->query($sql);
$sql_all = $sql;

$rugs_all = array();    
foreach ( $rugs_search_all as $rug ) {
	$tmp = new Rug($rug['id']);
	$rugs_all[] = $tmp->data; 
}

//echo "rug_count: ".count($rugs_all)."<br />";
//echo "<br />---FULL---".$sql;
$page_count = $this->query($count_sql);
$pages = ceil(array_shift(array_shift($page_count)) / $search_results_per_page);
$search_results_per_page = 10;
if(isset($_GET['page'])) {
	$cur_page = intval($_GET['page'])-1;
	if($cur_page==0) {
		$sql .= " LIMIT 0, $search_results_per_page";
	} else {			
		$sql .= " LIMIT ".($search_results_per_page * $cur_page).", $search_results_per_page";
	}
} else {
	$sql .= " LIMIT 0, $search_results_per_page";
}
//echo $sql;
// page the result set			
$rugs_search = $this->query($sql);
$rugs = array();    
foreach ( $rugs_search as $rug ) {
	$tmp = new Rug($rug['id']);
	// apply promotions
	$tmp->applyPromotion();
	if(!isset($_GET['admin-search']) OR (isset($_GET['admin-search']) && $_GET['admin-search'] == 0)){
		// eliminate rugs without any stock       
		if ( $tmp->useQuantity() ) {
			$stock = 0;
			// look for active stock        
			foreach ( $tmp->data['stock']->data as $stock => $values ) {
				if ( $values['active'] == '1' && $values['stock'] > 0 ){
					$stock += $values['stock'];
				}
			}
			if ( $stock > 0 ){
				$rugs[] = $tmp->data;
			}
		}else{
			$rugs[] = $tmp->data;
		}
	}else{
		$rugs[] = $tmp->data;
	}

}
return array( 'this' => $rugs, 'all' => $rugs_all, 'sql' => $sql_all) ;
}
?>

[Mal1]

We also don't know if the applyPromotion() or useQuantity() methods in the Rug class are running queries of their own.

 

Apply promotion checks to see if there is a promotion active and if it applies to an item. Use Quantity is a field we have to determine whether something needs to be in stock or can be ordered from a supplier - if UseQuantity is active then the number of stock items if finite and once they are gone the item is removed from search results. Ignore Quantity (opposite of UseQuantity) treats the item as if it has unlimited stock - the stock counter ticks down if things are in stock but once it hits zero the item remains active and customers can purchase as many as they like.

[batwimp]

But do those methods query the database as well?