Remove syscalls from calling date_default_timezone_set(), date_default_timezone_get()

#2: timezone.patch queued for re-testing.

SubScribe

+++ includes/bootstrap.inc
@@ -1943,9 +1944,26 @@ function drupal_get_user_timezone() {
+function drupal_set_configured_timezone() {

- What about calling this function drupal_set_timezone()? I'm not sure that '_configured_' part adds value.

- In the phpDoc I would explain why we want to set the timezone.

- With the patch, drupal_get_user_timezone() is only called once. It took me a while to grok this patch. I think inlining drupal_get_user_timezone() in drupal_set_(configured_)timezone() would improve readability -- might be a small clean-up. Only for D8.

I rolled back the accidental commit of #2.

#9: timezone.patch queued for re-testing.

This patch causes notices during install: Notice: date_default_timezone_set(): Timezone ID '' is invalid in drupal_set_timezone() (line 1968 of includes/bootstrap.inc).

I'd say drupal_get_user_timezone() should return a valid time zone, which NULL is not. We could check for NULL and then fallback to @date_default_timezone_get():

 $timezone = variable_get('date_default_timezone');
 return isset($timezone) ? $timezone : @date_default_timezone_get(); 

or at least use UTC, which is the last resort of date_default_timezone_get(): return variable_get('date_default_timezone', 'UTC');

By the way there's a typo in this comment, // Note that is no site-wide timezone is set should be if.

There's another problem with this approach. It doesn't account for the fact that the current value of the default time zone may be different from the date.timezone ini setting. See for example the FormatDateUnitTest, where we simulate a logged in user by setting the time zone.

The test fails if the date.timezone ini setting is America/Los_Angeles (the test user's time zone), but the logged in user running the test has some other time zone configured. The time zone needs to be changed by drupal_set_timezone(), but won't be because the test user's time zone is the same as date.timezone.

OK the tests pass now in my environment. One minor thing, I believe elseif is preferred vs. else if.

It looks strange a fallback to ini settings - shared hostings could have this setting unconfigured or wrong for sites they hosts.

I see no reason to use php defaults for site - maybe better use UTC.

#19 introduces a big api change drupal_get_user_timezone() now could return NULL (previously it was php defaults)

I think _drupal_init_timezone() is a better name because it actually initialize a timezone by setting it to user or system timezone...

Modified so that drupal_get_user_timezone() never returns NULL.

+++ includes/bootstrap.incundefined
@@ -1943,9 +1943,32 @@ function drupal_get_user_timezone() {
+/**
+ * Set the timezone for a request.
+ *
+ * This function ensures a timezone is set if it is available in either php.ini,
+ * the default timezone for the site, or user settings.
+ */
+function drupal_set_timezone() {

drupal_init_timezone() is a better name because it means just called once and drupal_set_timezone() supposes a mirror function drupal_get_timezone()

Powered by Dreditor.

I'd agree re: init, since this function isn't guaranteed to actually set the time zone (as seen in the tests).

bikeshedding.

Whatever...

To continue bikeshedding in terms of performance.

+++ includes/bootstrap.incundefined
@@ -1943,9 +1943,32 @@ function drupal_get_user_timezone() {
-    return variable_get('date_default_timezone', @date_default_timezone_get());
+    return variable_get('date_default_timezone', ini_get('date.timezone') ? ini_get('date.timezone') : 'UTC');

ini_get() is called twice probably

 $system_timezone = ini_get('date.timezone');
  return variable_get('date_default_timezone', $system_timezone ? $system_timezone : 'UTC');

should be faster

EDIT: but this depends on PHP - memory allocation for variable could take more time then array key manipulation

Powered by Dreditor.

Benchmarks?

PHP 5.3, 10 nodes with 10 images, load 5 times for each.

Before
=======

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 38.56    0.010000         154        65           select
 13.75    0.003565          19       189           poll
 11.91    0.003088           2      1425       995 open
 10.61    0.002752           0      7044           gettimeofday
  7.88    0.002044           5       435           close
  7.62    0.001977           5       420         5 stat
  6.52    0.001691           1      1155       195 read
  2.04    0.000530           3       200           writev
  0.89    0.000230           0       894           fcntl
  0.22    0.000057           0       200           write
  0.00    0.000000           0       235           fstat
  0.00    0.000000           0        15         5 lstat
  0.00    0.000000           0         5           socket
  0.00    0.000000           0         5           connect
  0.00    0.000000           0         6           accept
  0.00    0.000000           0         6           getsockname
  0.00    0.000000           0         5           setsockopt
  0.00    0.000000           0        10           getdents
  0.00    0.000000           0       195           times
------ ----------- ----------- --------- --------- ----------------
100.00    0.025934                 12509      1200 total






After
=======

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 25.68    0.002832           0      7020           gettimeofday
 20.38    0.002248           2      1425       995 open
 18.71    0.002063           9       235           fstat
 12.19    0.001344           3       435           close
 11.90    0.001312           1      1159       195 read
  8.09    0.000892           2       420         5 stat
  1.70    0.000188           0       870           fcntl
  1.36    0.000150           1       200           write
  0.00    0.000000           0        15         5 lstat
  0.00    0.000000           0       189           poll
  0.00    0.000000           0       200           writev
  0.00    0.000000           0        69           select
  0.00    0.000000           0         5           socket
  0.00    0.000000           0         5           connect
  0.00    0.000000           0         5           setsockopt
  0.00    0.000000           0        10           getdents
  0.00    0.000000           0       195           times
  0.00    0.000000           0         6           restart_syscall
------ ----------- ----------- --------- --------- ----------------
100.00    0.011029                 12463      1200 total

Yes, disable Xdebug reduce a bit of calls. But i think the big different it you are tested on anonymous users and Im logged.

here is the result of dsiable xdebug and anony user (load homepage with 1 node 5 times)

Before
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 72.51    0.018980         204        93           poll
  8.89    0.002327          23       100           fstat
  5.94    0.001556          15       105           writev
  4.85    0.001269           0      3424           gettimeofday
  3.84    0.001004           2       600       400 open
  2.95    0.000771           2       514        99 read
  1.03    0.000270           1       230         5 stat
  0.00    0.000000           0       105           write
  0.00    0.000000           0       205           close
  0.00    0.000000           0        15         5 lstat
  0.00    0.000000           0        20           select
  0.00    0.000000           0         5           socket
  0.00    0.000000           0         5           connect
  0.00    0.000000           0         6           accept
  0.00    0.000000           0         6           getsockname
  0.00    0.000000           0         5           setsockopt
  0.00    0.000000           0       434           fcntl
  0.00    0.000000           0        10           getdents
  0.00    0.000000           0       100           times
------ ----------- ----------- --------- --------- ----------------
100.00    0.026177                  5982       509 total


After
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 42.47    0.006285          67        94           poll
 16.33    0.002416           4       600       400 open
 12.01    0.001778           3       515       100 read
 11.25    0.001665           8       205           close
  9.03    0.001336           0      3400           gettimeofday
  4.77    0.000706           7       105           writev
  4.14    0.000613           6       100           fstat
  0.00    0.000000           0       105           write
  0.00    0.000000           0       230         5 stat
  0.00    0.000000           0        15         5 lstat
  0.00    0.000000           0        20           select
  0.00    0.000000           0         5           socket
  0.00    0.000000           0         5           connect
  0.00    0.000000           0         5           setsockopt
  0.00    0.000000           0       410           fcntl
  0.00    0.000000           0        10           getdents
  0.00    0.000000           0       100           times
  0.00    0.000000           0         6           restart_syscall
------ ----------- ----------- --------- --------- ----------------
100.00    0.014799                  5930       510 total

Just in case, I ended up on these calls by having a breakpoint in the error handler, in D7 we can do this straight forward patch (that will avoid most calls):

Replace this code (bootstrap.inc line 2212):

function drupal_get_user_timezone() {
  global $user;
  if (variable_get('configurable_timezones', 1) && $user->uid && $user->timezone) {
    return $user->timezone;
  }
  else {
    // Ignore PHP strict notice if time zone has not yet been set in the php.ini
    // configuration.
    return variable_get('date_default_timezone', @date_default_timezone_get());
  }
}

By this code:

function drupal_get_user_timezone() {
  global $user;
  if (variable_get('configurable_timezones', 1) && $user->uid && $user->timezone) {
    $timezone = $user->timezone;
  }
  else if (!$timezone = variable_get('date_default_timezone')) {
    // Ignore PHP strict notice if time zone has not yet been set in the php.ini
    // configuration.
    $timezone = @date_default_timezone_get();
  }
  return $timezone;
}

I will post a patch in D7 only bug since D8 seems to diverge too deeply with D7 at this stade.

as PHP 5.4 requirement it makes more sense to clean-up.
Also most of core use UTC:

core8$ git grep date_default_timezone_set
core/lib/Drupal/Component/Gettext/PoHeader.php:81:    // date_default_timezone_set() is called.
core/lib/Drupal/Core/Cache/TimeZoneCacheContext.php:28:    // date_default_timezone_set() is called in SessionManager::initialize(), so
core/lib/Drupal/Core/Session/SessionManager.php:152:    date_default_timezone_set(drupal_get_user_timezone());
core/lib/Drupal/Core/Session/SessionManager.php:306:    date_default_timezone_set(drupal_get_user_timezone());
core/modules/system/src/Tests/Common/FormatDateTest.php:133:    date_default_timezone_set(drupal_get_user_timezone());
core/modules/system/src/Tests/Common/FormatDateTest.php:155:    date_default_timezone_set(drupal_get_user_timezone());
core/modules/system/src/Tests/Datetime/DrupalDateTimeTest.php:103:    date_default_timezone_set(drupal_get_user_timezone());
core/modules/system/src/Tests/Datetime/DrupalDateTimeTest.php:115:    date_default_timezone_set(drupal_get_user_timezone());
core/tests/bootstrap.php:95:date_default_timezone_set('UTC');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/filters/date.test:35:date_default_timezone_set('Europe/Paris');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/filters/date_default_format.test:7:date_default_timezone_set('UTC');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/filters/date_default_format_interval.test:9:date_default_timezone_set('UTC');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/filters/date_immutable.test:18:date_default_timezone_set('Europe/Paris');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/filters/date_interval.test:10:date_default_timezone_set('UTC');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/filters/date_modify.test:7:date_default_timezone_set('UTC');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/filters/date_namedargs.test:8:date_default_timezone_set('UTC');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/functions/date.test:11:date_default_timezone_set('UTC');
core/vendor/twig/twig/test/Twig/Tests/Fixtures/functions/date_namedargs.test:7:date_default_timezone_set('UTC');

also drupal_get_user_timezone() is the only pita finally clean-up global user

Faced with that again

Rerolled patch from #25.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.