2
\$\begingroup\$

I've developed a log reader which can filter it's output following a few criteria: errorlevel, product, category, id. It is functioning, but when the log file is really big, it becomes very slow, to the point of going beyond the time limit.

log_reader.php

<?php
    session_start();
    unset($_SESSION['errorlevel']);
    unset($_SESSION['category']);
    unset($_SESSION['product']);
    unset($_SESSION['id']);
    $lines = file("error.log");
    $categories = array();
    $products = array();
    $ids = array();
    foreach($lines as $line){
        $segments = explode("][",substr($line,1,strrpos($line,']')-1));
        if((!in_array($segments[2],$categories))&&(!empty($segments[2]))){
            $categories[] = $segments[2];
        }
        if((!in_array($segments[3],$products))&&(!empty($segments[3]))){
            $products[] = $segments[3];
        }
        if((!in_array($segments[4],$ids))&&(!empty($segments[4]))){
            $ids[] = $segments[4];
        }
    }
    sort($categories);
    sort($products);
    sort($ids);
?>

<html>
  <head>
    <title>Log Reader</title>
    <link rel="stylesheet" type="text/css" href="style.css" />
    <script type="text/javascript">
      function showHide(selection){
          var name = selection.name;
          var opt = selection.options[selection.selectedIndex].value;
          if (window.XMLHttpRequest){ 
              //code for IE7+, Firefox, Chrome, Opera, Safari 
              xmlhttp=new XMLHttpRequest(); 
          }else{ 
              // code for IE6, IE5 
              xmlhttp=new ActiveXObject('Microsoft.XMLHTTP'); 
          }
          xmlhttp.onreadystatechange=function(){ 
              if (xmlhttp.readyState==4 && xmlhttp.status==200){

                  document.getElementById('logterminal').innerHTML=xmlhttp.responseText; 
              } 
          }
          xmlhttp.open('GET', 'filteredlog.php?'+name+'='+opt, true); 
          xmlhttp.send();
          return false; 
      }
    </script>
  </head>
  <body>
    <div style="background-color:#fffacd;">
      <form>
        <select name="errorlevel" onchange="return showHide(this);">
          <option value="DEBUG">DEBUG</option>
          <option value="INFO" selected="selected">INFO</option>
          <option value="WARN">WARN</option>
          <option value="ERROR">ERROR</option>
        </select>
        <select name="category" onchange="return showHide(this);">
          <option value="All">All Categories</option>
          <?php
            foreach($categories as $category){
                echo "<option value='".$category."'>".$category."</option>";
            }
          ?>
        </select>
        <select name="product" onchange="return showHide(this);">
          <option value="All">All Products</option>
          <?php
            foreach($products as $product){
                echo "<option value='".$product."'>".$product."</option>";
            }
          ?>
        </select>
        <select name="id" onchange="return showHide(this);">
          <option value="All">All IDs</option>
          <?php
            foreach($ids as $id){
                echo "<option value='".$id."'>".$id."</option>";
            }
          ?>
        </select>
      </form>
    </div>
    <div id="logterminal">
    <?php
        include("filteredlog.php");
    ?>
    </div>
  </body>
</html>

filteredlog.php

<?php
if(count(debug_backtrace())==0){;
    session_start();
}
$errorlevel = $_GET['errorlevel'];
$errorlevels = array("DEBUG", "INFO", "WARN", "ERROR");
if(empty($errorlevel)){
    if(isset($_SESSION['errorlevel'])){
        $errorlevel = $_SESSION['errorlevel'];
    }else{
        $errorlevel = "INFO";
    }
}else{
    $_SESSION['errorlevel']=$errorlevel;
}

$category = $_GET['category'];
if(empty($category)){
    if(isset($_SESSION['category'])){
        $category = $_SESSION['category'];
    }else{
        $category = "All";
    }
}else{
    $_SESSION['category']=$category;
}

$product = $_GET['product'];
if(empty($product)){
    if(isset($_SESSION['product'])){
        $product = $_SESSION['product'];
    }else{
        $product = "All";
    }
}else{
    $_SESSION['product']=$product;
}

$id = $_GET['id'];
if(empty($id)){
    if(isset($_SESSION['id'])){
        $id = $_SESSION['id'];
    }else{
        $id = "All";
    }
}else{
    $_SESSION['id']=$id;
}

$lines = file("error.log");
$categories = array();
$products = array();
$ids = array();
foreach($lines as $line){
    $segments = explode("][",substr($line,1,strrpos($line,']')-1));
    if((!in_array($segments[2],$categories))&&(!empty($segments[2]))){
        $categories[] = $segments[2];
    }
    if((!in_array($segments[3],$products))&&(!empty($segments[3]))){
        $products[] = $segments[3];
    }
    if((!in_array($segments[4],$ids))&&(!empty($segments[4]))){
        $ids[] = $segments[4];
    }
}

echo $errorlevel . $category . $product . $id . "<br /><br />";


