Klasa do ładowania plików, duże obciążenie... Opcje

[cojack]

No więc tak, mam sobie klasę do ładowania plików, i gdy odpaliłem profilera php to zobaczyłem że dość duże obciążenie mam na tej klasie, bo z wszystkich wywołań metod wszystkie wywołania a jest ich 33 z tego co dobrze widzę, klasyfikują na 2 miejscu w stopniu najbardziej obciążającym system, no i szczerzę powiedziawszy to nie mam bladego pojęcia co jest tak mega super złego w tej klasie że takie obciążenie powoduje, oto kod:

[PHP] pobierz, plaintext 
<?php
/**
 * BDT_Loader klasa odpowiedzialna za ładowanie plików
 *
 * @author     Przemysław Czekaj <przemyslaw.czekaj@aichra.pl>
 * @link       http://aichra.pl
 * @version    0.1
 * @since      03.29.2010
 * @package    BDT
 * @subpackage lib
 * @charset    utf8
 **/
 
/**
 * @author  cojack
 * @TODO    Błędy w postaci GETTEXT
 */
class BDT_Loader {
 
   private static $_path;
 
   /**
    * Konstruktor klasy
    *
    * @return  void
    */
   public static function initialize() {
      chdir( './..' );
      self::$_path = getcwd() . '/';
   }
 
   /**
    * Metoda przyjmuje argument jako tablice iteracyjną
    * Adres do pliku powinien zaczynać się od ./ i uwzględnić
    * zmianę ścieżki podczas wczytywania danych.
    * Zachowywujemy się tak jakbyśmy byli w / tego fw.
    *
    * @access  public
    * @param   array    $files   Tablica w postaci array ('plik', 'plik', 'plik');
    * @return  void
    */
   public static function loadFile( $files = array() ) {
      foreach( $files as $file ) {
         if( self::_checkFile( $file ) ) {
               require_once( self::$_path.$file );
         }
      }
   }
 
   /**
    * Metoda wczytuje pojedyncze pliki XML
    *
    * @access  public
    * @param   string    $files   Względna ścieżka do pliku
    * @return  object    Xml object
    */
   public function loadFileXML( $file ) {
      if( self::_checkFile( $file ) ) {
         libxml_use_internal_errors( TRUE );
         $sxe = new SimpleXMLElement( self::$_path.$file, NULL, TRUE );
         if( $sxe === FALSE ) {
            $errorMsg  = 'Błąd w pliku XML: ' . $file . "<br />";
            $errorMsg .= 'Ścieżka: ' . self::$_path.$file . "<br />";
            foreach( libxml_get_errors() as $error ) {
               $errorMsg .=  "<p>" . $error->message . "</p><br />";
            }
            trigger_error( $errorMsg , E_USER_WARNING );
         }
      }
      return $sxe;
   }
 
   /**
    * Metoda sprawdza czy dany plik jest możliwy do wczytania
    *
    * @param   string   $file    Względna ścieżka do pliku
    * @return  boolean  true, false
    */
   private static function _checkFile( $file ) {
      if( is_file( self::$_path.$file ) ) {
         if( is_readable( self::$_path.$file ) ) {
            return TRUE;
         } else {
            trigger_error( 'Plik: '. $file . ' jest nie dla odczytu, sprawdź uprawnienia.' , E_USER_WARNING );
         }
      } else {
         trigger_error( 'Zła ścieżka i/lub nazwa dla pliku: '. $file . "\n" . 'Ścieżka: ' . self::$_path . $file , E_USER_WARNING );
      }
      return FALSE;
   }
}
[PHP] pobierz, plaintext 

Gdyby ktoś mógł mi wskazać jakieś wskazówki by poprawić wydajność tej klasy, będę wdzięczny, pomysły z autoload zachować dla siebie, od razu mówię nie, nie i jeszcze raz nie, także bez nich.

Ten post edytował cojack 23.08.2010, 20:18:19

[wookieb]

1) Nie latwiej zdefiniować nawet stałej która przechowuje ci getcwd() i dawać ją w potrzebnych miejscach? (że nie wspomnę o używaniu include_path)

[PHP] pobierz, plaintext 
require_once ACTUAL_CWD.'plik.php';
[PHP] pobierz, plaintext 

