Skip to content

Commit

Permalink
- performance require_once should be called only once for shared plug…
Browse files Browse the repository at this point in the history 
…ins #280
  • Loading branch information
@uwetews
uwetews committed Sep 6, 2016
1 parent e1d27d6 commit 51e0d5c
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions libs/plugins/modifier.date_format.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ function smarty_modifier_date_format($string, $format = null, $default_date = ''
if ($format === null) {
$format = Smarty::$_DATE_FORMAT;
}
/**
* require_once the {@link shared.make_timestamp.php} plugin
*/
if (!is_callable('smarty_make_timestamp')) {
require_once(SMARTY_PLUGINS_DIR . 'shared.make_timestamp.php');
}
if ($string != '' && $string != '0000-00-00' && $string != '0000-00-00 00:00:00') {
$timestamp = smarty_make_timestamp($string);
} elseif ($default_date != '') {
Expand Down

5 comments on commit 51e0d5c

@AnrDaemon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Optimizations" like these are very poisonous.
On small samples (under 1'000 attempts), straight require_once $file is no slower than if(!is_callable(...)) require_once $file even on a very old versions of PHP, but code readability suffers.
https://3v4l.org/b5HCA

@juangacovas
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. But this comes from a real use case (see the associated issue #280), profiling the "date_format" modifier with 300 calls on a template. Real profiling giving 100 ms, optimized went to 2 ms. Perhaps some weird scenario (SAPI, environment, I didn't test everything) but seemed a good performance gain.

@AnrDaemon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then move require_once away from function. Will be much better performance, as it were in multiple instances. Why moving it inside a function and then guarding with obnoxious flow control switches?

@AnrDaemon
Copy link
Contributor

@AnrDaemon AnrDaemon commented on 51e0d5c May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding to that, the use case provided in #280 is wastly different from the current master implementation.
Compare local variable dereference with a registered functions' table lookup?

@juangacovas
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the commit now. If that's indeed faster, then nice one. Thanks.

Please sign in to comment.