Optimising this code

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

User avatar
spamyboy
Forum Contributor
Posts: 266
Joined: Sun Nov 06, 2005 11:29 am
Location: Lithuania, vilnius

Optimising this code

Post by spamyboy »

Ok, could some one give me example on how to optimise this code to work with huge database (over 645mbs).
I never havent worked with such databases so I never had such problems, basicly could you show what to do in example this code to optimise queries.

Code: Select all

<?php
include_once"generate.php";
function main()
{
global $youtube;
	$query_cfg="select * from config where cfg_id=1";
	$rs_cfg=mysql_query($query_cfg);
	$row_cfg=mysql_fetch_array($rs_cfg);
	$max_new_db=$row_cfg["max_new"];
	$max_top_rated_db=$row_cfg["max_top_rated"];
	$lyric_str="";
	if(isset($_REQUEST["lyric_id"]) && ($_REQUEST["lyric_id"] != 0 ))
	{
		$lyric_id=$_REQUEST["lyric_id"];
		$lyric_str=" and lyric_id=$lyric_id ";
	}
?>
<?php if(!isset($_REQUEST["lyric_id"]) and !isset($_REQUEST["do"])) { ?>
<!-- News -->
<table width="100%" cellpadding="0px" cellspacing="0px" border="0px">
<tr height="49px">
<td colspan="2" class="poemhead"><strong>News</strong></td>
</tr>
</table>
<?php
$result = mysql_query("SELECT * FROM news ORDER BY news_date DESC limit 4");
while ($row=mysql_fetch_array($result)) {
?>
<div class="news_frame" id="news_id#<? echo $row['news_id'];?>">
<div class="news_title"><? echo $row['news_title'];?> <span class="news_date">posted on: <? echo $row['news_title'];?></span></div>
<div class="news_content"><? echo $row['news_text'];?></div>
</div>
<?php } ?>
<!-- News END -->
<? } ?>
<!-- Lyric -->
<?php
if(count($_POST)<>0)
	{
		$lyric=$_REQUEST["lyric"];
		$score=$_REQUEST["score"];
		$lyric_id=$_REQUEST["lyric_id"];
		$query_update="update lyrics set score=score+$score, no_of_ratings=no_of_ratings+1 where lyric_id=$lyric_id";
		if(mysql_query($query_update))
		{
			?>
      <div align="center">Thanks for rating this lyric</div>
      <?php
		}
		else {
			echo "Unable to complete operation, Please try again";
		unset($_REQUEST["lyric"]);
		}
	$lyric_str="";
	if(isset($_REQUEST["lyric_id"]) && ($_REQUEST["lyric_id"] != 0 ))
	{
		$lyric=1;
		$lyric_id=$_REQUEST["lyric_id"];
		$lyric_str=" and lyric_id=$lyric_id ";
	}
}
if(isset($_REQUEST["lyric_id"]) && ($_REQUEST["lyric_id"] != 0 )) {
if (isset($_SESSION["view$lyric_id"]) && $_SESSION["view$lyric_id"]==TRUE){
mysql_query("UPDATE lyrics SET views=(views + 1) WHERE lyric_id=$lyric_id");
$_SESSION["view$lyric_id"]=TRUE;
}
?>
<table width="100%" cellpadding="0px" cellspacing="0px" border="0px">
<?php
$query="select *,score/no_of_ratings as score from lyrics where 1  $lyric_str and approved='yes'";
$rs=mysql_query($query);
if($row=mysql_fetch_array($rs))
{
?>
<tr height="49px">
<td colspan="2" class="poemhead"><strong><?php echo $row["lyric_artist"]; ?></strong> - <?php echo $row["lyric_title"]; ?></td>
</tr>
<tr>
<td colspan="2" class="poem">
<div><?php
if(strlen($row["youtube"])>6) {
$youtube=$row["youtube"];
} else {
$youtubesearchquery = str_replace(" ", "+", $row["lyric_artist"]." ".$row["lyric_title"]);
$a = file_get_contents("http://youtube.com/results?search_query=$youtubesearchquery&search=");
if(preg_match("/<a class=\"newvtitlelink\" href=\"(.*?)\" rel=\"nofollow\"/s", $a,$matches)) {
if($matches>0){
$b = file_get_contents("http://youtube.com".$matches[1]);
}
preg_match("/<input name=\"embed_code\" type=\"text\" value=\'(.*?)\' class=\"vidURLField\"/s", $b,$matchesb);
$youtube=$matchesb[1];
mysql_query("update lyrics set youtube='$matchesb[1]' where lyric_id ='$lyric_id'");
} else {
echo "No video avaible";
}
}
?></div>

<?php
$trunc_lyric=str_replace("\n","<br>",$row["lyric_text"]);
echo $trunc_lyric;
?>
<form name="form1" method="post" action="index.php">
                                      <table width="100%" border="0">
                                        <tr> 
                                          <td class="maintablestyle"><strong>Rate This lyric</strong> 
                                            <strong>( 
                                            <?php 
									  	$no_of_stars=$row["score"]/2;
										for($i=1;$i<=$no_of_stars;$i++)
											echo '<img src="images/star1.gif">';
										for(;$i<=5;$i++)
											echo '<img src="images/star2.gif">';
										?>
                                            )</strong> <input name="lyric_id" type="hidden" id="lyric_id" value="<?php echo $lyric_id; ?>"> 
                                            <input name="lyric" type="hidden" id="lyric" value="<?php echo $lyric; ?>"></td>
                                          <td><select name="score" id="score" class="form_select">
                                              <?php 	for($i=1;$i<=10;$i++)
													echo "<option value=\"$i\">$i</option>";?>
                                            </select> </td>
                                          <td><input type="submit" name="Submit2" value="Rate" class="form_button" /></td>
                                        </tr>
                                      </table>
                                    </form>
<?php
function formatDate($val)
  {
      list($date, $time) = explode(" ", $val);
      list($year, $month, $day) = explode("-", $date);
      list($hour, $minute, $second) = explode (":", $time);
      return date("l, m.j.y @ H:ia", mktime($hour, $minute, $second, $month, $day, $year));
}
$commentquery = mysql_query("SELECT * FROM comments WHERE comment_lyricid='$lyric_id' ORDER BY comment_date") or die(mysql_error());
        $commentNum = mysql_num_rows($commentquery);
		if($commentNum>=1){
        echo "<div id=\"currentcomments\" class=\"submitcomment\"><h3 class=\"formtitle\">Current Comments</h3>\n";
		}
        while($commentrow=mysql_fetch_row($commentquery)){
        $commentbb = $commentrow[4];
                $commentDate = formatDate($commentrow[6]);
                echo "<div class=\"commentbody\" id=\"$commentrow[0]\">\n
                <p>$commentbb</p>\n
                <p class=\"postedby\">Posted by ";
                if($commentrow[3]){
                echo "<a href=\"$commentrow[3]\">$commentrow[2]</a> ";
                } else {
                echo "$commentrow[2] ";
                }
                echo "on $commentDate | #$commentrow[0]</p>\n
                \n</div>";
        }
        echo "</div>";
?>
<script language="javascript">
function form_Validator(form)
{
  if (form.name.value == "")
  {
    alert("Please enter your name.");
    form.name.focus();
        return (false);
     }
  if (form.message.value == "")
  {
    alert("Please enter your message.");
    form.message.focus();
    return (false);
  }
  return (true);
  }
</script>
<div id="submitcomment" class="submitcomment">
<form name="submitcomment" method="post" action="submitcomment.php" onSubmit=" return form_Validator(this)">
<table width="100%">
<tr>
<td><input tabindex="1" id="name" name="name" class="topinput" />
  <span class="form_text">Name</span></td>
</tr>
<tr>
<td><input tabindex="2" id="email" name="email" class="topinput" />
  <span class="form_text">Email</span></td>
</tr>
<tr>
<td><input tabindex="3" id="url" name="url" class="topinput" />
  <span class="form_text">URL</span></td>
</tr>
<tr valign="top">
<td><textarea tabindex="4" id="message" name="message" rows="10" cols="50" class="form"></textarea>
  <span class="form_text">Comment</span></td>
</tr>
<tr>
<td><input type="submit" name="post" value="Submit Comment" class="form_button" /><br />
</td>
</tr>
</table>
<input type="hidden" name="lyricurl" value="index.php?lyric_id=<?php echo $lyric_id;?>" />
<input type="hidden" name="lyricid" value="<?php echo $lyric_id;?>" />
</form>
</div>
</td>
</tr>
</table>
<?php } ?>
<?php } ?>
<!-- Lyric END -->

<?
if(isset($_REQUEST["do"]) && ($_REQUEST["do"] =="list" )) {
$page_name="index.php?do=list";
if(!isset($start)) {
$start = 0;
}

$eu = ($start - 0); 
$limit = 10;
$this1 = $eu + $limit; 
$back = $eu - $limit; 
$next = $eu + $limit; 

$query2=" SELECT * FROM lyrics  ";
$result2=mysql_query($query2);
$nume=mysql_num_rows($result2);

echo "<table class=\"listing\"><tr>";
echo "<td><a href='$page_name&column_name=lyric_id'>Id</a></td>";
echo "<td><a href='$page_name&column_name=lyric_artist'>Artist</a></td>";
echo "<td><a href='$page_name&column_name=lyric_title'>Title</a></td>";
echo "<td><a href='$page_name&column_name=score'>Score</a></td>";
echo "<td><a href='$page_name&column_name=views'>Views</a></td></tr>";

$query=" SELECT * FROM lyrics ";
if(isset($_REQUEST["column_name"]) and strlen($_REQUEST["column_name"])>0){
$query = $query . " order by $_REQUEST[column_name]";
}
$query = $query. " limit $eu, $limit ";
$result=mysql_query($query);

while($row = mysql_fetch_array($result))
{
echo "<tr >";
echo "<td><a href=\"index.php?lyric_id=$row[lyric_id]\" class=\"listing_results\">$row[lyric_id]</a></td>"; 
echo "<td><a href=\"index.php?lyric_id=$row[lyric_id]\" class=\"listing_results\">$row[lyric_artist]</a></td>"; 
echo "<td><a href=\"index.php?lyric_id=$row[lyric_id]\" class=\"listing_results\">$row[lyric_title]</a></td>"; 
echo "<td><a href=\"index.php?lyric_id=$row[lyric_id]\" class=\"listing_results\">$row[score]</a></td>"; 
echo "<td><a href=\"index.php?lyric_id=$row[lyric_id]\" class=\"listing_results\">$row[views]</a></td>";
echo "</tr>";
}
echo "</table>";
echo "<table><tr><td>";
if($back >=0) { 
print "<a href='$page_name&start=$back&column_name=$column_name'>previous</a>"; 
} 
echo "</td><td>";
$i=0;
$l=1;
for($i=0;$i < $nume;$i=$i+$limit){
if($i <> $eu){
echo " <a href='$page_name&start=$i&column_name=$column_name'>$l</a> ";
}
else { echo "$l";}
$l=$l+1;
}

echo "</td><td>";
if($this1 < $nume) { 
print "<a href='$page_name&start=$next&column_name=$column_name'>next</a>";} 
echo "</td></tr></table>";
}
?>
<?php

}
include 'template.php';
?>
Thank you.