$currentlvl = "";
$filterlines = array();
foreach($lines as $line){
    $segments = explode("][",substr($line,1,strrpos($line,']')-1));
    $line_errlvl = $segments[0];
    if(!in_array($line_errlvl,$errorlevels)){
        $line_errlvl = $currentlvl;
    }
    switch($errorlevel){
        case "DEBUG":
            if($line_errlvl=="DEBUG"){
                $filterlines[] = $line;
            }
        case "INFO":
            if($line_errlvl=="INFO"){
                $filterlines[] = $line;
            }
        case "WARN":
            if($line_errlvl=="WARN"){
                $filterlines[] = $line;

            }
        case "ERROR":
            if($line_errlvl=="ERROR"){
                $filterlines[] = $line;
            }
        default:
            $currentlvl = $line_errlvl;
            break;
    }
}
if($product != "All"){
    $lines = $filterlines;
    $currentcat = "";
    $filterlines = array();
    foreach($lines as $line){
        $segments = explode("][",substr($line,1,strrpos($line,']')-1));
        $line_cat = $segments[2];
        if(!in_array($line_cat,$categories)){
            $line_cat = $currentcat;
        }
        if($line_cat==$category){
           $filterlines[] = $line;
        }
        $currentcat = $line_cat;
    }
}
if($product != "All"){
    $lines = $filterlines;
    $currentprod = "";
    $filterlines = array();
    foreach($lines as $line){
        $segments = explode("][",substr($line,1,strrpos($line,']')-1));
        $line_prod = $segments[3];
        if(!in_array($line_prod,$releases)){
            $line_prod = $currentprod;
        }
        if($line_prod==$product){
           $filterlines[] = $line;
        }
        $currentprod = $line_prod;
    }
}
if($id != "All"){
    $lines = $filterlines;
    $currentid = "";
    $filterlines = array();
    foreach($lines as $line){
        $segments = explode("][",substr($line,1,strrpos($line,']')-1));
        $line_id = $segments[4];
        if(!in_array($line_id,$ids)){
            $line_id = $currentid;
        }
        if($line_id==$id){
           $filterlines[] = $line;
        }
        $currentid = $line_id;
    }
}

foreach($filterlines as $line){
    echo $line . "<br />";
}

?>
\$\endgroup\$
2
\$\begingroup\$

I wouldn't say this would affect your performance much, but usually if((!in_array($segments[4],$ids))&&(!empty($segments[4]))) Is written as if(!empty($segments[4])&&!in_array($segments[4],$ids)) so as to short out if the item is empty.

\$\endgroup\$
0
\$\begingroup\$

Well, if you have product, ID, and category set to "ALL" (or equivalent), then your script calls:

$segments = explode("][",substr($line,1,strrpos($line,']')-1));

ON EVERY LINE OF THE FILE, FIVE TIMES.

Consider, instead:

$segs = array();
foreach($lines as $line){
    $segments = explode("][",substr($line,1,strrpos($line,']')-1));
    $segs[] = $segments;
    // then continue modifying/filtering the processed data instead of the strings

This block creates a series of variables which are not used anywhere else. I recommend chopping it:

$categories = array();
$products = array();
$ids = array();
foreach($lines as $line){
    $segments = explode("][",substr($line,1,strrpos($line,']')-1));
    if((!in_array($segments[2],$categories))&&(!empty($segments[2]))){
        $categories[] = $segments[2];
    }
    if((!in_array($segments[3],$products))&&(!empty($segments[3]))){
        $products[] = $segments[3];
    }
    if((!in_array($segments[4],$ids))&&(!empty($segments[4]))){
        $ids[] = $segments[4];
    }
}

As a general rule, test to see if the array index exists before using it too, but once you've gotten rid of that block, you won't have to worry about that as that type of array look-up only happens there.

On a more subtle point:

// this was foreach($lines as $line){, but I am assuming the fix suggested earlier.
foreach($segs as $segments){
    $line_errlvl = $segments[0];
    if(!in_array($line_errlvl,$errorlevels)){
        $line_errlvl = $currentlvl;
    }
    switch($errorlevel){
        case "DEBUG":
            if($line_errlvl=="DEBUG"){
                $filterlines[] = $line;
            }//...

So, in_array performs a basic search of the array and returns true or false. If, however, we change this to array_search we could use the < operator. Consider:

$numericLevel = array_search($errorlevel, $errorlevels);
$currentlvl   = 0;
$filterlines = array();
foreach($segs as $idx => $segments){
    // in_array basically does the same thing (on the machine level), 
    // only it is less useful here.
    $line_errlvl = array_search($segments[0], $errorlevels);
    if(FALSE === $line_errlvl) $line_errlvl = $currentlvl;
    if($line_errlvl >= $numericLevel) $filterlines[] = $lines[$idx];
    $currentlvl = $line_errlvl;
}

That will both future-proof your code (you can add in a log level "Skunk" later, and it will still work), and it also makes it more efficient -- if you were set to "DEBUG" and ran the original:

switch($errorlevel){
        case "DEBUG":

You would end up comparing a DEBUG line to INFO, WARN, and ERROR. This makes exactly one comparison.


Personally, I also feel that this is not somewhere you want to be using sessions. This should just be a series of reads, it does not really make sense to store it.

\$\endgroup\$
  • \$\begingroup\$ I see only one problem: if you replace foreach($lines as $line) by foreach($segs as $segments){, how do you get the line in the expression if($line_errlvl >= $numericLevel) $filterlines[] = $line;? \$\endgroup\$ – Eldros Sep 27 '11 at 7:39
  • \$\begingroup\$ @Eldros Modified to reference numeric index of original array \$\endgroup\$ – cwallenpoole Sep 27 '11 at 12:38
  • \$\begingroup\$ Ok, that's already one problem down. Now, choping the session code means that only the changed variable would be sent to filteredlog.php and all other variable would take their default value in the best of the case. What would you advice in this case? \$\endgroup\$ – Eldros Sep 27 '11 at 14:01
  • \$\begingroup\$ @Eldros make everything work through GET and send all values -- since there are only four possible GET variables it will not add real overhead to either the server-side code or the client side code and it means that you don't need to store extra info on the SESSION (or even call session_start). \$\endgroup\$ – cwallenpoole Sep 27 '11 at 17:33
  • \$\begingroup\$ So, I went through your changes, and it react quite quickly when the only filter specified is the errorlevel, but as soon as I choose a category, a product and/or and id, the performance drop abruptly. \$\endgroup\$ – Eldros Sep 28 '11 at 8:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.