Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Saving some memory for echo. #403

Closed
wants to merge 1 commit into from
Closed

Conversation

dseguy
Copy link

@dseguy dseguy commented May 19, 2015

No description provided.

@nicktacular
Copy link
Contributor

@dseguy What problem are you solving here?

@serbanghita
Copy link
Owner

Wanted to ask the same question.

@SiM07
Copy link

SiM07 commented May 19, 2015

@serbanghita
Copy link
Owner

@SiM07 thanks for pointing to that resource. I think both me and Nick are a bit confused by this because export/exportToJSON.php is meant to be use only once per release/update and not millions of times per second :)

@nicktacular
Copy link
Contributor

Interesting. My interest is piqued. However, the article is from 2007 and PHP has change a lot since then. Here's a little test:

<?php

// use error log so we can eliminate the other echos to STDOUT
error_log(sprintf("echo test for %s\n", phpversion()));

$iterations = 1000000;
$stats = [];

// concat test
$start = microtime(true);
for ($i = 0; $i < $iterations; $i++) {
    echo "this" . "is" . "the" . "real" . "deal" . PHP_EOL;
}
$elapsed = microtime(true) - $start;
$stats['concat'] = $elapsed;

// comma test
$start = microtime(true);
for ($i = 0; $i < $iterations; $i++) {
    echo "this" , "is" , "the" , "real" , "deal" , PHP_EOL;
}
$elapsed = microtime(true) - $start;
$stats['comma'] = $elapsed;

error_log(sprintf(
    "Concat timing: %s\nComma timing: %s\nIterations: %s\n",
    number_format($stats['concat'], 8),
    number_format($stats['comma'], 8),
    $iterations
));
[zero]~→ php comma.php > /dev/null
echo test for 5.5.20

Concat timing: 0.85571814
Comma timing: 4.35616016
Iterations: 1000000

[zero]~→ php comma.php > /dev/null
echo test for 5.5.20

Concat timing: 0.85867500
Comma timing: 3.72889185
Iterations: 1000000

[zero]~→ php comma.php > /dev/null
echo test for 5.5.20

Concat timing: 0.85669303
Comma timing: 3.75335598
Iterations: 1000000

[zero]~→ php comma.php > /dev/null
echo test for 5.5.20

Concat timing: 0.85059500
Comma timing: 3.89159322
Iterations: 1000000

[zero]~→ php comma.php > /dev/null
echo test for 5.5.20

Concat timing: 0.85772896
Comma timing: 3.90778303
Iterations: 1000000

[zero]~→ 

Several iterations of this show that comma is significantly slower to string concatenation.

Always test your optimizations before you optimize 😄

@serbanghita
Copy link
Owner

@nicktacular omg 🎯

@robhadfield
Copy link

haha! Amazing work @nicktacular !

@Rikk
Copy link

Rikk commented May 19, 2015

I have got slightly different results for the test.
I did few changes to the source just for compatibility with php5.3 and to avoid browser eating cpu. My source is here.

On Windows with XAMP/PHP5.5, results are almost identical for both concat and comma, the first being a bit faster:

echo test for 5.5.19
Concat timing: 1.43157411
Comma timing: 1.45181012
Iterations: 2000000

echo test for 5.5.19
Concat timing: 2.86604404
Comma timing: 2.90947986
Iterations: 4000000

Now, on Linux with PHP 5.3, commas are clearly faster:

echo test for 5.3.10-1ubuntu3.18
Concat timing: 1.38291311
Comma timing: 1.06930518
Iterations: 2000000

echo test for 5.3.10-1ubuntu3.18
Concat timing: 2.76822305
Comma timing: 2.23554182
Iterations: 4000000

@omarabid
Copy link

@nicktacular But he is mentioning memory here, not running time?

@nicktacular
Copy link
Contributor

Added memory:

[zero]~ → php comma.php > /dev/null
echo test for 5.5.20

Concat timing: 1.02360797
Concat memory: 744B
Comma timing: 4.03189707
Comma memory: 272B
Iterations: 1000000

500 byte savings over 1 million iterations seems insignificant to me here. Performance improvements frequently come with tradeoffs, but the tradeoff for 500 byte savings with an added ~3 second runtime doesn't make much sense to me.

The script, for reference:

<?php

// use error log so we can eliminate the other echos to STDOUT
error_log(sprintf("echo test for %s\n", phpversion()));

$iterations = 1000000;
$stats = array();

$stats['concat_mem'] = memory_get_usage();