And I also don't understand, why for e.g. zend says here is "bug":

Code: Select all

while ($row=mysql_fetch_array($result)) {

Code: Select all

Assignment in condition (line 28)
And here is to part that I don't understand:

Code: Select all

<?php
if(strlen($row["youtube"])>6) {
$youtube=$row["youtube"];
} else {
$youtubesearchquery = str_replace(" ", "+", $row["lyric_artist"]." ".$row["lyric_title"]);
$a = file_get_contents("http://youtube.com/results?search_query=$youtubesearchquery&search=");
if(preg_match("/<a class=\"newvtitlelink\" href=\"(.*?)\" rel=\"nofollow\"/s", $a,$matches)) {
if($matches>0){
$b = file_get_contents("http://youtube.com".$matches[1]);
}
preg_match("/<input name=\"embed_code\" type=\"text\" value=\'(.*?)\' class=\"vidURLField\"/s", $b,$matchesb);
$youtube=$matchesb[1];
mysql_query("update lyrics set youtube='$matchesb[1]' where lyric_id ='$lyric_id'");
} else {
echo "No video avaible";
}
}
?>
It says:

Code: Select all

Variable $matches was used before it was defined (line 87)
.

Anyway, the most important think is to optimise queries, pleas help - asap.
Thank you.
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

Separate view from logic - at the moment it looks quite messy. Once you've done that, it's easier to profile SQL bottlenecks.
User avatar
spamyboy
Forum Contributor
Posts: 266
Joined: Sun Nov 06, 2005 11:29 am
Location: Lithuania, vilnius

Post by spamyboy »

what is "SQL bottle-necks" ? :|
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

Bottlenecks - well, imagine you want to drink a bottle of wine as quickly as possible. You tip the bottle as high as possible, but the speed with which the wine comes out doesn't change. The reason is that the opening the wine has to squeeze through (the bottleneck, i.e. top 3rd of the bottle) is much narrower than the bottom 2/3rds of the bottle. In short: bottlenecks are points that slow things down.
User avatar
spamyboy
Forum Contributor
Posts: 266
Joined: Sun Nov 06, 2005 11:29 am
Location: Lithuania, vilnius

Post by spamyboy »

Thanks for explanation :)

