Jump to content

Resource hog?

MySQL_Narb

By MySQL_Narb
in PHP Coding Help

 Share

Recommended Posts

MySQL_Narb Regular Member

MySQL_Narb

Posted
Posted

I made a pretty basic shoutbox/chatbox script, and I just put up a live version for people to test. But, within fifteen minutes the site was suspended automatically for using too much resources.

 

Would you consider the below too much:

Using AJAX to call loadChatbox.php every 2.5 seconds. loadChatbox.php will return a list of messages posted in the chatbox recently (limit of twenty-five).

Using AJAX to call for the latest announcement and online user list every six seconds. The user list extracts all online users, and the AJAX call also extracts one announcement (the latest).

 

loadChatbox.php (every 2 seconds)

<?php
    require('../includes/config.php');
    include('../structure/database.php');
    include('../structure/user.php');
    include('../structure/chat.php');
    
    $database = new database($db_host, $db_name, $db_user, $db_password);
    $user = new user($database);
    
    //store id so we don't have to reuse methods
    $id = $user->getIdBySession($_COOKIE['user']);
    
    $messages = $database->processQuery("SELECT `id`,`userid`,`console`,`status`,`message` FROM `chat` ORDER BY `id` DESC LIMIT 25", array(), true);
    
    foreach($messages as $message){
        $content = '';
        if(substr($message['message'], 0, 4) == '/me '){
            $action = explode(' ', $message['message'], 2);
            echo '*'.$user->displayName($message['userid']).' '. htmlentities($action[1]).'<br/>';
        }elseif($message['console'] == 0){
            if($user->getRank($id) >= 1) $content .= '[<img id="hide-'. $message['id'] .'" src="images/hide.png">]';
            echo ($message['status'] == 1) ? 'This message has been removed<br/>' : $content .= $user->displayName($message['userid']).': '. htmlentities($message['message']).'<br/>';
        }else{
            echo '<br/>[CONSOLE] <b>'. $message['message'].'</b><br/>';
        }
    }
?>

 

online.php (every six seconds)

<?php
require('../includes/config.php');
include('../structure/database.php');
include('../structure/user.php');
include('../structure/chat.php');

$database = new database($db_host, $db_name, $db_user, $db_password);
$user = new user($database);
$chat = new chat($database, $user);

$online_users = $chat->usersOnline();
?>
<b>Online [<?php echo count($online_users); ?>]:</b>
<hr>
<?php
    foreach($online_users as $user_online){
        $user_online_name = $user->displayName($user_online);
        echo $user_online_name.'<br/>';
    }
?>

 

latestNotice.php (every six seconds)

<?php
require('../includes/config.php');
include('../structure/database.php');
include('../structure/user.php');
include('../structure/chat.php');

$database = new database($db_host, $db_name, $db_user, $db_password);
$user = new user($database);
$chat = new chat($database, $user);

//get latest notice
$query = $database->processQuery("SELECT `message` FROM `notices` ORDER BY `id` DESC LIMIT 0,1", array(), true);

if($database->getRowCount() == 0){
    echo 'There is no notice to display.';
}else{
    echo $query[0]['message'];
}
?>

 

and last but not least, load.js

 

$(document).ready(function(){
       loadData(); setInterval(loadData, 6000);
       loadChatBox(); setInterval(loadChatBox, 2500);
       function loadData(){
        $.ajax({
            url: "js/latestNotice.php",
            cache: false
        }).done(function( notice ) {
            $("#notice").html('<b>Latest Notice:</b> ' + notice);
        });
        
        $.ajax({
            url: "js/online.php",
            cache: false
        }).done(function( online ) {
            $("#users").html(online);
        });
    }
    
    function loadChatBox(){
        $.ajax({
            url: "js/loadChatbox.php",
            cache: false
        }).done(function( chat ) {
            $("#chatbox").html(chat);
        });
    }
    
    function sendMessage(){
        message_data = $('input[id|="message"]').val();
        $.ajax({
            type: "POST",
            url: "js/sendMessage.php",
            data: { message: message_data }
        }).done(function(){
            $('input[id="message"]').attr('value', '');
        });
    }
    
    $('input[id="send"]').click(function(){
        sendMessage();
    });
    
    
    $('input[id="message"]').keydown(function(e){
        if(e.keyCode == 13) sendMessage();
    });
    
    $('image[id|="hide"]').click(function(){
        id = $(this).attr('id').split('-')[1];
        document.write(id);
    });
});

Link to comment
Share on other sites
MySQL_Narb Regular Member

MySQL_Narb

Posted
  • Author
Posted

might have to set a time out in the JavaScript part if it is loading every two seconds i imagine that uses a ton of bandwidth.

 

 

 

http://api.jquery.com/delay/

 

I believe .delay() is only for animations and that nature.

 