Bo to co teraz masz nie ma sensu
2) Nie rozumiem, dlaczego pakujesz tutaj ładowanie xml
3) Korzystamy z wyjątków a nie trigger_errorów.
4) Klasa loadera wcale nie jest tak wolna. Czasem narzut pracy wynika po prostu z konieczności sparsowania , wykonania załadowanych plików przez php.
5)

Kod

   * @return  boolean  true, false

Facepalm :/
6)

Kod

* @return  object    Xml object

Typujemy silnie. Czyli nie "object" ale "SimpleXMLElement". Ma to ogromne znaczenie dla wszelkiego rodzaju IDE.
7) Nadal nie widzę sensu korzystania z metody _checkFile. Szczerze mówiąc przyda się tylko w środowisku developerskim gdzie sam na podstawie wywalonego przez php błędu będziesz wiedział co zrobić.

Ten post edytował wookieb 23.08.2010, 20:30:47

[cojack]

ad 1) nie używam stałych (DEFINE) w całym FW, taka filozofia
ad 2) bo to jest klasa do ładowania plików?
ad 3) odp jest w ad 6.
ad 4) Może i w tym właśnie tkwi problem, że musi php sparsować pozostałe pliki, przyjże się wywołaniom lepiej i spróbuję to jakoś odgadnąć.
ad 5)
ad 6) Bez sensu byłoby stosowanie tutaj wyjątków, ponieważ kod byłby tak naszpikowany try {} catch {} że to się mija z celem, a błędy i tak gromadzę i pakuje w jedno miejsce, i tak czy siak używam trigger_error. A poza tym używam tej klasy też poza ciałem klas, także wyjątki w tym miejscu nie mają prawa bytu, gdyż ładuje w ten sposób klasy z których dana kasa dziedziczy, no i pomyśl sobie jakby to wyglądało.
ad 7) Z tym się zgodzę, będę miał to na uwadze, na pewno wprowadzę opcje sprawdzania typu środowiska przed wywołaniem _checkFile.

Ten post edytował cojack 23.08.2010, 20:41:52

[Pilsener]

A ta klasa to ma być upload, parsowanie, jakiś model danych, czemu ma to służyć? Może efekt ma być diametralnie inny i nie ma sensu analizować kodu tej klasy, nie wiem na przykład, czy problem dotyczy includowania zwykłych plików czy xml? Jeśli to ma być jakiś autoloader to może warto przyjrzeć się, jak rozwiązano to w popularnych frameworkach?

[erix]

require_once jest wolniejsze od require, z tego co pamiętam.

Poza tym, dlaczego dokonujesz tylu sprawdzań na poziomie systemu plików? Samo is_readable wystarczy, po co is_file?

ad 1) nie używam stałych (DEFINE) w całym FW, taka filozofia

Hmm, filozofia w stylu kobiecej bo tak!, czy jakaś konkretna argumentacja? Nie wyobrażam sobie większego systemu bez choćby jednej stałej określającej katalog roboczy aplikacji.

ad 2) bo to jest klasa do ładowania plików?

No niech Ci będzie, ale też tego nie rozumiem.

ad 6) Bez sensu byłoby stosowanie tutaj wyjątków, ponieważ kod byłby tak naszpikowany try {} catch {} że to się mija z celem, a błędy i tak gromadzę i pakuje w jedno miejsce, i tak czy siak używam trigger_error. A poza tym używam tej klasy też poza ciałem klas, także wyjątki w tym miejscu nie mają prawa bytu, gdyż ładuje w ten sposób klasy z których dana kasa dziedziczy, no i pomyśl sobie jakby to wyglądało.

Takie myślenie jest bez sensu. Są wyjątki, zwiększają czytelność kodu i usprawniają wiele rzeczy, np:

[PHP] pobierz, plaintext 
 private static function _checkFile( $file ) {
      if( is_file( self::$_path.$file ) ) {
         if( is_readable( self::$_path.$file ) ) {
            return TRUE;
         } else {
            trigger_error( 'Plik: '. $file . ' jest nie dla odczytu, sprawdź uprawnienia.' , E_USER_WARNING );
         }
      } else {
         trigger_error( 'Zła ścieżka i/lub nazwa dla pliku: '. $file . "\n" . 'Ścieżka: ' . self::$_path . $file , E_USER_WARNING );
      }
      return FALSE;
   }