Question: Would using such class would help to optimise website ?

Code: Select all

<?php
class class_connectmysql {

    var $query;
    function connect($host,$user,$pass,$table) {
        $this->connected = mysql_connect($host, $user, $pass) or die ("Could not connect to MySQL");
          mysql_select_db ($table) or die ("Could not select database");
      }
    function query($sql) {
        if ($this->connected) {
            $this->query = mysql_query($sql) or die("Error: " . mysql_error());
        }
        else {
            return "Not connected to server or database is not selected";
        }
    }
}

$db=new class_connectmysql;
$db->connect("localhost","root","******","lyrics");
?>
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

Optimise in terms of speed? Probably not really.
Optimise in terms of readability? Most definitely. What I mean by separating view from logic: take those queries out of the PHP code (i.e. the page you posted initially), put them in a separate class whose methods then return the results.
User avatar
spamyboy
Forum Contributor
Posts: 266
Joined: Sun Nov 06, 2005 11:29 am
Location: Lithuania, vilnius

Post by spamyboy »

Well, ok... I'm kinde of new in using class, for e.g. this query, how could I put in class and use it in most optimised way ?
Sorry if this sounds like I would lazy, but basicly I try to do it more than 3 days and nothing good comes out of it.

Code: Select all

$commentquery = mysql_query("SELECT * FROM comments WHERE comment_lyricid='$lyric_id' ORDER BY comment_date") or die(mysql_error());
        $commentNum = mysql_num_rows($commentquery);
		if($commentNum>=1){
        echo "<div id=\"currentcomments\" class=\"submitcomment\"><h3 class=\"formtitle\">Current Comments</h3>\n";
		}
        while($commentrow=mysql_fetch_row($commentquery)){
        $commentbb = $commentrow[4];
                $commentDate = formatDate($commentrow[6]);
                echo "<div class=\"commentbody\" id=\"$commentrow[0]\">\n
                <p>$commentbb</p>\n
                <p class=\"postedby\">Posted by ";
                if($commentrow[3]){
                echo "<a href=\"$commentrow[3]\">$commentrow[2]</a> ";
                } else {
                echo "$commentrow[2] ";
                }
                echo "on $commentDate | #$commentrow[0]</p>\n
                \n</div>";
        }
        echo "</div>";
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

