WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 10 months ago

#23132 new enhancement

wp_timezone_override_offset() has noticeable performance hit

Reported by: Rarst Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 3.5
Component: Date/Time Keywords: has-patch
Focuses: Cc:

Description

While wandering through XHProf generated callgraph of my test stack I came upon wp_timezone_override_offset() eating 3.4% of runtime (73 calls). I do not expect noticeable resource consumption from such minor configuration-related function.

The obvious performance aspect is that it creates and discards two objects (DateTime and DateTimeZone - latter seemingly more heavy) per each call, while calculating same result each run.

The less obvious aspect is that the only use of this function is to pre-filter gmt_offset option, and it essentially hijacks option cache for it.

Possible solutions:

  • calculate result once per run and cache in static variable inside function (straightforward, but retains multiple function call)
  • filter gmt_offset on set rather than get (more involved)
  • scrap the extra function and/or option altogether? is there still practical need for having two options for timezone information?

Attachments (1)

23132.patch (1.1 KB) - added by SergeyBiryukov 15 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 Rarst16 months ago

  • Cc contact@… added
  • Summary changed from wp_timezone_override_offset() has notiecable performance hit to wp_timezone_override_offset() has noticeable performance hit

comment:2 Otto4216 months ago

  • Cc Otto42 added

SergeyBiryukov15 months ago

comment:3 SergeyBiryukov15 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.6

23132.patch implements the first option.

comment:4 TJNowell15 months ago

  • Cc tom@… added

comment:5 nacin15 months ago

Using a static variable cache that wraps a get_option() call means that this function won't work for switch_to_blog(). It'd need to be keyed by the option value (which I think is okay).

comment:6 follow-up: Otto4214 months ago

Long term, this should be gutted and rewritten to eliminate gmt_offset entirely. This stuff is a holdover from when PHP 4 was still supported. The new stuff added in 2.8 was done this way to support PHP 4 as well and keep the date/time consistent across the systems. For example, the default timezone is set to GMT no matter what you select in order to keep this PHP 4/5 code working identically.

The right way:

  • Kill gmt_offset entirely.
  • Gut the various timezone calculation code to put it back on PHP 5's code to do the grunt work.
  • Most of this can be handled by using the native gmdate/date functions correctly with date_default_timezone_set.

Nacin's right that switch_to_blog would have to change timezones, but it would be able to do so by calling date_default_timezone_set to do so instead of using static cached variables.

We're PHP 5 native now, we should take advantage of this. It is a big job though.

comment:7 in reply to: ↑ 6 nacin14 months ago

Replying to Otto42:

We're PHP 5 native now, we should take advantage of this. It is a big job though.

It's also not realistic to do, as it'll be a pretty big backwards compatibility hit.

comment:8 Otto4214 months ago

I don't think it'll be as bad as you think, really. But again, it is long term.

comment:9 ocean9010 months ago

  • Milestone changed from 3.6 to Future Release
Note: See TracTickets for help on using tickets.