[php-maint] Bug#618462: Bug#618462: php5: Use of systems' embedded timezone data causes performance regression

[Maciej Wiercinski]

Hi Sean,

I've spotted it while debugging system with misconfigured Apache2
prefork mpm. System kept spawning / killing processes at quite high
rate. Approximately 20% of the system time was spent in kernel mode,
of which my rough estimate (I have straced apache and its children,
not the whole system) is that 50% was spent on stat().

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 58.64   12.022001         446     26926      3275 stat
 23.83    4.884488        2429      2011       391 access
 15.42    3.160267          10    320599      1734 lstat

Another very rough estimate is that mentioned zoneinfo stat()s
accounted for about 80-85% of total stats(). When you sum it up you
will get a figure of less than 10% of CPU time.

Now the point is that the system was misconfigured, thus I could
expect performance fireworks. In light of this I wouldn't call this
issue critical, more a wishlist item.

I had a quick look through the code

 const timelib_tzdb *timelib_builtin_db(void)

is being called only twice in the main source tree and the actual use
is straightforward:

/* {{{ php_date_set_tzdb - NOT THREADSAFE */
PHPAPI void php_date_set_tzdb(timelib_tzdb *tzdb)
{
        const timelib_tzdb *builtin = timelib_builtin_db();

        if (php_version_compare(tzdb->version, builtin->version) > 0) {
                php_date_global_timezone_db = tzdb;
                php_date_global_timezone_db_enabled = 1;
        }
}
/* }}} */

It sounds feasible to try to rewrite it so it appends stuff to
timelib_tzdb->index as it's requested (with possible forced full read
on methods like timezone_abbreviations_list etc.). Are you aware of
any reasons why would you need the whole timezone index in the memory
(only thing I can think of is consistent indexing , as this data is
alpha sorted ) ?

Cheers,
Maciej


On Tue, Mar 15, 2011 at 1:46 PM, sean finney <seanius at debian.org> wrote:
> Hi Maciej,
>
> Does this actually cause a quantifiable and significant performance
> regression?  This possibility of performance issues was discussed some
> time ago but it was decided that the stat calls would just hit the kernel
> fs cache and not cause any serious problems.
>
> If there are indeed problems, there are certainly ways this could be
> worked around (building a binary cache kept up to date with the pkg
> management system, for example), but it would add even further complexity
> to the patch which we'd all prefer to avoid if possible.
>
>
> To give you some extra background, since the PHP authors certainly have
> their own take on the situation, but EVERY serious linux distribution
> ships this patch in some form.  Redhat, Fedora, Debian, Ubuntu, SLES,
> Opensuse (okay, maybe not Gentoo, but anyway...) all ship with this patch.
> So please keep that in mind when you here both sides of this argument :)
>
>
> The problem is that when the OS distributors release a timezone update,
> they don't want to *also* have to go package by package updating
> indivudual and "customized" timezone databases that might be embedded
> in a given application.  Neither do they want to continuously update the
> version of PHP in their "stable" releases and have to deal with the numerous
> regressions that would result.  The PHP tzdata changes are mixed in with the
> mainline development, and sometimes depend on other changes within the
> engine, so it's not really feasible to cherry pick out the changes into
> a stable release, even if we wanted to.
>
> This is a point of disagreement with the PHP authors, who want to have
> control over this aspect of the engine themselves (and they certainly have
> their justifications, such as systems with outdated or nonexistant tzdata,
> plus they add some extra TZ annotations in their private copy).
> Unfortunately they are interested in providing any other way to work
> around this issue, despite the periodic overture from us or RedHat.
> The invitation is still open to try and find a reasonable technical
> solution for this, but I have been led to beleive that Derick has really
> dug in his heels on the issue and it's not worth any of our time to
> raise a big stink about it.
>
>
>
>        sean
>