Once you separated logic from view you can profile how long individual methods take. Since all the queries are in separate methods, that's easy.

Code: Select all

$time_start = microtime(true);

// call the method
$myLogicClass->grabDataFromDB();

$time_end = microtime(true);
$time = $time_end - $time_start;

echo "->grabDataFromDB took $time seconds to execute.\n";
User avatar
spamyboy
Forum Contributor
Posts: 266
Joined: Sun Nov 06, 2005 11:29 am
Location: Lithuania, vilnius

Post by spamyboy »

You didn't realy realised what I'm asking for, I mean how for e.g. this query I could turn into class:

Code: Select all

$commentquery = mysql_query("SELECT * FROM comments WHERE comment_lyricid='$lyric_id' ORDER BY comment_date") or die(mysql_error());
        $commentNum = mysql_num_rows($commentquery);
                if($commentNum>=1){
        echo "<div id=\"currentcomments\" class=\"submitcomment\"><h3 class=\"formtitle\">Current Comments</h3>\n";
                }
        while($commentrow=mysql_fetch_row($commentquery)){
        $commentbb = $commentrow[4];
                $commentDate = formatDate($commentrow[6]);
                echo "<div class=\"commentbody\" id=\"$commentrow[0]\">\n
                <p>$commentbb</p>\n
                <p class=\"postedby\">Posted by ";
                if($commentrow[3]){
                echo "<a href=\"$commentrow[3]\">$commentrow[2]</a> ";
                } else {
                echo "$commentrow[2] ";
                }
                echo "on $commentDate | #$commentrow[0]</p>\n
                \n</div>";
        }
        echo "</div>";
