0

I just wrote this php tracker code, that will be included in themes, to have some simple stats. Now since it will process a lot of requests, it needs to be solid and fast. My php level isn't that good, so i'm looking for help / best practices to optimize this badboy.

I wrote a simple .htaccess file that redirects all requests to the index.php so i can process the request uri with php: $_SERVER['REQUEST_URI'], /this-is-the-theme-slug /user-name

<?php
/** MySQL Database Settings */
require dirname(__FILE__) . '/inc/database.php';

/** Klogger Log Framework*/
require dirname(__FILE__) . '/lib/KLogger.php';
$log = KLogger::instance(dirname(__FILE__).'/log/', KLogger::DEBUG);

/** Process Request String **/
$request = $_SERVER['REQUEST_URI'];
$ipaddr = $_SERVER['REMOTE_ADDR'];

$uri = explode('/',$request);
$slug = $uri[1];
$user = $uri[2];

/** Global Variables **/
$theme_id = NULL;
$account_id = NULL;


function process_request(){
    global $log, $slug, $user;
    if(!empty($slug) && !empty($user)){

        $the_slug = validate_slug($slug);
        $the_user = account_exists($user);

        if($the_slug){  // if the slug is valid
            if($the_user){  // and the user exists
                // update entry
                update_entry($user);
            }else{
                // create new entry
                create_entry($user);
            }
        }

        return true;
    }else{
        $log->logError('process_request:: Bad request');
        return false;
    }
}

function validate_slug($slug){
    global $log, $theme_id;

    $con = mysql_connect(DB_HOST,DB_USER,DB_PASSWORD);
    if(!$con){
        $log->logError('validate_slug:: Database connection failed');
        return false;   
    }else{
        $select = mysql_select_db(DB_NAME, $con);   
        $query = sprintf("SELECT id FROM ".DB_PREFIX."themes WHERE slug='%s'", mysql_real_escape_string($slug));
        $result = mysql_query($query);
        if(mysql_num_rows($result)==0){
            $log->logNotice('validate_slug:: Slug not found');
            return false;
        }else{
            $theme_id = mysql_result($result,0);
            return true;
        }   
        mysql_close($con);
    }
}

function account_exists($user){
    global $log, $account_id;

    $con = mysql_connect(DB_HOST,DB_USER,DB_PASSWORD);
    if(!$con){
        $log->logError('account_exists:: Database connection failed');
        return false;   
    }else{
        $select = mysql_select_db(DB_NAME, $con);   
        $query = sprintf("SELECT id FROM ".DB_PREFIX."stats WHERE account='%s'", mysql_real_escape_string($user));
        $result = mysql_query($query);
        if(mysql_num_rows($result)==0){
            $log->logNotice('account_exists:: Account not found');
            return false;
        }else{
            $account_id = mysql_result($result,0);
            return true;
        }   
        mysql_close($con);
    }
}

function create_entry($user){
    global $log, $ipaddr, $theme_id;

    $con = mysql_connect(DB_HOST,DB_USER,DB_PASSWORD);
    if(!$con){
        $log->logError('create_entry:: Database connection failed');
        return false;   
    }else{
        $select = mysql_select_db(DB_NAME, $con);
        $query = sprintf("INSERT INTO ".DB_PREFIX."stats (id,active,account,date,ip,hits,theme) VALUES ('','1','%s',NOW(),'".$ipaddr."','1','".$theme_id."')", mysql_real_escape_string($user));
        $result = mysql_query($query);

        $log->logNotice('create_entry:: Account created with id '.mysql_insert_id() );
        return true;
    }
    mysql_close($con);
}


function update_entry($user){
    global $log, $ipaddr, $account_id;  
    $con = mysql_connect(DB_HOST,DB_USER,DB_PASSWORD);
    if (!$con) {
        $log->logError('update_entry:: Database connection failed');
        return false;
    } else {
        $select = mysql_select_db(DB_NAME, $con);
        $query = sprintf("UPDATE ".DB_PREFIX."stats SET date=NOW(),ip='".$ipaddr."',hits=hits+1 WHERE id='".$account_id."'");
        $result = mysql_query($query);

        $log->logNotice('update_entry:: Entry with id '.$account_id.' is updated.' );
        return true;
    }
    mysql_close($con);
}


process_request();

