Skip to content

Conversation

blar
Copy link
Contributor

@blar blar commented Aug 15, 2017

timezone_transitions_get is very slow, because php_format_date in add_nominal with the default value ZEND_LONG_MIN took very long to iterate from ZEND_LONG_MIN to ZEND_LONG_MAX on 64 bit systems. No transitions exists at this time before the timestamp -2147483648 (INT32_MIN).

With this patch:

Default Value
Duration: 0.000201

Timestamp for 1970-01-01 00:00:00
Duration: 0.000092

Timestamp for 0001-01-01 00:00:00
Duration: 0.000087

PHP 7.1.6:

Default Value
Duration: 0.045996

Timestamp for 1970-01-01 00:00:00
Duration: 0.000082

Timestamp for 0001-01-01 00:00:00
Duration: 0.000097

Benchmark

function measure_duration(callable $callback) {
    $start = microtime(true);
    call_user_func($callback);
    $duration = microtime(true) - $start;
    printf("Duration: %s\n\n", number_format($duration, 6));
}

$timezone = new DateTimeZone('Europe/Berlin');

measure_duration(function() use($timezone) {
    printf("Default Value\n");
    $timezone->getTransitions();
});

measure_duration(function() use($timezone) {
    $timestamp = 0;
    printf("Timestamp for %s\n", date('Y-m-d H:i:s', $timestamp));
    $timezone->getTransitions($timestamp);
});

measure_duration(function() use($timezone) {
    $timestamp = -62135596800;
    printf("Timestamp for %s\n", date('Y-m-d H:i:s', $timestamp));
    $timezone->getTransitions($timestamp);
});

https://3v4l.org/MhkR8

@KalleZ
Copy link
Member

KalleZ commented Aug 16, 2017

cc @derickr

@derickr
Copy link
Member

derickr commented Aug 16, 2017

This patch fixes a symptom, and not a cause. The real problem lies in timelib:

https://github.com/derickr/timelib/blob/master/unixtime2tm.c#L77-L81

should really do the same as:

https://github.com/derickr/timelib/blob/master/unixtime2tm.c#L52-L55

So I am closing this pull request, and I've added an issue on timelib:

derickr/timelib#18

@derickr derickr closed this Aug 16, 2017
@derickr
Copy link
Member

derickr commented Aug 16, 2017

BTW, I'd be more than welcome to accept a patch for timelib if you wish to make one.

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.

3 participants