Opened 4 years ago
Closed 15 months ago
#52759 closed feature request (fixed)
Add a filter to get_available_post_mime_types() function to allow overriding extremely slow query.
Reported by: | maciejmackowiak | Owned by: | joemcgill |
---|---|---|---|
Milestone: | 6.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | has-patch commit |
Focuses: | performance | Cc: |
Description
On sites sites with lot of posts the query used in get_available_post_mime_types is extremely slow.
It would be nice to have a filter that will allow us to override this query and for example cache the results.
It could be something simillar to this filter:
https://core.trac.wordpress.org/browser/tags/5.6/src/wp-includes/media.php#L4172
<?php function get_available_post_mime_types($type = 'attachment') { global $wpdb; $types = apply_filters('get_available_post_mime_types', null, $type); if ( ! is_array( $types ) ) { $types = $wpdb->get_col($wpdb->prepare( "SELECT DISTINCT post_mime_type FROM $wpdb->posts WHERE post_type = %s", $type ) ); } return $types; }
Change History (29)
#1
@
4 years ago
- Component changed from General to Media
- Focuses performance added
- Keywords needs-patch good-first-bug added
This ticket was mentioned in PR #1088 on WordPress/wordpress-develop by maciejmackowiak.
4 years ago
#3
- Keywords has-patch added; needs-patch removed
#4
@
4 years ago
Here is a pull request:
https://github.com/WordPress/wordpress-develop/pull/1088/files
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
4 years ago
#6
@
4 years ago
Hi there,
I'm definitely for a filter to short circuit this slow query, but I think first we need to fix this slow query by implementing a cache, wether it use the internal cache API system from Core or the Transient API, (check the slack thread we started for further comments).
Related tickets talking about the performance issues of this query: https://core.trac.wordpress.org/ticket/43658
#8
@
4 years ago
check the slack thread we started for further comments
Can you point me to that thread, please?
#9
@
4 years ago
@maciejmackowiak The link is right above. https://wordpress.slack.com/archives/core-media/p1615475295157700
#10
in reply to:
↑ 2
@
3 years ago
Replying to archon810:
For us, it adds an additional 0.35s or so (right now, more if db is loaded).
SELECT SQL_NO_CACHE DISTINCT post_mime_type FROM wp_posts WHERE post_type = 'attachment' > OK > Time: 0.35sand I hope with the new proposed filter, we will be able to speed up the site even more.
Ok, let's say a new filter is added, what next? How would you like to improve the query for better performance?
Drop an example of such a query. I am curious about your experience with this.
This ticket was mentioned in PR #5063 on WordPress/wordpress-develop by @rcorrales.
16 months ago
#11
Add a filter to get_available_post_mime_types().
#12
@
16 months ago
Submitted a PR for adding a filter, similar to the one previously submitted, but with docs added and changing the $types
variable name to $mime_types
to avoid confusion with the $type
variable, which is used for the post_type
:
#13
@
16 months ago
- Milestone changed from Awaiting Review to 6.4
- Owner set to johnbillion
- Status changed from new to reviewing
@johnbillion commented on PR #5063:
16 months ago
#16
#17
@
16 months ago
Should this be a pre_*
filter instead? Like pre_get_available_post_mime_types
--I feel like other filter naming conventions follow similarly:
pre_get_*
short circuits if a non-null value is returned.get_*
filters the results of the data from the function.
#18
@
16 months ago
- Keywords 2nd-opinion added; good-first-bug has-patch commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
To be honest I would have preferred available_post_mime_types
(without the get_
prefix) so it's more declarative, but a quick search showed that there's a large number of filters with a get_
prefix so I left it at that.
I guess it could be a pre_
filter but core is really inconsistent in this regard too. Re-opening in case anyone has a strong opinion; if not we can re-close.
#19
@
16 months ago
@spacedmonkey can you please investigate this from the performance point of view?
#20
@
16 months ago
I agree that a pre_ filter name would be better here, since we're not filtering the return value from the DB, good suggestion @emrikol.
@oglekler there isn't a specific performance impact to adding the filter here, but it does create an opportunity for sites to potentially avoid an expensive DB query, which is why it makes sense as a performance enhancement.
This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.
16 months ago
This ticket was mentioned in PR #5110 on WordPress/wordpress-develop by @rcorrales.
16 months ago
#22
- Keywords has-patch added
Updating the filter name to pre_get_available_post_mime_types
.
#23
@
16 months ago
Thanks for the suggestion. I added a PR for updating the filter name to pre_get_available_post_mime_types
.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
16 months ago
This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.
16 months ago
#26
@
16 months ago
- Keywords commit added; 2nd-opinion removed
The PR looks good to me. Going to leave for @johnbillion to approve, but marking for commit.
+1 for this.
For us, it adds an additional 0.35s or so (right now, more if db is loaded).
On the grid view it is called from here:
https://github.com/WordPress/WordPress/blob/785cb6cc7d4d17ef0aa16b187dcfb6a8dc513e1b/wp-admin/includes/class-wp-media-list-table.php#L97
It calls this function:
https://github.com/WordPress/WordPress/blob/00680f2e89766e4ac9a71c68a8cccd3a141afb34/wp-admin/includes/post.php#L1287
And here is the query:
https://github.com/WordPress/WordPress/blob/bf83c368fdfcee2b879d00b193f505038e1681f0/wp-includes/post.php#L7517
We were able to cache and greatly speed up this query already:
and I hope with the new proposed filter, we will be able to speed up the site even more.