EDIT I have successfully created a working class for my tracker, See code below. If you have any improvements in speed, let me know! I tried connecting with mysqlli and pdo, but it didn't work, apearrently my hosting didn't support it?

    <?php 
    require_once(__DIR__ . '/inc/database.php');
    require_once(__DIR__ . '/lib/KLogger.php');

    Class ThemeStatsTracker
    {
        public $log;

        private $theme_name;
        private $theme_user;
        private $theme_name_db_id;
        private $theme_user_db_id;

        public function __construct()
        {
            // Setup Log 
            $this->log = KLogger::instance(dirname(__FILE__).'/log/', KLogger::DEBUG);

            // Process URI variables
            $uri = explode('/',$_SERVER['REQUEST_URI']);    
            $this->theme_name = (!empty($uri[1])) ? $uri[1] : NULL;
            $this->theme_user = (!empty($uri[2])) ? $uri[2] : NULL;

            // Handle the Request
            $this->database_connect();
            if($this->validate_theme()){
                if($this->user_entry_exists()){
                    $this->user_entry_update(); 
                }else{
                    $this->user_entry_create();
                }   
            }

            // Always serve an image as response
            $this->serve_image();
        }

        private function validate_theme()
        {
            $query  = sprintf("SELECT id FROM ".DB_PREFIX."themes WHERE slug='%s' LIMIT 1", mysql_real_escape_string($this->theme_name));
            $result = mysql_query($query);
            if (!$result){
                $this->log->logError(__FUNCTION__ . ' FAIL: ' . $query . ' BECAUSE: ' . mysql_error());
                return FALSE;
            }
            if(mysql_num_rows($result)==0){
                $this->log->logError(__FUNCTION__ . ' Theme name ' . $this->theme_name . ' NOT found');
                return FALSE;
            }else{
                $this->log->logInfo(__FUNCTION__ . ' Theme name ' . $this->theme_name . ' found');
                $this->theme_name_db_id = mysql_result($result,0);
                return TRUE;
            }
        }

        private function user_entry_exists()
        {
            $query  = sprintf("SELECT id FROM ".DB_PREFIX."stats WHERE account='%s' LIMIT 1", mysql_real_escape_string($this->theme_user));
            $result = mysql_query($query);
            if (!$result){
                $this->log->logError(__FUNCTION__ . ' FAIL: ' . $query . ' BECAUSE: ' . mysql_error());
                return FALSE;
            }
            if(mysql_num_rows($result)==0){
                $this->log->logInfo(__FUNCTION__ . ' New user ' . $this->theme_user);
                return FALSE;
            }else{
                $this->log->logInfo(__FUNCTION__ . ' Existing user ' . $this->theme_user);
                $this->theme_user_db_id = mysql_result($result,0);
                return TRUE;
            }   
        }

        private function user_entry_create()
        {
            $query  = sprintf("INSERT INTO ".DB_PREFIX."stats (id,active,account,date,ip,hits,theme) VALUES ('','1','%s',NOW(),'".$_SERVER['REMOTE_ADDR']."','1','".$this->theme_name_db_id."')", mysql_real_escape_string($this->theme_user));
            $result = mysql_query($query);
            if (!$result){
                $this->log->logError(__FUNCTION__ . ' FAIL: ' . $query . ' BECAUSE: ' . mysql_error());
                return FALSE;
            }
            $this->log->logNotice(__FUNCTION__ . ' New user created with id ' . mysql_insert_id());
            return TRUE;
        }

        private function user_entry_update()
        {
            $query  = sprintf("UPDATE ".DB_PREFIX."stats SET date=NOW(),ip='".$_SERVER['REMOTE_ADDR']."',hits=hits+1 WHERE id='".$this->theme_user_db_id."' LIMIT 1");
            $result = mysql_query($query);
            if (!$result){
                $this->log->logError(__FUNCTION__ . ' FAIL: ' . $query . ' BECAUSE: ' . mysql_error());
                return FALSE;
            }
            $this->log->logNotice(__FUNCTION__ . ' User with id ' . $this->theme_user_db_id . 'updated');
            return TRUE;
        }

        private function serve_image()
        {
            header("Content-type: image/gif");
            header("Content-length: 43");
            $fp = fopen("php://output","wb");
            fwrite($fp,"GIF89a\x01\x00\x01\x00\x80\x00\x00\xFF\xFF",15);
            fwrite($fp,"\xFF\x00\x00\x00\x21\xF9\x04\x01\x00\x00\x00\x00",12);
            fwrite($fp,"\x2C\x00\x00\x00\x00\x01\x00\x01\x00\x00\x02\x02",12);
            fwrite($fp,"\x44\x01\x00\x3B",4);
            fclose($fp);
        }

        private function database_connect()
        {
            $con = mysql_connect(DB_HOST,DB_USER,DB_PASSWORD);
            if(!$con){
                $this->log->logError(__FUNCTION__ . ' FAIL: ' . $query . ' BECAUSE: ' . mysql_error());
                return FALSE;
            }
            $select = mysql_select_db(DB_NAME, $con);
            if (!$select){
                $this->log->logError(__FUNCTION__ . ' FAIL: ' . $query . ' BECAUSE: ' . mysql_error());
                return FALSE;
            }
        }       
    }

    $stats = new ThemeStatsTracker();
CKDT
  • 97
  • 11
  • Premature optimization is the root of all evil. Go figure. – Your Common Sense Dec 27 '12 at 19:47
  • Unless you have a specific problem that needs a solution, [Code Review SE](http://codereview.stackexchange.com/) would be a better fit for this question. (Edit: of course, using `mysql_` functions is the first red flag for me.) – DCoder Dec 27 '12 at 19:47
  • if it works it ain't broke – nathan hayfield Dec 27 '12 at 19:48
  • 2
    This would probably be a good time to let you know you should start replacing the `mysql_*` functions. As of `PHP 5.5.0` they are deprecated. Use something like [PDO](http://php.net/manual/en/book.pdo.php) or [MySQLi](http://php.net/manual/en/book.mysqli.php) – PhearOfRayne Dec 27 '12 at 19:48

3 Answers3

1

Whenever there is a potential performance issue, you can bet it is in the I/O subsystem and in modern web applications that usually means the data base, so that is where you go to find and fix performance issues. Here are my brief rules of thumb.

  1. Avoid SELECT * and instead SELECT only the columns you actually need.
  2. Use EXPLAIN SELECT on every complex query. By "complex" I mean any query that has more than one or two WHERE clause elements, or that uses more than one table.
  3. Have indexes on every column used in WHERE, JOIN, ORDER or GROUP
  4. Use a LIMIT clause on every query that does not absolutely require a complete table scan, and that includes UPDATE queries.

Those are general principles that you can apply to the query statements.

To the specifics in the code above, make your server connect and db select once at the start of the script. The connection and selection are effectively global in scope. Remove the close-connection statements. The PHP garbage collector will do that for you.

You might want to read up on the query() functions. Most of the data base functions return values, usually a resource or FALSE on failure. Your script should test these values and deal with the behavior. MySQL is not a black box; it can and will fail for reasons outside of your control. When that happens, you want to log the error and raise an alert.

You might consider rewriting this in object-oriented notation, using protected class properties for things like $slug and $log. It would help you avoid the use of the global statement, which is a sure path to confusion because it destroys encapsulation.

HTH, ~Ray

Ray Paseur
  • 2,106
  • 2
  • 13
  • 18
  • Thanks for the quick reply. The mysql select tips and the overall remarks on the code are a big help. As for the PDO or MySQLi methods. I think they are way to advanced for me. Do you happen to know a good resource for learning more about the basics of OOP in php. I mean, this is pretty basic stuff, so i am really interested in how to create a good class out of these functions. Cheers. – CKDT Dec 27 '12 at 20:43
  • I recommend this book: http://www.amazon.com/PHP-Object-Oriented-Solutions-David-Powers/dp/1430210117 and at a little more advanced level, this book: http://www.amazon.com/Objects-Patterns-Practice-Experts-Source/dp/143022925X/ – Ray Paseur Dec 27 '12 at 20:50
  • This man page is useful http://php.net/manual/en/language.constants.predefined.php – Ray Paseur Dec 27 '12 at 20:53
  • This man page and extensions will be helpful, too. http://php.net/manual/en/language.oop5.php – Ray Paseur Dec 27 '12 at 20:55
0

For comparison purposes, this is how I might write the tracker using OOP notation. I have never tested this, but it seems pretty close to correct. Also, PHP has a built in function to log errors and notices. http://php.net/manual/en/function.error-log.php

<?php // RAY_temp_ckdt.php
error_reporting(E_ALL);

/** MySQL Database CONSTANT DEFINITIONS */
require_once( __DIR__ . '/inc/database.php');

/** Klogger Log Framework*/
require_once( __DIR__ . '/lib/KLogger.php');
$log = KLogger::instance(dirname(__FILE__).'/log/', KLogger::DEBUG);



Class Tracker
{
    protected $return, $slug, $user, $theme_id, $account_id;
    public function __construct($log)
    {
        $this->return = TRUE;
        $this->log    = $log;
        $con = mysql_connect(DB_HOST,DB_USER,DB_PASSWORD);
        if(!$con){
            $this->log->logError('DB FAIL: ' . mysql_error());
        }
        $select = mysql_select_db(DB_NAME, $con);
        if (!$select){
            $this->log->logError('DB FAIL: ' . mysql_error());
            return FALSE;
        }
        $uri = explode(DIRECTORY_SEPARATOR, $_SERVER['REQUEST_URI']);
        $this->slug = (!empty($uri[1])) ? $uri[1] : NULL;
        $this->user = (!empty($uri[2])) ? $uri[2] : NULL;

        if(empty($this->slug) || !empty($this->user)){
            $this->log->logError(__FUNCTION__ . ": Bad request");
            return FALSE;
        }
        $the_slug = validate_slug($this->slug);
        $the_user = account_exists($this->user);

        if($the_slug){
            if($the_user){
                update_entry();
            }else{
                create_entry();
            }
        }
        return $this->return;
    }

    protected function validate_slug(){
        $query  = sprintf("SELECT id FROM ".DB_PREFIX."themes WHERE slug='%s' LIMIT 1", mysql_real_escape_string($this->slug));
        $result = mysql_query($query);
        if (!$result){
            $this->log->logError(__FUNCTION__ . "FAIL: $query BECAUSE: " . mysql_error());
            $this->return = FALSE;
            return FALSE;
        }
        if(mysql_num_rows($result)==0){
            $this->log->logNotice(__FUNCTION__ . ": Slug $this->slug not found");
            $this->return = FALSE;
            return FALSE;
        }else{
            $this->theme_id = mysql_result($result,0);
            $this->return = TRUE;
            return TRUE;
        }
    }

    protected function account_exists(){
        $query  = sprintf("SELECT id FROM ".DB_PREFIX."stats WHERE account='%s' LIMIT 1", mysql_real_escape_string($this->user));
        $result = mysql_query($query);
        if (!$result){
            $this->log->logError(__FUNCTION__ . "FAIL: $query BECAUSE: " . mysql_error());
            $this->return = FALSE;
            return FALSE;
        }
        if(mysql_num_rows($result)==0){
            $this->log->logNotice(__FUNCTION__ . ": Account $this->user not found");
            $this->return = FALSE;
            return FALSE;
        }else{
            $this->account_id = mysql_result($result,0);
            $this->return = TRUE;
            return TRUE;
        }
    }

    protected function create_entry(){
        $query  = sprintf("INSERT INTO ".DB_PREFIX."stats (id,active,account,date,ip,hits,theme) VALUES ('','1','%s',NOW(),'".$_SERVER['REMOTE_ADDR']."','1','".$theme_id."')", mysql_real_escape_string($this->user));
        $result = mysql_query($query);
        if (!$result){
            $this->log->logError(__FUNCTION__ . "FAIL: $query BECAUSE: " . mysql_error());
            $this->return = FALSE;
            return FALSE;
        }

        $this->log->logNotice(__FUNCTION__ . ': Account created with id '.mysql_insert_id() );
        $this->return = TRUE;
        return TRUE;
    }

    public function update_entry(){
        $query  = sprintf("UPDATE ".DB_PREFIX."stats SET date=NOW(),ip='".$_SERVER['REMOTE_ADDR']."',hits=hits+1 WHERE id='".$this->account_id."' LIMIT 1");
        $result = mysql_query($query);
        if (!$result){
            $this->log->logError(__FUNCTION__ . "FAIL: $query BECAUSE: " . mysql_error());
            $this->return = FALSE;
            return FALSE;
        }

        $this->log->logNotice(__FUNCTION__ . ': Entry with id '.$account_id.' is updated.' );
        $this->return = TRUE;
        return TRUE;
    }
} // END CLASS TRACKER

// TRACK THIS REQUEST
$x = new Tracker($log);
Ray Paseur
  • 2,106
  • 2
  • 13
  • 18
  • Thank you for the comparison class,really helpfull. I am aware of the Error reporting class of php. But i need the errors to be logged to a file, because the code should in the end return an image header. So i can't print errors on screen. – CKDT Dec 27 '12 at 21:11
0

The rules of Optimization Club:

  1. The first rule of Optimization Club is, you do not optimize.
  2. The second rule of Optimization Club is, you do not optimize without measuring.
  3. If your app is running faster than the underlying transport protocol, the optimization is over.
  4. One factor at a time.
  5. No marketroids, no marketroid schedules.
  6. Testing will go on as long as it has to.
  7. If this is your first night at Optimization Club, you have to write a test case.

The most important rule is #1. If you don't know that your code is slow, then don't try to speed it up. Then, for #2, if you think you need to speed it up, you have to measure what about it is slow. This can be low-tech like putting in print microtime() calls throughout your code so you can see how long individual chunks of code take to run, or you can use a profiling tool like XDebug to discover exactly which lines of code take how long to run.

Whatever you do, don't take working code and just start whacking at it trying to make it faster.

(And, unrelated to your question, you should be using parametrized queries for safety and ease: http://bobby-tables.com/php.html has examples)

Andy Lester
  • 91,102
  • 13
  • 100
  • 152