I'm already using intervals/delays:

loadData(); setInterval(loadData, 6000);
loadChatBox(); setInterval(loadChatBox, 2500);

Link to comment
Share on other sites
darkfreaks Advanced Member

darkfreaks

Posted
Posted

might have to set a time out in the JavaScript part if it is loading every two seconds i imagine that uses a ton of bandwidth.

 

 

 

http://api.jquery.com/delay/

 

I believe .delay() is only for animations and that nature.

 

I'm already using intervals/delays:

loadData(); setInterval(loadData, 6000);
loadChatBox(); setInterval(loadChatBox, 2500);

 

 

The setInterval() method will continue calling the function until clearInterval() is called, or the window is closed.

Link to comment
Share on other sites
MySQL_Narb Regular Member

MySQL_Narb

Posted
  • Author
Posted

might have to set a time out in the JavaScript part if it is loading every two seconds i imagine that uses a ton of bandwidth.

 

 

 

http://api.jquery.com/delay/

 

I believe .delay() is only for animations and that nature.

 

I'm already using intervals/delays:

loadData(); setInterval(loadData, 6000);
loadChatBox(); setInterval(loadChatBox, 2500);

 

 

The setInterval() method will continue calling the function until clearInterval() is called, or the window is closed.

 

Yeah...that's the point?

Link to comment
Share on other sites
PFMaBiSmAd Advanced Member

PFMaBiSmAd

Posted
Posted

You are calling at least two different methods of your user class inside of loops. I will guess you are likely performing queries inside those methods. In the case of the loadChatbox.php code, that would result in up to 50 queries (assuming only one query is being executed in each method) every 2 seconds.

 

What is the code in your user class, so that more specific help can be given?

 

In general, you should never retrieve a set of data from a database by performing a query inside of a loop. You should form a query that gets all the data you want using one single query and then simply iterate over that data and output it the way you want.

Link to comment
Share on other sites
MySQL_Narb Regular Member

MySQL_Narb

Posted
  • Author
Posted

You are calling at least two different methods of your user class inside of loops. I will guess you are likely performing queries inside those methods. In the case of the loadChatbox.php code, that would result in up to 50 queries (assuming only one query is being executed in each method) every 2 seconds.

 

What is the code in your user class, so that more specific help can be given?

 

In general, you should never retrieve a set of data from a database by performing a query inside of a loop. You should form a query that gets all the data you want using one single query and then simply iterate over that data and output it the way you want.

 


<?php

class user {
    private $database;
    private $isLoggedIn;
    private $id;
    
    function __construct(database $database = null){
        if(is_null($database)){
            die('USER:CONSTRUCTOR ERROR: database can\'t be null');
        }else{
            $this->database = $database;
            
            if(isset($_COOKIE['user'])){
                $database->processQuery("SELECT * FROM `users` WHERE `session` = ? LIMIT 1", array($_COOKIE['user']), false);
                
                if($database->getRowCount() == 0){
                    setcookie('user', null, time()-100000, '/');
                }else{
                    if($this->isBanned($this->getIdBySession($_COOKIE['user']))){
                        setcookie('user', null, time()-100000, '/');
                    }else{
                        $this->isLoggedIn = true;
                        $this->id = $this->getIdByUsername($this->getUsernameBySession($_COOKIE['user']));
                    }
                }
            }
        }
    }
    
    public function isLoggedIn(){
        return ($this->isLoggedIn) ? true : false;
    }
    
    public function returnId(){
        return $this->id;
    }
    
    public function getUsernameById($id){
        $query = $this->database->processQuery("SELECT `username` FROM `users` WHERE `id` = ? LIMIT 1", array($id), true);
        return $query[0]['username'];
    }
    
    public function getIdByUsername($username){
        $query = $this->database->processQuery("SELECT `id` FROM `users` WHERE `username` = ? LIMIT 1", array($username), true);
        return $query[0]['id'];
    }
    
    public function getIdBySession($session){
        $query = $this->database->processQuery("SELECT `id` FROM `users` WHERE `session` = ? LIMIT 1", array($session), true);
        return $query[0]['id'];
    }
    
    public function getUsernameBySession($session){
        $query = $this->database->processQuery("SELECT `username` FROM `users` WHERE `session` = ? LIMIT 1", array($session), true);
        return $query[0]['username'];
    }
    
    public function getRank($id){
        $query = $this->database->processQuery("SELECT `level` FROM `users` WHERE `id` = ? LIMIT 1", array($id), true);
        return $query[0]['level'];
    }
    
    public function banUser($id){
        $this->database->processQuery("UPDATE `users` SET `banned` = 1 WHERE `id` = ? LIMIT 1", array($id), false);
    }
    
    public function muteUser($id){
        $this->database->processQuery("UPDATE `users` SET `muted` = 1 WHERE `id` = ? LIMIT 1", array($id), false);
    }
    