[PHP] pobierz, plaintext 

Na co tu tyle? Zrób sobie error_handler, który pluje wyjątkiem przy E_WARNING. PHP nie jest wielowątkowe, więc przy samym require_once wypluje wyjątek w dokładnie tym miejscu, co trzeba, a będziesz miał centralne zarządzanie odpowiednimi zdarzeniami.

Poza tym, żeby require_once wypluło błąd, to też musi sprawdzić, jaki on jest, czyli też sprawdza, czy plik da się odczytać, czy są do niego uprawnienia. Więc wywołanie is_readable jest - de facto - ponownym wykonaniem czegoś, co miało miejsce. Więc po co?

[cojack]

No przeta mam error_handler ;] Ok, to już wiem co należy poprawić, dziękuwa erix, btw w calym FW nie ma ani jednej stałej zdefiniowanej przez DEFINE.

require_once jest wolniejsze od require, z tego co pamiętam. winksmiley.jpg

może i jest wolniejsze, ale ja nie potrzebuje mieć 2 razy tego samego kodu w aplikacji.

[erix]

No przeta mam error_handler ;]

Ale on nie wypluwa wyjątków i wszędzie musisz wykonywać 2x to samo.

może i jest wolniejsze, ale ja nie potrzebuje mieć 2 razy tego samego kodu w aplikacji.

A o cache'u, to słyszał?

Podpowiedź: static działa nie tylko w klasach.

[wookieb]

Sorry erix ale mam NIE używać require_once bo require jest o 1 setną sekundą szybszy przy iluś tam wywołaniach i mam się męczyć o to, żeby kontrolować czy czasem nie dołączam tego samego pliku? No sorry.

[erix]

żeby kontrolować czy czasem nie dołączam tego samego pliku?

Hmm, w swoich libach jestem świadomy, czy dołączyłem już, czy nie, ale to tak nawiasem.

I skoro już jesteśmy przy temacie:

• autoloader ładuje w razie potrzeby
• w funkcji ładującej możesz zrobić cache w zmiennej statycznej i sprawdzać via in_array. IMO będzie szybsze niż odwoływanie się do funkcji I/O

[wookieb]

• w funkcji ładującej możesz zrobić cache w zmiennej statycznej i sprawdzać via in_array. IMO będzie szybsze niż odwoływanie się do funkcji I/O

Wiem, że w php-ie można spodziewać się różnych krzaków ale no sorry erix :/

[PHP] pobierz, plaintext 
 
class Loader
{
    private static $loaded = array();
 
    public static function load($file)
    {
        if (!in_array($file, self::$loaded))
        {
            require $file;
            self::$loaded[] = $file;
        }
    }
}
 
$start = microtime(true);
for ($i=0; $i<1000; $i++)
{
    require_once 'test2.php';
}
echo microtime(true) - $start;
echo '<br/>';
 
$start = microtime(true);
for ($i=0; $i<1000; $i++)
{
    Loader::load('test2.php');
}
echo microtime(true) - $start;
 
[PHP] pobierz, plaintext 

Wynik

Kod

test0.0037469863891602
test0.050887823104858

Gdzie test jest zawartością pliku test2.php

Oczywiście, że require_once jest wolniejsze od require ale tylko przy pierwszym wywołaniu. I teraz pytanie. Czy warto się męczyć o zysk 1 setnej sekundy? I to w dodatku z ryzykiem wielu błędów (jeżeli nie korzystamy z autoloadera)? Nie sądzę.

Napomnę jeszczę taką kwestię iż nie zawsze korzystanie z funkcji autoloadera będzie ok.
Czasem w nagłówkach klas z łapy podaje się require_once i ścieżkę do pliku, żeby nie szukał jej autoloader.

Prosiłbym jeszcze kogoś o przetestowanie powyższego kodu z jakimś akceleratorem. Ponieważ APC mi nie działa na windowsie :/

Ten post edytował wookieb 24.08.2010, 10:56:04