[wp-trac] [WordPress Trac] #16894: Bulk load Comment Meta upon access

WordPress Trac noreply at wordpress.org
Thu Sep 17 15:16:33 UTC 2015


#16894: Bulk load Comment Meta upon access
-----------------------------------+--------------------------
 Reporter:  dd32                   |       Owner:  bradt
     Type:  defect (bug)           |      Status:  reopened
 Priority:  normal                 |   Milestone:  4.4
Component:  Comments               |     Version:  3.1
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |     Focuses:  performance
-----------------------------------+--------------------------
Changes (by boonebgorges):

 * keywords:  has-patch => has-patch 2nd-opinion


Comment:

 [attachment:16894.4.diff] is a naive implementation of [comment:20 dd32's
 suggestion]. No lazy-loading - all comment meta is primed during
 `WP_Comment_Query::query()`, configurable with the
 'update_comment_meta_cache' flag.

 Lazy-loading is obviously going to be more complicated, and we don't have
 good systems in place for it anywhere else in core. My inclination would
 be to go with the naive implementation in 4.diff for general use of
 `WP_Comment_Query`. Then we could introduce lazy-loading as a progressive
 enhancement in places where we already have a container for keeping track
 of global state, like in `WP_Query` (which doesn't use `WP_Comment_Query`
 anyway). See [attachment:16894.5.diff] for an implementation that I think
 is in the spirit of what dd32 originally suggested.

 I'm not sure I understand the "by-reference array manipulation" issue
 wonderboymusic refers to in [34245]. Is it just because of how
 `wp_list_pluck()` works? I skipped using it in [attachment:16894.5.diff].

 Either way, let's clean this up, because builds are failing due to the
 `update_comment_cache` unit test.

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


More information about the wp-trac mailing list