[wp-trac] [WordPress Trac] #57683: Improve performance of mysql2date

[WordPress Trac]

#57683: Improve performance of mysql2date
--------------------------+-----------------------------
 Reporter:  spacedmonkey  |       Owner:  (none)
     Type:  enhancement   |      Status:  new
 Priority:  normal        |   Milestone:  Future Release
Component:  Date/Time     |     Version:  0.71
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:  performance
--------------------------+-----------------------------

Comment (by Rarst):

 I know that this is confusing (welcome to Date/Time component!), please
 bear with me. :)

 Let's break it down from scratch.

 We have input of a local time in MySQL format (that doesn't include time
 zone information) and we need to produce a couple of formatted outputs.
 The formats are historically fixed and do not contain time zone
 information either.

 The easiest way to do this semantically is to delegate the operation to a
 DateTime object, which is very good at both parsing date inputs and
 formatting date outputs. However DateTime object doesn't have a concept of
 ambiguous "local" time, it must instantiate with a concrete time zone,
 provided explicitly or taken from the runtime default.

 So the operation `date_create( $post->post_date )` takes a local input and
 doesn't provide time zone, leaving it to runtime default (which would be
 UTC as set by WordPress core on boot... or whatever user changed it to,
 because people do that and we can't stop them).

 Now you have an object instantiated from local time using a time zone that
 does not correspond to that local time. That object is in completely
 invalid state and points to an altogether wrong moment in time, that is
 different from the original input. **This is bad and should not be done.**

 Now, since our output ''in this case'' is coarse to a day, the empirical
 output will happen to be the same in this case. **That does not justify
 this as an acceptable practice.** We spent a lot of time fixing issues
 stemming from doing exactly this - transforming time inputs in an invalid
 time zone states. I will die on a hill on not having a code like this in
 core, because we shouldn't be ever giving this as viable example of doing
 such.

 Given that context I see the options worked out so far are following:
 1. Use a DateTime object as is (we can halve the instances, by not calling
 mysql2date twice).
 2. Use a DateTime object and cache a time zone information to reuse. In my
 experience bespoke caches increase complexity and decrease testability.
 3. Do not use a DateTime object. Given the specific input and outputs in
 this case we can do that with a bit of string manipulation.

 To be clear and restate my positions:
 1. Using invalid DateTime object is a hard no from me.
 2. Introducing a bespoke cache for time zone - so far I am unconvinced the
 downsides are worth the performance improvement. Caching options should be
 generally left to its own cache, not sprinkled randomly through the core.
 3. Not using a DateTime object is fine by me in this case, given relative
 simplicity and closeness of input/output.

 Sanity check - would you go through entire core and cache every single
 options access manually "for performance"? No? Then why treat this option
 fetch as special, it's not.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/57683#comment:20>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform