Re: cvs: php-benchmarks / bench.php
| From: | Paul Biggar | Date: | Sun, 24 May 2009 18:11:55 +0000 |
| Subject: | Re: cvs: php-benchmarks / bench.php | ||
| References: | 1 | Groups: | php.qa |
| Request: | Send a blank email to php-qa+get-65003@lists.php.net to get a copy of this message | ||
Hi Alexander,
On Sun, May 24, 2009 at 6:45 PM, Alexander Hjalmarsson <hjalle@php.net> wrote:
> xұFM??
> %http://cvs.php.net/viewvc.cgi/php-benchmarks/bench.php?view=markup&rev=1.1
> Index: php-benchmarks/bench.php
> +++ php-benchmarks/bench.php
> * @param $log File to log the output to.
> * Set to false if logging is
> * disabled.
$log is not a param here.
> function console($msg,$print = true)
> {
> if(VERBOSE == true)
> {
> if($print == true)
No need for $x == true, just use if ($x).
> if(LOG_FILE != false && $print)
I don't see where LOG_FILE is defined as a constant. Presumably this
is meant to use 'global $LOG_FILE'?
> {
> $fp = fopen(LOG_FILE,'a');
> fwrite($fp,$msg);
> fclose($fp);
> }
Was there a reason not to use file_put_contents?
> function get_test_files($test_dirs)
> {
> $files = array();
> foreach($test_dirs as $dir)
> {
> foreach (glob("$dir/test_*.php") as $filename) {
> $t['filename'] = $filename;
>
> preg_match('/^'.preg_quote($dir,'/').'\/test_(.+)[.]php$/i',
> $filename, $matches);
> $t['name'] = $matches[1];
> $files[] = $t;
Please document the purpose of the regex.
> * Returning array have this structure:
> * array("/path/to/test/test_name.php)" => "name.php",...,);
> * @param $test_dirs An array of directories to search in
> * @return Array of test files and their names.
This could be clearer.
> function arguments($argv,$flags,$arguments) {
> $ARGS = array();
> foreach ($argv as $arg) {
> if (ereg('--[a-zA-Z0-9]*=.*',$arg)) {
> $str = split("=",$arg); $arg = '';
> $key = ereg_replace("--",'',$str[0]);
> for ( $i = 1; $i < count($str); $i++ ) {
> $arg .= $str[$i];
> }
I think there are a couple of idiomatic ways of parsing command line
parameters, such as Console_getargs from PEAR
(http://pear.php.net/package/Console_Getargs). However, since this is
written, and presumably works, I'd be satisfied with a comment to that
effect in the short term.
> foreach($tests_list as $test)
> {
> $cmd = "$PHP -d memory_limit={$MEMORY_LIMIT}M
> ".$test['filename'];
> console("$cmd\n",DEBUG);
> $timer->start();
> $deb =
$cmd;
$deb is $debug?
I don't think `` catches stderr.
Other than those minor nits, this is excellent code, with nice
documentation. Keep up the good work.
Paul
--
Paul Biggar
paul.biggar@gmail.com