# Poor date() performance (v 5.4.9) [PATCH] - Thread: https://externals.io/message/64127 - Original archive: https://news-web.php.net/php.internals/64127 - Started by Paul Taulborg on 2012-12-01T18:06:40+00:00 - Messages: 6 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #64127 — Paul Taulborg — 2012-12-01T18:06:40+00:00 URL: https://externals.io/message/64127#64127 I am migrating from 4.4.9 to some new servers I built out, and wrote a benchmark suite that is testing many individual aspects of PHP so I could see what sort of performance benefits I might see not only from the new server, but moving off my custom forked 4.4.9 branch. Here's a snippet of some of the comparisons: (sorry for the poor formatting) -- each test is run using 1 million loops. 4.4.9 on old machine vs 5.4.9 on new machine: for : 0.213 sec for : 0.019 sec while : 0.145 sec while : 0.014 sec if else : 0.449 sec if else : 0.069 sec switch : 0.547 sec switch : 0.087 sec Ternary : 0.418 sec Ternary : 0.066 sec str_replace : 1.043 sec str_replace : 0.421 sec preg_replace : 3.627 sec preg_replace : 1.678 sec preg_match : 1.250 sec preg_match : 0.509 sec As you can see, the new machine is considerably faster, and there are no major issues with wanting to switch... until I get to the date functions I make frequent use of: date : 1.856 sec date : 2.111 sec strtotime : 2.963 sec strtotime : 3.133 sec and just to test (though I don't currently use it): strftime : 2.679 sec strftime : 1.764 sec The former two actually are slower on the new box and newer version of php, when everything else is 2 to 200x faster. Relevant code to the functions: (tested with and without the $now parameter -- which makes no perceptible difference) date('F j, Y, g:i a', $now); strftime('%B %e, %Y, %l:%M %P', $now); This type of formatting is pretty common. I started digging into the source code, and found an obvious place where there was a performance issue: timelib_isoweek_from_date(t->y, t->m, t->d, &isoweek, &isoyear); is being called every time, even though it's only used in two rather obscure cases, and ones that are probably very uncommon in actual practice. So, to test, I created a "is set" type variable, and moved the function call into each case, with a condition checking if it was already populated, if not, call the function to populate isoweek and isoyear, then resume as before. (Patch will be attached as a file) I then recompiled and reran my benchmark and here is the result: date: 1.763 sec Which is a performance increase of nearly 20%. My patch was thrown together rather quickly to just do a quick test, so it may warrant some tweaking before being applied to any branches. I plan to continue digging, as I feel that I should be able to continue to improve the performance of these functions further. The rest will be a little less obvious, is there is much more cross functionality issues to contend with to ensure nothing is broken. Side note- I attempted the same concept with not setting the timezone information if those flags were not used in the switch (which they aren't in anything I use), but it didn't appear to have any noticeable performance increase. My next step is to start tracing through actual execution and see if I can't find any other obvious issues. My initial thoughts are that it may be faster to try and cache some of this (for fcgi purposes), or even have a compile time option to allow a build to use old 4.4.9 functionality that uses localtime_r() and actually trusts that the server has the right information set. Thanks in advance for looking at this with me! --- ## #64128 — Felipe Pena — 2012-12-01T18:14:34+00:00 URL: https://externals.io/message/64127#64128 · In reply to: #64127 Hi, 2012/12/1 Paul Taulborg > I am migrating from 4.4.9 to some new servers I built out, and wrote a > benchmark suite that is testing many individual aspects of PHP so I > could see what sort of performance benefits I might see not only from > the new server, but moving off my custom forked 4.4.9 branch. Here's a > snippet of some of the comparisons: (sorry for the poor formatting) -- > each test is run using 1 million loops. > > 4.4.9 on old machine vs 5.4.9 on new machine: > for : 0.213 sec for : 0.019 sec > while : 0.145 sec while : 0.014 sec > if else : 0.449 sec if else : 0.069 sec > switch : 0.547 sec switch : 0.087 sec > Ternary : 0.418 sec Ternary : 0.066 sec > str_replace : 1.043 sec str_replace : 0.421 sec > preg_replace : 3.627 sec preg_replace : 1.678 sec > preg_match : 1.250 sec preg_match : 0.509 sec > > As you can see, the new machine is considerably faster, and there are > no major issues with wanting to switch... until I get to the date > functions I make frequent use of: > date : 1.856 sec date : 2.111 sec > strtotime : 2.963 sec strtotime : 3.133 sec > > and just to test (though I don't currently use it): > strftime : 2.679 sec strftime : 1.764 sec > > The former two actually are slower on the new box and newer version of > php, when everything else is 2 to 200x faster. > > Relevant code to the functions: (tested with and without the $now > parameter -- which makes no perceptible difference) > date('F j, Y, g:i a', $now); > strftime('%B %e, %Y, %l:%M %P', $now); > > This type of formatting is pretty common. I started digging into the > source code, and found an obvious place where there was a performance > issue: > timelib_isoweek_from_date(t->y, t->m, t->d, &isoweek, &isoyear); > is being called every time, even though it's only used in two rather > obscure cases, and ones that are probably very uncommon in actual > practice. So, to test, I created a "is set" type variable, and moved > the function call into each case, with a condition checking if it was > already populated, if not, call the function to populate isoweek and > isoyear, then resume as before. (Patch will be attached as a file) > > I then recompiled and reran my benchmark and here is the result: > date: 1.763 sec > Which is a performance increase of nearly 20%. > > My patch was thrown together rather quickly to just do a quick test, > so it may warrant some tweaking before being applied to any branches. > I plan to continue digging, as I feel that I should be able to > continue to improve the performance of these functions further. The > rest will be a little less obvious, is there is much more cross > functionality issues to contend with to ensure nothing is broken. > > Side note- I attempted the same concept with not setting the timezone > information if those flags were not used in the switch (which they > aren't in anything I use), but it didn't appear to have any noticeable > performance increase. My next step is to start tracing through actual > execution and see if I can't find any other obvious issues. My initial > thoughts are that it may be faster to try and cache some of this (for > fcgi purposes), or even have a compile time option to allow a build to > use old 4.4.9 functionality that uses localtime_r() and actually > trusts that the server has the right information set. > > Thanks in advance for looking at this with me! > > As far I remember, the patch must be in a .txt extension to be sent to the list. Thanks. -- Regards, Felipe Pena --- ## #64129 — Paul Taulborg — 2012-12-01T18:17:11+00:00 URL: https://externals.io/message/64127#64129 · In reply to: #64127 My apologies, pasting the patch in directly: --- php-5.4.9_orig/ext/date/php_date.c 2012-11-20 23:12:20.000000000 -0600 +++ php-5.4.9/ext/date/php_date.c 2012-12-01 05:38:22.136264276 -0600 @@ -948,6 +948,7 @@ timelib_time_offset *offset = NULL; timelib_sll isoweek, isoyear; int rfc_colon; + char weekYearSet = 0; if (!format_len) { return estrdup(""); @@ -974,7 +975,6 @@ offset = timelib_get_time_zone_info(t->sse, t->tz_info); } } - timelib_isoweek_from_date(t->y, t->m, t->d, &isoweek, &isoyear); for (i = 0; i < format_len; i++) { rfc_colon = 0; @@ -990,8 +990,12 @@ case 'z': length = slprintf(buffer, 32, "%d", (int) timelib_day_of_year(t->y, t->m, t->d)); break; /* week */ - case 'W': length = slprintf(buffer, 32, "%02d", (int) isoweek); break; /* iso weeknr */ - case 'o': length = slprintf(buffer, 32, "%d", (int) isoyear); break; /* iso year */ + case 'W': + if(!weekYearSet) { timelib_isoweek_from_date(t->y, t->m, t->d, &isoweek, &isoyear); weekYearSet = 1; } + length = slprintf(buffer, 32, "%02d", (int) isoweek); break; /* iso weeknr */ + case 'o': + if(!weekYearSet) { timelib_isoweek_from_date(t->y, t->m, t->d, &isoweek, &isoyear); weekYearSet = 1; } + length = slprintf(buffer, 32, "%d", (int) isoyear); break; /* iso year */ /* month */ case 'F': length = slprintf(buffer, 32, "%s", mon_full_names[t->m - 1]); break; --- ## #64130 — Paul Taulborg — 2012-12-01T18:21:43+00:00 URL: https://externals.io/message/64127#64130 · In reply to: #64127 --- ## #64131 — Christopher Jones — 2012-12-02T00:06:26+00:00 URL: https://externals.io/message/64127#64131 · In reply to: #64130 > > [php_date.c patch] Thanks for the patch. To ensure it isn't lost, can you open a bug at https://bugs.php.net/ and attach it? And/or submit a pull request at https://github.com/php/php-src Regards, Chris -- christopher.jones@oracle.com http://twitter.com/ghrd --- ## #64132 — Paul Taulborg — 2012-12-02T00:53:44+00:00 URL: https://externals.io/message/64127#64132 · In reply to: #64131 Submitted here: https://bugs.php.net/bug.php?id=63666 Also note that this can probably be back-ported into other 5.x branches where this is applicable. I have only personally checked out the latest release branch at this time. Thanks! On Sat, Dec 1, 2012 at 6:06 PM, Christopher Jones wrote: > > > > > […] > > > Thanks for the patch. To ensure it isn't lost, can you open a bug at > https://bugs.php.net/ and attach it? And/or submit a pull request at > https://github.com/php/php-src > > Regards, > > Chris > > -- > christopher.jones@oracle.com > http://twitter.com/ghrd