Poor performance: Instantiation of objects, includes and classes

[ken.kristoffersen]

Hi guys,

 

I'm in over my head here and hoping that one of you genuises can help me out

 

I'm currently converting my site from procedural to object oriented and i'm seriously doing one or more things completely wrong because the performance of the site sucks even for the smallest tasks.

 

The filestructure, and my code can be seen beneath. I have also attached my project.

 

What I think that I'm doing wrong is the instantiation of the MySQLiDAO object in the constructor of my ForeignExchange class.

I just don't know where else to put it.

 

I hope one of you can point me in the right direction or enlighten me regarding the best way of doing object instantiation.

 

Thank you very much in advance.

 

Best regards

Kenneth

 

Filestructure:

- index.php

- lib [folder]

- - model [folder]

- - - BaseModel.php

- - - ForeignExchangeModel.php

- - logic [folder]

- - - ForeignExchange.php

- - data [folder]

- - - MySQLiDAO.php

 

index.php

require_once('lib/data/MySQLiDAO.php');
require_once('lib/model/BaseModel.php');
require_once('lib/model/ForeignExchangeModel.php');
require_once('lib/logic/ForeignExchange.php');
require_once('lib/logic/curl.php');

$url = 'http://nationalbanken.dk/dndk/valuta.nsf/valuta.xml';

$curl = new Curl();
$content = $curl->load($url);

$xml = simplexml_load_string($content);

foreach ($xml->dailyrates[0]->currency as $currency) {
    if (trim($currency['rate']) == '-') {
        continue;
    }

    $foreignExchange = new ForeignExchange();

    $foreignExchange->setDate($xml->dailyrates[0]['id']);
    $foreignExchange->setCurrency($currency['desc']);
    $foreignExchange->setCurrencyCode($currency['code']);
    $foreignExchange->setRate($currency['rate']);

    $foreignExchange->create();
    $foreignExchange = null;
}
[/php

lib/data/MySQLiDAO.php
[code=php:0]
class MySQLiDAO {
    private $db_host;
    private $db_user;
    private $db_pass;
    private $db_name;
    
    private $link;
    
    private $table = null;
    
    function __construct() { 
        $this->db_host = 'HOST';
        $this->db_user = 'USR';
        $this->db_pass = 'PWD';
        $this->db_name = 'NAME';
        
        $this->link = new mysqli($this->db_host, $this->db_user, $this->db_pass, $this->db_name);
    }
    
    function insert($array) {
        $mysqli = $this->link;
        
        $columns = '';
        $values = '';
        
        $bindParams =  array();
        
        foreach ($array as $k => $v) {
            if (is_null($v)) {
                continue;
            }
            
            $bindParams[] = $v;
            
            if ($columns == '') {
                $columns = $k;
            } else {
                $columns = $columns . ', ' . $k;
            }
            
            if ($values == '') {
                $values = '?';
            } else {
                $values = $values . ', ?';
            }
        }
        
        $sql = 'INSERT INTO ' . $this->getTable() . ' (' . $columns . ') values (' . $values . ')';
        
        $stmt = $mysqli->prepare($sql);
        $this->bindVars($stmt, $bindParams);
        $stmt->execute();
        $stmt->close();
        
        $insertId = mysqli_insert_id($mysqli);
        $mysqli->close();
        
        return $insertId;
    }

    function bindVars($stmt,$params) {    
        if ($params != null) {        
            $types = '';                        //initial sting with types        
            
            foreach($params as $param) {        //for each element, determine type and add            
                if(is_int($param)) {                
                    $types .= 'i';              //integer            
                } elseif (is_float($param)) {                
                    $types .= 'd';              //double            
                } elseif (is_string($param)) {                
                    $types .= 's';              //string            
                } else {                
                    $types .= 'b';              //blob and unknown            
                }
            }         
            
            $bind_names[] = $types;             //first param needed is the type string                                            
                                                // eg:  'issss'         
            for ($i=0; $i<count($params);$i++) {//go through incoming params and added em to array            
                $bind_name = 'bind' . $i;       //give them an arbitrary name            
                $$bind_name = $params[$i];      //add the parameter to the variable variable            
                $bind_names[] = &$$bind_name;   //now associate the variable as an element in an array        
            }                                   //call the function bind_param with dynamic params        
            
            call_user_func_array(array($stmt,'bind_param'),$bind_names);
        }    
        
        return $stmt;                           //return the bound statement 
    }        

    function setTable($value) {
            $this->table = $value;
    }

    function getTable() {
            return $this->table;
    }
}

 

lib/logic/ForeignExchange.php

class ForeignExchange extends ForeignExchangeModel {
    private $db = null;
    
    function __construct() {
        parent::__construct();
        
        $this->db = new MySQLiDAO();
        $this->db->setTable('foreign_exchange');
    }
    
    function create() {
        $this->db->insert($this->obj2array());
    }
    
    function obj2array() {
        $array = array();
        
        $array['dato'] = $this->getDato();
        $array['valuta'] = $this->getCurrency();
        $array['valuta_code'] = $this->getCurrencyCode();
        $array['kurs'] = $this->getRate();
        
        return $array;
    }
}

 

lib/model/ForeignExchangeModel.php

class ForeignExchangeModel extends BaseModel {
    public $COLUMN;
    private $date;
    private $currency;
    private $currencyCode;
    private $rate;

    function __construct(){
        $this->COLUMN['date'] = 'dato';
        $this->COLUMN['currency'] = 'valuta';
        $this->COLUMN['currency_code'] = 'valuta_code';
        $this->COLUMN['rate'] = 'kurs';
    }
    
    function getDate() {
        return $this->date;
    }

    function setDate($value) {
            $this->date = $value;
    }

    function getCurrency() {
            return $this->currency;
    }

    function setCurrency($value) {
            $this->currency = $value;
    }
    
    function getCurrencyCode() {
            return $this->currencyCode;
    }

    function setCurrencyCode($value) {
            $this->currencyCode = $value;
    }
    
    function getRate() {
            return $this->rate;
    }

    function setRate($value) {
            $this->rate = $value;
    }
}

 

lib/model/BaseModel.php

class BaseModel {
    private $table = null;
    
    function setTable($value) {
        $this->table = $value;
    }
    
    function getTable($value) {
        return $this->table;
    }
}

18889_.zip

[MMDE]

What you need to do is to try figure out what is taking the longest time and try to fix that first.

I would start with loops and mysql queries.

 

To figure this out use microtime()

http://php.net/manual/en/function.microtime.php

 

$start = microtime(TRUE);
$start_line = (__LINE__ - 1);

 

then when you want to check how long it has taken to the point you are in the code:

echo 'line: ' . __LINE__ . ', time spent since line ' . $start_line . ': ' (microtime(TRUE)-$start);

 

I think you get the point.

Tell us what is going slow and then you can ask us how to improve it.

[Psycho]

I would start with loops and mysql queries.

 

Never, ever run queries in loops. It is one of the worst things you can do with regard to performance. In 80% of cases there is a way to accomplish the same thing without a loop, in another 19% of cases you have probably built your database schema incorrectly and in the remaining 1% of cases you are just doing something stupid. Of course, those percentages can change based upon the skillset of the programmer.

 

In your code above you have a foreach() that calls the create method, which in turn calls the insert method that does a DB insert.. You need to change it so it is generating a single query with ALL the records to be added. Then execute the query one time.

[MMDE]

I would start with loops and mysql queries.

 

Never, ever run queries in loops. It is one of the worst things you can do with regard to performance. In 80% of cases there is a way to accomplish the same thing without a loop, in another 19% of cases you have probably built your database schema incorrectly and in the remaining 1% of cases you are just doing something stupid. Of course, those percentages can change based upon the skillset of the programmer.

 

In your code above you have a foreach() that calls the create method, which in turn calls the insert method that does a DB insert.. You need to change it so it is generating a single query with ALL the records to be added. Then execute the query one time.

 

I meant, he should start looking into his various loops, and his various queries, to see where it is going the slowest. Figure out what is slowing down his script, as queries and loops, not just together, but in general, especially loops are the most likely reason for a slow-down. Not that he should start using looped queries, but maybe this is what he has done. ;p

[Psycho]

I meant, he should start looking into his various loops, and his various queries, to see where it is going the slowest. Figure out what is slowing down his script, as queries and loops, not just together, but in general, especially loops are the most likely reason for a slow-down. Not that he should start using looped queries, but maybe this is what he has done. ;p

 

I wasn't disagreeing with anything you said, I was agreeing with a specific emphasis on queries - and he is running queries in loops as I stated above. He is running through an xml file and on each record he is calling the create() method which in turn calls the insert() method (which does a DB insert).

 

I have to admit that I am not fully up to speed on the mysqli extension yet, but I'm pretty sure you can define a prepared statement and then run that prepared statement many times without the same performance issue as you would with running individual queries with mysql extension. What I also notice in the code is that for each line in the xml file the code is rebuilding the prepared statement. You should just be able to create the prepared statement on the first record and then just pass the new data to the prepared statement on each successive record. Rebuilding the prepared statement each time would destroy the benefit of creating a prepared statement in the first place.

 

So, create a private property for the class to be the prepared statement (set default to false) then int the insert method check if the prepared statement exists - if not, create it. Then it would only be created once.

 

I also see you are closing the db connection after each insert. I didn't really walk through all of the processes, but you should only open the DB connection when starting the processing of the XML file and close it when done.

[Christian F.]

In addition to what's been said already, the best bet you have is to get yourself a profiler. Only after profiling the code can you accurately tell what's slowing the code down, and figure out why. Without profiling the code all we can do is guess, and not very accurately either.

 

Personally I'm very fond of Zend Studio's profiler, especially in combination with Zend Server. Though, it will cost you a bit of money (?300 for Studio,  ?1,195+ for Server), so I recommend downloading the free trials first. You can also make do with the community version of Zend Server, if you don't need any of the more advanced features. Remote debugging and profiling with Zend Studio still works.