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

Performance drop when using modifiers like date_format #280

Closed
juangacovas opened this issue Aug 26, 2016 · 3 comments
Closed

Performance drop when using modifiers like date_format #280

juangacovas opened this issue Aug 26, 2016 · 3 comments

Comments

@juangacovas
Copy link

Smarty 3.1.30
File: plugins\modifier.date_format.php

Noticed while doing some profiling: for 300 hits to date_format (say, a table with 300 calls to date_format), the code for date_format is doing 300 "require_once", which turns into >100 ms. (ranking top position on my profiling case).

function smarty_modifier_date_format($string, $format = null, $default_date = '', $formatter = 'auto')
{
[...]
  require_once(SMARTY_PLUGINS_DIR . 'shared.make_timestamp.php');
[...]
}

Simple fix to go ~2 ms for 300 date_format calls:

function smarty_modifier_date_format($string, $format = null, $default_date = '', $formatter = 'auto')
{
  static $firsttime = 1;
  if ($firsttime === 1) {
    require_once(SMARTY_PLUGINS_DIR . 'shared.make_timestamp.php');
    $firsttime = 0;
  }
[...]
}

This also happens on some other modifiers.

@uwetews
Copy link
Contributor

uwetews commented Sep 1, 2016

This is now fixed in the master branch

@uwetews uwetews closed this as completed Sep 1, 2016
@sauliux
Copy link

sauliux commented Sep 5, 2016

Hi. Now we are receiving errors after this fix:

array ( 'message' => 'Call to undefined function smarty_make_timestamp()', 'file' => '.../smarty/libs/plugins/modifier.date_format.php', 'line' => 36, )

Maybe instead of require removal you should add this code:

if (!is_callable('smarty_make_timestamp')) { require_once(SMARTY_PLUGINS_DIR . 'shared.make_timestamp.php'); }

Thanks.

@uwetews
Copy link
Contributor

uwetews commented Sep 6, 2016

Sorry, it should have been like that. it's now fixed

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

No branches or pull requests

3 participants