Make WordPress Core

Opened 14 years ago

Closed 12 years ago

#15400 closed enhancement (fixed)

Recent comments widget triggers query for each post not in cache

Reported by: nacin's profile nacin Owned by:
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch commit
Focuses: Cc:

Description

Ideally, we should reduce this to a single query.

Attachments (6)

reduct_recent_comments_widget_queries.patch (1.2 KB) - added by chalet16 14 years ago.
patch
15400.patch (1.8 KB) - added by chalet16 14 years ago.
15400-updated.patch (2.0 KB) - added by chalet16 14 years ago.
More safety patch (add and also remove object cache)
15400-1-reduce-to-one-query-when-pagination-disabled.patch (747 bytes) - added by westi 14 years ago.
Patch from http://www.google-melange.com/gci/task/show/google/gci2010/wordpress/t129162975592
15400-2-reduce-queries-to-number-of-posts-when-pagination-enable.patch (1.7 KB) - added by westi 14 years ago.
Another patch from http://www.google-melange.com/gci/task/show/google/gci2010/wordpress/t129162975592
15400.diff (843 bytes) - added by mitchoyoshitaka 12 years ago.
Quick patch using _prime_post_caches

Download all attachments as: .zip

Change History (26)

#1 @scribu
14 years ago

  • Milestone changed from 3.1 to Future Release
  • Type changed from defect (bug) to enhancement

#2 @scribu
14 years ago

  • Keywords 3.2-early added

#3 @scribu
14 years ago

  • Keywords needs-patch added; 3.2-early removed
  • Milestone changed from Future Release to 3.1

Sorry, I misread the 3.1 schedule. This can still go in until Nov. 15.

#4 @voyagerfan5761
14 years ago

  • Cc WordPress@… added

#5 @jane
14 years ago

  • Keywords 3.2-early added
  • Milestone changed from 3.1 to Future Release

Punting due to beta.

#6 follow-up: @westi
14 years ago

I think in general this Widget makes too many queries as we also run queries for all the links to work out which page the comments are on when comment paging is enabled.

#7 @westi
14 years ago

  • Keywords gci added

@chalet16
14 years ago

@chalet16
14 years ago

More safety patch (add and also remove object cache)

#8 @westi
14 years ago

I've reviewed your patch and have the following comments.

  1. You shouldn't directly access/delete cache objects from within the widget code
  2. It would be better to do two queries one to fetch the comments and one with get_posts to fetch and cache the posts.

#9 @garyc40
14 years ago

  • Keywords has-patch added; needs-patch removed

#10 @nacin
12 years ago

  • Keywords needs-patch added; has-patch 3.2-early gci removed
  • Milestone changed from Future Release to 3.5

We can now use _prime_post_caches() here.

@mitchoyoshitaka
12 years ago

Quick patch using _prime_post_caches

#11 @mitchoyoshitaka
12 years ago

  • Cc mitcho@… added
  • Keywords has-patch added; needs-patch removed

#12 @nacin
12 years ago

A term cache *might* be necessary if the permalink has term structures like %category%. I'm fine with the patch as is, but something to think about.

#13 @mitchoyoshitaka
12 years ago

I propose that we open another ticket to create something like a permalinks_include_terms() function (which may have other uses elsewhere?), but otherwise land this for the time being. Nacin, thoughts?

#14 in reply to: ↑ 6 @nacin
12 years ago

Replying to westi:

I think in general this Widget makes too many queries as we also run queries for all the links to work out which page the comments are on when comment paging is enabled.

I forgot about this part.

#15 @nacin
12 years ago

  • Component changed from Cache to Comments

#16 @nacin
12 years ago

Let's at least get _prime_post_caches() in for 3.4. I am thinking we do this as a play-it-safe:

_prime_post_caches( $post_ids, strpos( get_option( 'permalink_structure' ), '%category%' ), false );

#17 @nacin
12 years ago

And by 3.4, I mean 3.5. Tough to keep track these days.

#18 @nacin
12 years ago

  • Keywords commit added

#19 @nacin
12 years ago

In [22278]:

Prime post caches for the Recent Comments widget. props mitchoyoshitaka. see #15400.

#20 @nacin
12 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Opened #22261 for pagination-related queries.

Note: See TracTickets for help on using tickets.