WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#27985 closed defect (bug) (fixed)

wp_enqueue_media() may be slow on very large sites

Reported by: mboynes Owned by: johnbillion
Milestone: 3.9.1 Priority: normal
Severity: normal Version: 3.9
Component: Media Keywords: fixed-major commit
Focuses: performance Cc:
PR Number:

Description

I have sites with very large databases and I've noticed some slow queries since 3.9 on new/edit post screens:

SELECT post_mime_type, COUNT( * ) AS num_posts FROM wp_posts WHERE post_type = 'attachment' AND post_status != 'trash' GROUP BY post_mime_type

From my experience, this query can take upwards of a second to run on a large dataset. Given that it's not on every screen, and it's only really a problem on large databases, perhaps the benefits outweigh the costs. I figured it was best to open a ticket to ensure those costs are properly considered, as well as alternative solutions to get the same end result.

This query is coming from wp_count_attachments() which is being called by wp_enqueue_media(). It looks like this was added in #27554.

Attachments (2)

27985.diff (2.3 KB) - added by pento 6 years ago.
27985.2.diff (2.9 KB) - added by wonderboymusic 6 years ago.

Download all attachments as: .zip

Change History (16)

#1 @nacin
6 years ago

I kind of feared this one, but since it was run on the media list table I didn't think too much of it.

We can change this to two queries, which would be faster:

SELECT ID FROM $wpdb->posts WHERE post_type = 'attachment' AND post_mime_type LIKE 'video/%' LIMIT 1
SELECT ID FROM $wpdb->posts WHERE post_type = 'attachment' AND post_mime_type LIKE 'audio/%' LIMIT 1

There is no index on post_mime_type though, so this is till a where on however many attachment records there are.

#2 @nacin
6 years ago

We could/should also cache those values. This isn't unlike is_multi_author(), though persistent cache is probably fine (versus a transient) as it isn't being used on the frontend.

#3 @nacin
6 years ago

  • Milestone changed from Awaiting Review to 3.9.1

@pento
6 years ago

#4 @pento
6 years ago

  • Keywords has-patch needs-testing added

attachment:27985.diff is a first pass at caching the query in wp_count_attachments().

This ticket was mentioned in IRC in #wordpress-dev by nacin1. View the logs.


6 years ago

#6 @nacin
6 years ago

  • Owner set to johnbillion
  • Status changed from new to assigned

#7 @wonderboymusic
6 years ago

27985.2.diff - I missed the chatter about @johnbillion doing it

#8 @johnbillion
6 years ago

That's fine, my approach was going to be similar. I don't think there's any point in changing the counts code in the media view for a point release.

#9 @johnbillion
6 years ago

Ok I think this is fit to go in.

In 4.0, I'd like to see the attachment type counts removed and replaced with boolean transients that are populated in much the same way that the value of is_multi_author() is (eg. has_video_attachments and has_audio_attachments).

#10 @johnbillion
6 years ago

In 28191:

Avoid an expensive attachment counting query on the post editing screen. See #27985, for trunk.

#11 @johnbillion
6 years ago

  • Keywords fixed-major commit added; has-patch needs-testing removed

#12 @johnbillion
6 years ago

In 28194:

Remove some unnecessary abstraction introduced in [28191]. See #27985.

#13 @nacin
5 years ago

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

In 28261:

Avoid an expensive attachment counting query on the post editing screen.

Merges [28191], [28194] to the 3.9 branch.

props johnbillion.
fixes #27985.

#14 @ocean90
4 years ago

Follow-up: #32264

Note: See TracTickets for help on using tickets.