Make WordPress Core

Opened 6 months ago

Closed 6 months ago

#61651 closed enhancement (duplicate)

attachment_url_to_postid() performs possibly slow SELECT against meta_value

Reported by: apermo's profile apermo Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.0
Component: Media Keywords:
Focuses: performance Cc:

Description

The function attachment_url_to_postid() in wp-includes/media.php will perform this query.

SELECT post_id, meta_value FROM wp_postmeta WHERE meta_key = '_wp_attached_file' AND meta_value = %s;

While this query looks super harmless, it can be really really slow. On a site that I maintain, this query takes a total of 4s (four seconds).

As I already described in #61383 this issue will happen on large scale websites with several 100k attachments. And even though this is an edge case, I would like to spark an open discussion about it.

In my case, I've verified an index on meta_value, which improved the query from 4s to approx. 5-10ms, I lacked the confidence to do so, so I introduced a custom table for as lookup table, which improved the query down to 2ms. I would have been absolutely happy with the 5-10ms though.

I still need to check wether the extensive calls of attachment_url_to_postid() where caused by WordPress core, or a third Party plugin. I will add this information as soon as possible.

While it may not slow down smaller sites as much as it slowed down my clients site, it will on a larger scale, still burn a noticeable amount of CPU time, that will ultimately consume power and thus produce CO2.

Following a topic on GitHub regarding performance. Otto42 replied to the suggestion to add indexes to wp_postmeta with

Indexing the meta_value is a bad idea, because you should not be searching for anything based on the meta_value in the first place.
---
Otto42 commented on Apr 17, 2022
https://github.com/WordPress/performance/issues/132#issuecomment-1100829520

At WordCamp Europe 2024 in Torino, @luehrsen asked in the Q&A about database improvement. https://www.youtube.com/watch?v=RxQ5yhsxig4&t=429s

I can completely agree with Matts reply, that the WordPress database is extremely flexible and any changes should be made with care.
But since there is a contradiction between "should not be searching for anything base on the meta_value" and what happens inside attachment_url_to_postid(), I think we should discuss and improve this.

I have no clue what the best solution could be.

A few further thoughts to spark the discussion.

wp_posts does have a column post_mime_type.
Why is the mime type in wp_posts and while the path to the attached file is in meta? Could it be the solution to make a column attached_file?
The problem with this is the balance between redundancy and backwards compatibility. Since there will be plugins directly accessing _wp_attached_file in wp_postmeta this would require filters to make get_post_meta() still return _wp_attached_file and set_post_meta() write it. So while I really like this approach, I think this one has a lot special cases and possible side effects.

Indices in MySQL have a length what they index, meta_value for _wp_attached_file will contain something like 2024/07/my-fancy-foto-with-a-speaking-name.jpg (46 characters). So adding even adding a short index, like 10-20 characters long, would already be a gain here. This would likely solve any issues with most queries against wp_postmeta

Change History (5)

#1 @spacedmonkey
6 months ago

  • Focuses performance added; sustainability removed
  • Version changed from trunk to 4.0

#2 @adamsilverstein
6 months ago

@apermo thanks for the ticket -

#3 @apermo
6 months ago

Hello @adamsilverstein.

While #61383 will allow custom solutions for edge cases, I think there should be a general improvement since not everyone will be aware of it, nor will be able to implement a solution.

As mentioned beforehand, a lot of people will not be aware of this issue, I was not aware of it, before I wrote a custom tool for profiling and saw a 4s query, even though the site was already professionally maintained before, the other developers had not found the same issue.

From a quick scan I'd say it describes the same issue and the described solution would be a fix for this topic. So I'd be happy to mark this as duplicate to #41281, my idea was to spark a discussion. That's why I also suggested looking for other solutions than just adding an index to wp_postmeta. I think there should be an open discussion for an improvement for everyone.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


6 months ago

#5 @adamsilverstein
6 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

@apermo closing as a duplicate of #41281, let's continue the discussion there.

Note: See TracTickets for help on using tickets.