MySQL_Narb Posted August 5, 2012 Share Posted August 5, 2012 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); }); }); Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/ Share on other sites More sharing options...
darkfreaks Posted August 5, 2012 Share Posted August 5, 2012 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/ Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366892 Share on other sites More sharing options...
MySQL_Narb Posted August 5, 2012 Author Share Posted August 5, 2012 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); Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366893 Share on other sites More sharing options...
darkfreaks Posted August 5, 2012 Share Posted August 5, 2012 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. Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366894 Share on other sites More sharing options...
MySQL_Narb Posted August 5, 2012 Author Share Posted August 5, 2012 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? Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366895 Share on other sites More sharing options...
PFMaBiSmAd Posted August 5, 2012 Share Posted August 5, 2012 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. Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366896 Share on other sites More sharing options...
darkfreaks Posted August 5, 2012 Share Posted August 5, 2012 before you go any further i would advise reading the below on periodic refresh. it talks a great deal on how to specifically reduce the load on resources. http://ajaxpatterns.org/Periodic_Refresh Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366899 Share on other sites More sharing options...
MySQL_Narb Posted August 5, 2012 Author Share Posted August 5, 2012 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! Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366900 Share on other sites More sharing options...
PFMaBiSmAd Posted August 5, 2012 Share Posted August 5, 2012 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? Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366903 Share on other sites More sharing options...
MySQL_Narb Posted August 5, 2012 Author Share Posted August 5, 2012 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 table: users: table: online_users Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366911 Share on other sites More sharing options...
PFMaBiSmAd Posted August 5, 2012 Share Posted August 5, 2012 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. Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1366938 Share on other sites More sharing options...
PFMaBiSmAd Posted August 6, 2012 Share Posted August 6, 2012 After you reduce your code and queries as has been suggested, you will be ready for the next step in optimizing this and use JOINed queries to get the data and the displayNames in one single query. Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1367146 Share on other sites More sharing options...
Christian F. Posted August 6, 2012 Share Posted August 6, 2012 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. Quote Link to comment https://forums.phpfreaks.com/topic/266695-resource-hog/#findComment-1367331 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.