I need only one e.g. and I will do everything else bms.
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

What I'm doing here with you is optimising, but not in the way you intend to. I'm trying to optimise your code so that you and others can look at it and see what's going on, see where possible bottlenecks are. The goal is readable code. Once you've achieved that, optimising for speed can be done.

Code: Select all

<?php
//note that this is PHP4.x syntax

class comments{

	function comments(){
		//permanent db-connection (best by reference)
	}

	function getCommentsByLyrics($id){
		$query	=	"SELECT * FROM comments WHERE id='$id' ORDER BY date";
		$result	=	mysql_query($query) or die("<xmp>".var_dump(debug_backtrace())."</xmp><br/>".mysql_error());
		while($record[]=mysql_fetch_row($commentquery)){
		}
		return $record;
	}
}

$comments 	= 	new comments();
$comment	=	$comments->getCommentsByLyrics(2);
echo"<h1>comment</h1><xmp>";print_r($comment);echo"</xmp>";
?>
User avatar
spamyboy
Forum Contributor
Posts: 266
Joined: Sun Nov 06, 2005 11:29 am
Location: Lithuania, vilnius

Post by spamyboy »

O thanks, that explained me alot, just one more question, what have you ment by saying:

Code: Select all

//permanent db-connection (best by reference)
"reference" ? you mean like.

Code: Select all

class MysQL {
      function connect($host,$user,$pass,$table) {
        $this->connected = mysql_connect($host, $user, $pass) or die ("Could not connect to MySQL");
          mysql_select_db ($table) or die ("Could not select database");
}
}

$db = new MySQL;
db->connect("localhost","root","*****","lyrics");
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

References: http://php.net/references
What I mean by "best by reference" is: different parts of code share the same DB-connection rather than creating new ones all the time (would be a definite performance issue).

Suppose you have two files

class_comments.inc.php

Code: Select all

<?php
//note that this is PHP4.x syntax

class comments{
	var $db;

	function comments($db){
		$this->db=$db; //here's where the reference is picked up and assigned to a class property;
	}

	function getCommentsByLyrics($id){
		$query	=	"SELECT * FROM comments WHERE id>'$id' ORDER BY date";
		$result	=	mysql_query($query,$this->db) or die("<xmp>".var_dump(debug_backtrace())."</xmp><br/>".mysql_error());
		while($record[]=mysql_fetch_row($result)){
		}
		return $record;
	}
}

?>
and test.php

Code: Select all

<?php
require_once("class_comments.inc.php");
$db = mysql_pconnect($db_details["host"], "root","");
mysql_select_db("lyrics",$connection);

$comments 	= 	new comments(&$connection); //here's were a reference to $connection gets passed to the constructor of the class
$comment	=	$comments->getCommentsByLyrics(2);
echo"<h1>comment</h1><xmp>";print_r($comment);echo"</xmp>";

?>
In this case, test.php establishes the connection but by passing it to class_comments they share the same connection.
Z3RO21
Forum Contributor
Posts: 130
Joined: Thu Aug 17, 2006 8:59 am

Post by Z3RO21 »

You can build the reference right into the functions parameters as well. For example

Code: Select all

<?PHP
  function foo(&$var) {
    $var++;
  }
  $a = 1;
  foo($a);
  echo $a //echos 2
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

yeah, but in this scenario it's pointless as it's a reference when passed to the class-constructor anyway.
User avatar
spamyboy
Forum Contributor
Posts: 266
Joined: Sun Nov 06, 2005 11:29 am
Location: Lithuania, vilnius

Post by spamyboy »

Thank you - helped me alot.
Now script works OK.
Post Reply