Reduce PHP memory usage / No unnecessary concatenation

[jrf]

While these code changes might just seem a trivial cosmetic / code style change to some, in reality they aren't.

Concatenation has a large impact on memory usage in PHP. The echo language construct can echo several things without concatenation by using commas.
Replacing all concatenations within echo statements in favour of commas reduces the memory used by PHP.
I haven't bench marked the effect yet [if someone's got a setup designed for that - please feel invited!].

But consider this: Even if the reduction is just ~0.5%, 0.5% less memory use for 70 million sites world-wide has an impact.

Some notes:
I won't guarantee that I've caught every occurrence. This patch was created using regex searches on the code base for certain patterns such as ^\s*echo .+?\. and then manually reviewing and adjusting the found code snippets.

Also:

• For the lines I've touched, I've made sure code style complies to the standard.
• In some cases several echo statements where used when one would be sufficient. Merged a number of those (another best practice).
• Some unnecessary double function calls, i.e. echo sprintf()'s replaced by printf().
• For the lines I've touched, I've done a sanity check for single quotes vs double quotes, especially if the resulting HTML would have single quoted attributes and adjusted accordingly.

Code review appreciated.

Specific review request:
The below files each contained an output string without localization function. Was there a reason for that ? I've wrapped them in a __() now.

• wp-includes/locale.php
• wp-includes/wp-db.php
• wp-admin/includes/class-wp-importer.php

For more information, see:
​https://github.com/dseguy/clearPHP/blob/master/rules/no-unnecessary-string-concatenation.md
​https://github.com/dseguy/clearPHP/blob/master/rules/no-repeated-print.md

[jrf]

Patch file containing all the changes

[jrf]

Oh and FYI: I have run the unit tests on the changes and found no regressions.

[pento]

Well, that's a giant patch. :-)

phpunit used slightly more memory (+0.25MB) and was a couple of seconds faster when run with this patch (this is a wildly unscientific comparison!).

I'd also be interested to see the results of a different test that could show memory usage under a more usual WordPress usage.

[johnbillion]

This is a strange change to propose without having actually done any performance tests. I would be quite surprised if this results in anything more than a negligible real-world reduction in memory usage, especially anywhere near 0.5%. It would be great if I'm wrong, of course!

The below files each contained an output string without localization function. Was there a reason for that ? I've wrapped them in a () now.

The error message in wp-db.php can occur before the localisation functions are available, which is why it's not wrapped in a call to __(). The other two look like they are indeed missed strings though. Can you open a separate ticket for those please?

For more information, see:
​https://github.com/dseguy/clearPHP/blob/master/rules/no-unnecessary-string-concatenation.md
​https://github.com/dseguy/clearPHP/blob/master/rules/no-repeated-print.md

Those two "rules" conflict with each other. Which is it to be?

[GaryJ]

Replying to johnbillion:

For more information, see:
​https://github.com/dseguy/clearPHP/blob/master/rules/no-unnecessary-string-concatenation.md
​https://github.com/dseguy/clearPHP/blob/master/rules/no-repeated-print.md

Those two "rules" conflict with each other. Which is it to be?

I don't see them as conflicting. The first says not do:

echo $a . $b . $c;

and the second says not to do:

echo $a;
echo $b;
echo $c;

The patch here advocates:

echo $a, $b, $c;

---

A very quick benchmark, based on code from ​http://www.electrictoolbox.com/php-echo-commas-vs-concatenation/
suggests that commas (​http://3v4l.org/48KHh/perf#tabs) are generally better performing than concat (​http://3v4l.org/alrdD/perf#tabs). The following tests / links also agree.

• ​https://www.fusionswift.com/2010/05/php-concatenation-benchmark-comma-vs-period/
• ​https://phpgoodness.wordpress.com/2010/07/23/some-php-performance-myths/
• ​http://www.spudsdesign.com/index.php?t=echo1

These are obviously no replacement for a real test of WP, but should be enough to consider doing that benchmark.

[dd32]

Using comma's should be faster, simply due to the fact that PHP doesn't need to merge the strings in memory before it uses it, technically it should be able to just add them straight to the output buffer. In reality I think it's an extreme micro-optimization that benefits WordPress very little. It might be able to shave 0.01ms off a page load if someone was lucky.. they'll get a better speed increase by upgrading their PHP version.

I also really question how much impact this change will have in PHP 7, simply due to the sheer amount of optimizations that have been done over time. The articles above are from 2007/2010, and it was a tiny optimization back then (they used 1million/4million iterations to get a ~140ms speed increase)

Should also be mentioned, using 3v4l for benchmarks should also be avoided, not only is it abusive, but the performance characteristics are not guaranteed - some of the PHP 5.3 runs matched the PHP7 runs in speed.

Finally.. the concatenation operator is also a far more well known and obvious operator to many, and with the miniscule speed & memory difference it makes it doesn't seem like a micro-optimization we should make IMHO.

[jorbin]

I agree with Dion that this is at best a microoptimization. I don't think it is worth the code churn. Even the benchmarks above show that this isn't even garunteed to improve performance across all PHP versions we support (the usertime for HHVM, 5.2, 5.3, and 7.0 is higher not using concatenation ). I think this is a case where practicality trumps theoretical correctness.

Suggest closing as wontfix.

[jrf]

There seems to be some confusion: this PR was not about performance in the sense of *speed*, but about performance in the sense of *memory use*.

Every concatenation will exponentially usurp memory, especially when used in an echo statement where there is a perfectly good alternative or rather, a much advocated best practice, as even the PHP manual will tell you.

While I realize that people who are not so well versed in PHP may not be as familiar with using comma's in echo statements, that doesn't take away from the original premise.

And while there have been lots of improvements in PHP7, this has not changed.

Anyways, as I said in the first line 'was'. I bow to your wisdom.

[GaryJ]

I'd still like to see a benchmark on WP done.

If favourable, instead of a big patch like this, could the approach be to advocate the use of commas in the coding standards, and switch over to them when code is added / updated, as per other coding standards fixes? I realise this isn't a "coding standards" thing per se, but that approach avoids the churn.