Bug #75088 date() and DateTimeZone::getTransitions() are slow with large timestamps #2693
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
motecshine
reviewed
Aug 18, 2017
ext/date/lib/unixtime2tm.c
Outdated
| while (tmp_days >= DAYS_PER_LYEAR) { | ||
| while (days >= DAYS_PER_LYEAR) { | ||
| cur_year++; | ||
| TIMELIB_DEBUG(printf("days=%lld, year=%lld\n", days, cur_year);); |
There was a problem hiding this comment.
may be you should remove it
motecshine
reviewed
Aug 18, 2017
ext/date/lib/unixtime2tm.c
Outdated
| tmp_days += 1460970; | ||
| while (days <= 0) { | ||
| cur_year--; | ||
| TIMELIB_DEBUG(printf("days=%lld, year=%lld\n", days, cur_year);); |
motecshine
reviewed
Aug 18, 2017
| i--; | ||
| } | ||
| TIMELIB_DEBUG(printf("A: ts=%lld, year=%lld, month=%lld, day=%lld,", ts, cur_year, i + 1, tmp_days - months[i]);); | ||
| TIMELIB_DEBUG(printf("A: ts=%lld, year=%lld, month=%lld, day=%lld,", ts, cur_year, i + 1, days - months[i]);); |
blar
changed the title
|
I guess changes should be done in upstream repository here |
|
@andrewnester is https://github.com/derickr/timelib correct or should i send a pull request to https://github.com/andrewnester/timelib? |
|
@blar yes, it should be https://github.com/derickr/timelib , sorry |
|
These changes belong upstream, the tests can be pulled into php-src when upstream pulls changes. |
|
On 6 September 2017 10:00:02 BST, Joe Watkins ***@***.***> wrote:
These changes belong upstream, the tests can be pulled into php-src
when upstream pulls changes.
Thanks! Did you merge the test though?
cheers,
Derick
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://bugs.php.net/bug.php?id=75088
getTransitions is slow with large negative or positive values. With the default values PHP_INT_MIN and PHP_INT_MAX the function takes a very long time.
@derickr As you wish in #2690. Please check if this patch is correct.
Comparing and Benchmark
I compared the output of date() with 1.000.000 random timestamps between PHP_INT_MIN and PHP_INT_MAX and 1.000.000 random timestamps between strtotime('1900-01-01') and strtotime('2100-01-01') with my local PHP 7.1.6 and PHP 7.3.0-dev.
PHP 7.1.6
PHP 7.1.6 (cli) (built: Jun 25 2017 18:59:00) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
real 79m48.622s
user 77m25.880s
sys 0m57.932s
PHP 7.3.0-dev
PHP 7.3.0-dev (cli) (built: Aug 11 2017 20:34:53) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.3.0-dev, Copyright (c) 1998-2017 Zend Technologies
real 0m18.027s
user 0m2.337s
sys 0m14.291s
Difference
With the 2.000.000 random timestamps i got only 4 different results which seems to be a bug. Any timestamp that is a multiple of 146097 (DAYS_PER_LYEAR_PERIOD) or 146096 (depends on timezone?) * 86400 (SECS_PER_DAY) lead to a zero day in a date (yyyy-01-00). https://3v4l.org/vBtut
PHP 7.1.6
PHP 7.3.0-dev