// concat test
$start = microtime(true);
for ($i = 0; $i < $iterations; $i++) {
    echo "this" . "is" . "the" . "real" . "deal" . PHP_EOL;
}
$elapsed = microtime(true) - $start;
$stats['concat'] = $elapsed;

$stats['concat_mem'] = memory_get_usage() - $stats['concat_mem'];

$stats['comma_mem'] = memory_get_usage();

// comma test
$start = microtime(true);
for ($i = 0; $i < $iterations; $i++) {
    echo "this" , "is" , "the" , "real" , "deal" , PHP_EOL;
}
$elapsed = microtime(true) - $start;
$stats['comma'] = $elapsed;

$stats['comma_mem'] = memory_get_usage() - $stats['comma_mem'];

error_log(sprintf(
    "Concat timing: %s\nConcat memory: %sB\nComma timing: %s\nComma memory: %sB\nIterations: %s\n",
    number_format($stats['concat'], 8),
    $stats['concat_mem'],
    number_format($stats['comma'], 8),
    $stats['comma_mem'],
    $iterations
));

@dseguy
Copy link
Author

dseguy commented Jun 19, 2015

I have read your comments with interest. Thanks for taking my PR into consideration.

Indeed, this is for memory optimization, not running time : the former should be the same or close.

When using variables instead of all literals concatenations, PHP can't optimize the code the same way than with literals. In the end, memory saving is more evident, and timing is still the same.

Concat timing: 7.07969689
Concat memory: 12768B
Comma timing: 6.29025602
Comma memory: 2016B
Iterations: 1000000

@serbanghita
Copy link
Owner

@dseguy what PHP are you running?

@serbanghita
Copy link
Owner

The problem here is that commas in echo are not concatenation.
If you take this out of the context, it immediately becomes obvious:

$str = 'a' . 'b';
echo $str;
$str = 'a','b'; // not good

Plus as Nick pointed there is little gain in timing or memory.

@Rikk
Copy link

Rikk commented Jun 20, 2015

@serbanghita I see no problem, it is defined by design... http://php.net/echo
echo is not a function but a language construct, and it allows the output of multiple parameters when using commas (example used by @dseguy), not concatenation.
$str = 'a','b'; contains a syntax error.
echo 'a','b'; will output two parameters.

@omarabid
Copy link

@nicktacular Which makes his point valid. And the difference is huge from my perspective.

500 byte savings over 1 million iterations seems insignificant to me here.

No, that's 500 bytes saving over 1 iteration. But that's the average of 1 million iteration.

In the end, I think this is interesting if you are doing LOTS ( and really LOTS ) of concatenation to start and see the performance improvements.

@serbanghita
Copy link
Owner

@Rikk true, but you missed my point. I can use concatenation . in both contexts (echo 'a' . 'b' or $variable = 'a' . 'b'; echo $variable), while the proposed optimization forces me to use ONLY echo a, b, c; for composing my output which is limited by design.

I would consider this in a cron or a process that has extremely high usage, but not in our case and not even in a PHP template.

@serbanghita
Copy link
Owner

This looks a bit confusing to me.

<body>
<?php $test = 'Serban'; ?>
<?=$test, ' was online on ', date('Y, m, d H:i:s')?>
</body>

@nicktacular
Copy link
Contributor

There are good points all around. I think that having a conversation about optimizations without understanding the precise goal of a particular optimization means that we'll end up talking past each other even though everyone has valid, valuable points.

@omarabid you're right, depending on context, this may or may not be significant a savings. If we drill down into the context of this particular PR, you'll notice that changing a single instance of echo 'Done. Check ',realpath($fileName),' file.'; to echo 'Done. Check ' . realpath($fileName) . ' file.'; doesn't seem to accomplish much.

@dseguy You're right, when using variables and combining with many, many echo operations, you can amount to significant memory savings over time. This, however, doesn't make sense in the context of the PR since there's a constant-time echo operation here.

There are use cases where I can imagine this optimization being very useful, but in this context, I don't see any benefit. (As an aside, there are also many counter-examples I can think of that indicate poor design when looking at code that iterates over operations that uses echo in the first place, rather than buffering, as an example.)

@omarabid
Copy link

@nicktacular I agree.

I don't think this optimization is valuable in this (or most of the PHP) context. Otherwise, it'd been documented as a best practice for major CMS vendors (like WordPress). I have been involved in a few projects that required such optimization. Its use-case happens when you have thousands of concatenation. The memory savings are then valuable because it's easy to jump over 2Gb memory.

For most projects, focus should be on time not memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants