Performance optimization support

Excellent suggestion! But one that needs more discussion and vetting of the propossed optimizations.

The agreed upon optimizations should added to the Guidelines for better performaing code.

In regards to:

++$i is faster than $i++

Needs to be careful and check for leading white space and a trailing semi-colon because

  $foo = ++ $i; // is not the same as
  $foo = $i ++;

I didn't get quite the discussion here that I was hoping for, but I went ahead and implemented a basic review with some of the rules you mention here.

I'll do that if you can confirm that the rules are basically working. Thanks!

I just committed the bulk of these. Thanks! I think that we should add something visually appealing that lets the user know they can now click the icons. I think we need something more obvious than a mouse-over, which is why I didn't add it. Any ideas?

beware ctype_digit.. that's not supported across all PHP instances.

Do you have a source for true being faster than TRUE? That's interesting.

Drupal requires php 4.3, and the php docs say it's available since 4.0.4. What's the concern?

It was previously used in core during the 4.7.x cycle and got taken out... see: http://drupal.org/node/72865

If you search for ctype_digit, there are other users on 4.4+ who still don't have this function. The PHP docs appear to be misleading.

Regarding ctype_digit - this is best avoided (unless you truly need it!) because several distributions (BSD and Fedora IIRC) don't install the php ctype package by default. For people on shared hosts this is a royal PITA, because it can be difficult or impossible on many shared hosting providers (especially budget ones!) to get additional packages installed.

Regarding syntactical performance optimizations in general I think we need to be *very* careful about what we include here, since there are many blind allies and people who say "oh this should be faster because it..." without providing benchmarks. Actual timed results are often counterintuative and also have an annoying tendency to change between PHP, OS and compiler versions.

Also, there is a very real risk of turning readable code, which uses the kind of functions and constructs that make intuitive logical sense with harder-to-read (and harder to debug!) code that does things in unconventional or unintuitive ways and yet is of no or negligible performance benefit.

Of course if there are things that can be done that have a very significant (benchmarked!) performance benefit for code that is called very repeatedly in Drupal, or can be done without impacting the readability then this sounds good.

However, for the vast majority of cases the slowness normally comes from poorly constructed or architected code or DB structures - where fixing the (normally minor) code construct, or refactoring the code fixes the 'right' problem. Also, caching is often neglected, but can give big speed increases for the effort put in.

Some useful benchmark suites/results:

http://phplens.com/benchmark_suite/
http://www.php.lt/benchmark/phpbench.php

Sun, the page listing the items you quoted (http://ilia.ws/archives/12-PHP-Optimization-Tricks.html) only documents the first 5 items on the list - the rest are taken from comments, and none of them have any benchmarking data associated with them. While some of them are definately true, I would like to see some solid benchmarks before we make any changes that could potentially affect code readability.

* if (!isset($foo{5})) is faster than if (strlen($foo) < 5)
This is much less readable in my opinion, but perhaps could be used occasionally (with a code comment) in places where this is called lots and lots of times.

* ++$i is faster than $i++
I don't think I have ever seen ++$i used in practice, so this is a bit of a moot point :)

* echo is faster then print
This has been well known for a long time (e.g. http://elliottback.com/wp/archives/2006/10/19/php-performance-echo-print/) - I vote for switching to this (unless someone can think of a reason not to).

* ctype_digit($foo) is faster than preg_match("![0-9]+!", $foo)
Not a fan of this, for the reasons indicated above

* if (isset($keys['foo'])) is faster than if (in_array('foo', $keys))
Yup, this is also indicated by benchmarks (e.g. http://www.php.lt/benchmark/phpbench.php) and I think is a good one to go in. It's still pretty readable IMO.

* single quotes ('foo') are faster than double quotes ("foo")
Despite seeming intuative, I think benchmarks pretty much disprove this (see http://www.php.lt/benchmark/phpbench.php), the usage of ' vs. " should be chosen by what you need to do IMO.

* Perl-regexps are faster than POSIX-regexps
Yes, this is well known - although somewhat moot as I don't think I have ever seem POSIX regexps in use!

* true is faster than TRUE
I think this is a pretty ingrained habit, that will be very difficult to get people to change from without benchmarks detailing a noticable improvement.

* do-while structure is faster than for structure (source)
While this may be the case, but I feel very strongly here that code is much clearer if one picks the most 'natural' construct here for the particular use in question, rather than twising the code to fit one or the other. There are workarounds too, of course - for example http://www.php.lt/benchmark/phpbench.php suggests that for is several times quicker if you precompute your final value.

In other words, all this is pretty much a 'mixed bag' and I think each item should be evaluated (on an evidence basis) on a per-item and a per-use level before we start making coder module recomendations and especially before we start changing Drupal coding standards :)

The main page says to add comments here, so even if this is not a performance issue, I'll raise it here:

1. THANKS! What an amazing tool, which will even further build Drupal's reputation for clean coding.

2. My issue is that this code:

$caption = t("All assertions on site.");

Yields two warnings:

Line 2061: string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms

Line 2061: do not use mixed case (camelCase), use lower case and _

But surely coding standards don't apply inside t() strings (and if standards apply, they are not the same as coding standards). Maybe, for now, strings inside t() could just be exempted from the checks?

how could true be faster than TRUE when PHP is case agnostic with its identifiers...?

There was a flurry of discussion about this on the development list, to which I replied here.

Given that discussion, I removed the TRUE/true rule. Then upon further review, I didn't see a single performance review that seemed agreeable. I had already removed the performance review in 6.x, so I removed it in 5.x also.

I think that a performance review would be a very good thing. But since coder is now more widely used, I won't be adding rules such as these without more widespread discussion and support. Let's discuss this at DrupalCon in Barcelona.

@sun, thanks for starting this thread. I wish that it had gotten further along :(

Can we check some basic logique optimisation like that:

//bad 
for ($i=0; $i<count($array); $i++)
{
  echo $array[$i];
}

//good
$count = count($array);
for ($i=0; $i<$count; $i++)
{
  echo $array[$i];
}