    public function isBanned($id){
        $this->database->processQuery("SELECT * FROM `users` WHERE `id` = ? AND `banned` = 1 LIMIT 1", array($id), false);
        return ($this->database->getRowCount() > 0) ? true : false;
    }
    
    public function isMuted($id){
        $this->database->processQuery("SELECT * FROM `users` WHERE `id` = ? AND `muted` = 1 LIMIT 1", array($id), false);
        return ($this->database->getRowCount() > 0) ? true : false;
    }
    
    public function doesExist($username){
        $this->database->processQuery("SELECT * FROM `users` WHERE `username` = ?", array($username), false);
        return ($this->database->getRowCount() == 0) ? false : true;
    }
    
    public function canRegister($username){
        $username = trim($username);
        
        $array = array(); 
        $array[0] = true;
        
        $this->database->processQuery("SELECT * FROM `banned_ips` WHERE `ip` = ? LIMIT 1", array($_SERVER['REMOTE_ADDR']), false);
        if($this->database->getRowCount() > 0) { $array[0] = false; $array[1] = 'You cannot create a new account.'; }
        
        if($this->doesExist($usernmae)) { $array[0] = false; $array[1] = 'This username has already been registered.'; }
        
        if(preg_match('~[^a-zA-Z0-9_ ]~',$username)) { $array[0] = false; $array[1] = 'Your username can only contain a-z, A-Z, 0-9, _, and spaces.'; }
        
        if(strlen($username) < 3 || strlen($username) > 12) { $array[0] = false; $array[1] = 'Your username must be at least three characters, and no more than twelve.'; }
        
        return $array;
    }
    
    public function generateSession(){
        $chars = array('A', 'b', 'C', 'D', '!', '^', '9', '8', '7', '6', '5', '4', '3', '2', '1', '0', ')', '(', '#', 'z', 'x', 'y', 't', 's', '?', '>', '<');
        
        $string = '';
        for($i = 0; $i < 60; $i++){
            $string .= $chars[rand(0, 26)];
        }
        
        return substr(hash(sha256, $string), 0, 35);
    }
    
    public function updateOnlineStatus($id){
        if($this->isLoggedIn()){
            $this->database->processQuery("SELECT * FROM `online_users` WHERE `userid` = ?", array($id), false);
        
            if($this->database->getRowCount() == 0){
                $this->database->processQuery("INSERT INTO `online_users` VALUES (?, ?)", array($id, time()), false);
            }else{
                $this->database->processQuery("UPDATE `online_users` SET `last_active` = ? WHERE `userid` = ?", array(time(), $id), false);
            }
        }
    }
    
    public function displayName($id){
        $username = $this->getUsernameById($id);
       
        switch($this->getRank($id)){
            case 0:
            default:
                $username = $username;
                break;
            case 1:
                $username = '[M] <font color="green">'. $username .'</font>';
                break;
            case 2:
            case 3:
                $username = '[A] <font color="red">'. $username .'</font>';
                break;
        }
        
        return $username;
    }
}

?>

 

Also, may I ask where I'm looping through queries unexpectedly? Thanks!

Link to comment
Share on other sites
PFMaBiSmAd Advanced Member

PFMaBiSmAd

Posted
Posted

Your user class is doing what I suggested. You are figuratively killing your database server with all the queries inside of loops.

 

Some suggestions -

 

For loadChatbox.php:

  • You need to get a list of the $message['userid'] values and execute one query that gets all the corresponding display names at one time, using WHERE id IN(a comma separated list of ids goes here...) in the query statement. Retrieve the display names into an array with the userid as the array key. Then simply reference the display name when you need it using $message['userid'] as the array index value.
  • $id is a fixed value during any one invocation of your script. Therefore,$user->getRank($id) is a fixed value during any one invocation of your script. By putting $user->getRank($id) inside of the foreach() loop, you are executing a query every pass through the loop to get the same value over and over again. Just assign $user->getRank($id) to a variable once before the start of the loop and reference that variable where you have the $user->getRank($id) reference now.

 

For online.php:

  • You need to get a list of userid's and execute one query that gets all the corresponding display names at one time, using WHERE id IN(a comma separated list of ids goes here...) in the query statement. Retrieve the display names into an array and then simply iterate over the array of display names.

 

Also, are your database tables properly indexed so that each query is as efficient as possible?

Link to comment
Share on other sites
MySQL_Narb Regular Member

MySQL_Narb

Posted
  • Author
Posted

Your user class is doing what I suggested. You are figuratively killing your database server with all the queries inside of loops.

 

So, with this:

 

foreach($messages as $message){
        //...
    }

 

I'm actually calling a query for each loop/item? If that's true, I'm going to assume it applies to all type of loops? I've been doing things this way since the beginning of my time with PHP, and I never thought of it that way. I always assumed the query is called once, and all the data is stored within the signed variable.

 

