Optimising/Cleaning code

[Philwn]

Hi,

This isn't so much a problem as my code/page works however my host shut down scripting on the site as too many connections to the database were left open apparently with this page. Also ive been looking into a way of cleaning up the code and making it less bloated but the only thing I have found is mysqli for running multiple queries in one go but I am unsure if this would help.

 

 

<?php session_start();
include("./includes/db_con.inc.php");
$image = $_GET["id"];
$id = str_replace("-", "/", $_GET["id"]);
$cat = $_GET["cat"];
$sub_cat = $_GET["sub"];
$page_title = $cat . " - " . $sub_cat . " - " . $id;
$sql_meta = mysql_query("SELECT * FROM english WHERE PRODID='$id' AND display='1'");
$row_meta = mysql_fetch_assoc($sql_meta);
$meta_desc = $row_meta['META_DESC'];
$meta_key = $row_meta['META_KEY'];
include("./includes/head.php"); 
include("./includes/header.php"); ?>

<div id="main-page-header">
<?php
$header = "img/product-pages/product-specific-headers/" . $image . ".jpg";
if (file_exists($header)) {
?>
  <img src="<?php echo $img_loc; ?>/product-pages/product-specific-headers/<?php echo $image; ?>.jpg" width="940" height="310" />
<?php } else {
  $desc_sql = mysql_query("SELECT * FROM sub_cat WHERE name='$sub_cat'");
  $row_desc = mysql_fetch_assoc($desc_sql);
$description = $row_desc['description'];
echo "<img src='$img_loc/product-pages/sub-cat-head/".str_replace(" ", "-", $row_desc['name'])."-sub-cat-head.jpg' width='940' height='310' />";
} ?>

</div>

  <div id="page-content">
  
    <div id="main-page-text">
    
    <h1><?php 

$result = mysql_query("SELECT * FROM $table WHERE PRODID='$id' AND display='1'");

while($row = mysql_fetch_array($result))
  {
  $prod_title = $row['PROD_TITLE'];
  echo $prod_title; ?>
  </h1>
  <h2 class="crumbs"><a href="<?php echo "$url/Products/$lang/"; ?>">Products</a> > <a href="<?php echo "$url/Category/$lang/".urlencode($cat)."/"; ?>"><?php echo $cat; ?></a> > <a href="<?php echo "$url/Range/$lang/".urlencode($cat)."/".urlencode($sub_cat)."/"; ?>"><?php echo $sub_cat; ?></a> > <?php echo $prod_title; ?> </h2>
<p><?php echo $row['DESCRIPTION'];
} ?></p>

  <script src="http://connect.facebook.net/en_US/all.js#xfbml=1"></script><fb:like layout="button_count" show_faces="false" width="170" font="verdana"></fb:like>
  <br />

<?php
$i = 1;
while ($i <=  {
	$filename = "img/product-pages/hovers-800px/" . $image . "-" . $i . ".jpg";
	if (file_exists($filename)) {
?>
<a class="fuglybox" rel="gxr" href="<?php echo "../../../../../" . $filename; ?>">
      <img src="../../../../../img/product-pages/product-detail-90px/<?php echo $image . "-" . $i . ".jpg"; ?>" alt="" width="90" height="90" /></a>
<?php
	} else {
	break;
	}
$i++;
}
?>
  <table>
    <th>Product Code</th> 
  <th>Description</th>

<?php 
$table_result = mysql_query("SELECT * FROM PRODID WHERE PRODID='$id'");
$table_entry = mysql_fetch_assoc($table_result);
echo "<tr><td class='prod-id'>" . $table_entry['PROD_CODE'] . "</td>\n"; echo "<td class='prod-desc'>" . $table_entry['TA_DESC'] . "</td></tr>\n";
?>
</table>
    
     <div id="FAQ-wrap"><h2>Frequently Asked Questions</h2>
  <?php
  $QA_table = $lang . "_qanda";
  if ($id=="") {
      $list_QA = mysql_query("SELECT * FROM $QA_table WHERE SUB_CATEGORY='$sub_cat' AND DISPLAY='1'");
  }
  else {
  $list_QA = mysql_query("SELECT * FROM $QA_table WHERE PRODID='$id' AND DISPLAY='1'");
	}
  while($row_QA = mysql_fetch_array($list_QA))
{ 
  echo "<div id='FAQ-QA'>";
  echo "<h2 class='FAQ-question'>Q: ".$row_QA['QUESTION']."</h2>";
     echo "<p class='FAQ-question'>A: ".$row_QA['ANSWER']."</p></div>";
}
  ?> 
    <div id="FAQ-question">
        <h2 class="white">Got a question about the <span class="got-question-product"><?php echo $sub_cat; ?></span> 
        </h2>  
      </div>
      </div>
    </div>
    <div id="totem-menu-container"><?php include("./includes/search.php") ?>
    <?php include("./includes/totem.php") ?>
    </div>
  </div>
  </div>  
<?php include("./footer.php") ?>

 

Please note in the db_con.inc.php is the connection to the database stored in variable $con and included in the footer.php is mysql_close($con)  which is also why I dont understand how connections are being left open, any help greatly appreciated.

 

[PFMaBiSmAd]

Since php automatically closes connections (provided you are not using persistent connections or you have a version of php with a bug in the exit/cleanup code), the only way that code could result in "too many connections" would be if the page is making more than one connection or you simply have a popular site that has enough concurrent visitors (or a hacker intentionally trying to trigger errors) to that page that you are exceeding the maximum number of connections that your web host permits.

 

Do any of the other files that are being included make a database connection, or perhaps have a dynamically produced image that is making a database connection? What exactly is the mysql_connect() code?

 

What's the exact wording of the error messages?

[premiso]

I am not really into going through all that and doing it for you. But I will give some pointers.

 

I see multiple duplicate queries, if you are fetching the data twice, just fetch it once into an array and re-use that array.

 

For this SQL:

$desc_sql = mysql_query("SELECT * FROM sub_cat WHERE name='$sub_cat'");

 

Since you are pulling a list of subcats, you might be able to cache that and only regenerate if something changes. That would reduce your calls. You can even cache the products if needed (but that kind of defeats the point of having a SQL database). So look at your queries, if you see the same query, why run it twice? If the data is not going to be changing really often, cache it.

[Philwn]

Thanks for the replies.

 

PFMaBiSmAd  - Here is the code I use to connect to the database. The website has between 26000-37000 unique visitors a month.

$con = mysql_connect($DBHost,$DBUser,$DBPassword);
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }

mysql_select_db($DBName, $con);

 

none of the other files make a database connection but in totem.php there is one mysql_query.

 

premiso - I do not expect anyone to redo my work for me, pointers is all i ask. I cannot see anywhere I have used the same query twice? I would store the initial query in variables if I had unless I have missed one??

 

I have wondered about looking into cache because of the reasonably large amount of traffic we recieve but only briefly touched on it. i will google more on cache as at first most of the reults were third party scripts.

 

 

 

[premiso]

I must be going cenial, I thought there was a duplicate, but upon closer inspection it is not. However these two queries:

 

$result = mysql_query("SELECT * FROM $table WHERE PRODID='$id' AND display='1'");

$table_result = mysql_query("SELECT * FROM PRODID WHERE PRODID='$id'");

 

If they are both using the same $id, can probably be combined into 1 query using either joins or a union statement, depending on if they are actually two different tables or you want one to do the display = 1 and the other to not.

 

As far as caching goes, well basically all you would do is store the built page in a "cache" folder. Upon page load, you can do a couple of things, either store the cache page id and last cache location / last cached time in the database and check that upon page load. If there is a cache at the location, then you just serve that page up using include. If it is there and your cache threshold has been reached you regenerate the page (using output buffering) and then save the output buffer into a file before echoing it, and that is your new cache and update the database with the last cache. That way, once the page has been generated you only have to do 1 query to check it. And if you want it to regenerate every 24 hours you just add in a check to verify the last time it was generated and update it.

 

That would be a "simple" caching system which should do well for you. Hopefully it makes sense either or Let me know if it needs to be clarified a bit.

 

If it does change, you can easily pull out

[Philwn]

They are different tables. The one that defines the table as being $table could be a possible of 4 tables(4 different languages) which is only required for that query and the other query checks a table called PRODID which is also the name of a field in the other tables which is where the confusion may of come from.(I lack imagination in my field and variable names)

 

That cache system sounds perfect I cant see any reason to need it to recache more than every 24 hours unless a change had been made for that recpord in the database. I will start looking into cache scripting tommorow and see how I go before asking anymore.

 

So if the host tells me theres too many connections left open its from somewhere else and not this page then?

 

Thanks for all the help so far.

[premiso]

Most likely, but as mabis said, if it is because too many open connections then they either have a limit of queries that can be ran, their mysql / php setup is kind of screwed up, or you are exceeding your limits with your hosts. You might look at a low end VPS provider, depending on your needs. I have a few from different sources. For that size of a site 512MB VPS would be good, or a 128MB if you can have the MySQL offloaded.

 

But all that aside, I really do not see why this would be causing you havoc. The caching should fix that issue, you might also want to look into CloudFlare, although I heard some nasty rumors that Dubai is blocking Cloudflare for political reasons, might be something for you if you are willing to test the waters. I have had mixed results with it, so yea. I do not necessarily recommend it persay, but you might be intrigued by it. 

[PFMaBiSmAd]

As already stated, php automatically closes connections (when the script on any page ends.) Barring a bug in your php version that prevents that or you are using persistent connections somewhere (and your web server/php combination even supports persistent connections), connections will not normally be left open if there are no instances of your page being requested.

 

And as already asked, what is the exact wording of the error messages or what exactly is the information that your web host provided? It should be explicate about who,what,when, where, and how many connections, otherwise it is just a rumor and it is extremely hard to fix rumors in programming.

[Philwn]

would this work to cache my website?