For loadChatbox.php:

  • You need to get a list of the $message['userid'] values and execute one query that gets all the corresponding display names at one time, using WHERE id IN(a comma separated list of ids goes here...) in the query statement. Retrieve the display names into an array with the userid as the array key. Then simply reference the display name when you need it using $message['userid'] as the array index value.
  • $id is a fixed value during any one invocation of your script. Therefore,$user->getRank($id) is a fixed value during any one invocation of your script. By putting $user->getRank($id) inside of the foreach() loop, you are executing a query every pass through the loop to get the same value over and over again. Just assign $user->getRank($id) to a variable once before the start of the loop and reference that variable where you have the $user->getRank($id) reference now.

 

1st item: That's a very useful tip there. Took me a bit to understand, but I did a quick search of the meaning. I will definitely be using this method. Thanks!

2nd item: I guess I overlooked that. I'll be sure to change this immediately along with the above.

 

For online.php:

  • You need to get a list of userid's and execute one query that gets all the corresponding display names at one time, using WHERE id IN(a comma separated list of ids goes here...) in the query statement. Retrieve the display names into an array and then simply iterate over the array of display names.

 

What do you think of this?

 

public function displayName(array $ids){
        $query = $this->database->processQuery("SELECT `level` FROM `users WHERE id IN ($IDS)", array(), true);
}

 

Obviously the method isn't functional, but my question is: How would I go about adding all the IDs within the IN statement in my query? Do I just append them to a string, like:

 

public function displayName(array $ids){
        //number of ids
        $num = count($ids);
        
        $id_list = '';
        for($i = 0; $i <= $num; $i++){
            $id_list .= ($i == $num) ? $ids[$i] : $ids[$i].',';
        }
        
        $query = $this->database->processQuery("SELECT `level` FROM `users WHERE id IN (?)", array($id_list), true);
}

 

Also, are your database tables properly indexed so that each query is as efficient as possible?

 

I'm going to assume I am.

 

table: chat

 

OL0N

 

table: users:

 

OL1w

 

table: online_users

 

OL1J

Link to comment
Share on other sites
PFMaBiSmAd Advanced Member

PFMaBiSmAd

Posted
Posted
I'm actually calling a query for each loop/item?

 

Your code inside the foreach() loops in loadChatbox.php and online.php is calling one or both of your $user->displayName(....) and $user->getRank(....) methods. Your code inside those two methods is executing a query using the passed id parameter and returning the resultant displayName or rank for that id.

 

I'm going to assume it applies to all type of loops?

 

You have got to be kidding. It applies to the loops where you have put code inside of the loop that executes a query.

 

I always assumed the query is called once

 

If that is the case, how would your current code ever be able to operate on more than one id value and return the different displayNames for all the different people who have a message displayed in the chat window or who is listed as being online?

 

You would use implode to form a comma separated list of id's to put into the IN() term in the query.

 

 

 

Link to comment
Share on other sites
Christian F. Member

Christian F.

Posted
Posted

Oh, man.... Where to start?

 

First off: You'll want to use an absolute minimum of SQL queries in these scripts. One query per load would be optimal, 2-3 could be permissible if they were optimized. Anything more than that, and you start to put quite a lot of stress on the server.

Absolutely never use any queries, or functions that do, inside a loop for things like these!

 

Secondly: Contain all of the code you want to use in a single file, to cut down on the number of filesystem access operations. Not that noticeable during regular load, but it adds up if you have a couple of hundred active users at any given time.

 

Third off: Don't use echo inside loops, but use a variable to contain the generated content instead. Same with PHP tags, you want only one set of those, as they are even more expensive than echo.

 

Fourth point: You will need to use something that archives old messages from your chat table, to ensure speed over time. When you get a few hundred thousand rows the response time of the queries will start to lag quite noticeably otherwise.

This happens because when people submit new chat messages it invalidates the cache the MySQL engine has built up already, so the more active people are at chatting the fewer rows will trigger this problem.

 

Fifth hint: As a part of doing everything in a single file, you might actually want to reproduce some code. So that you can avoid using your (full) User class, and instead just use the few lines of code that you absolutely need.

 

Sixth item: When I wrote my AJAX chat script, I even went as far as writing my own session read function. As "session_start ()" was too slow and put too much strain on the system, at peak times.

 

In conclusion you want to reduce and refactor your code as much as possible, put it through some really rigorous stress testing, as well as profiling the heck out of it to identify any (potential) bottlenecks. You will also have to monitor it quite often, to observe performance over time, as it can and will change.

This is one of the few cases where micro-optimization can actually be worthwhile and sensible. Though, always remember to profile your code first.

Link to comment
Share on other sites
This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
 Share
Go to topic listing
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.