Opened 12 years ago
Last modified 12 days ago
#24688 new defect (bug)
Memory exhaustion caused by very many unattached media
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 2.5 |
Component: | Media | Keywords: | has-patch has-test-info reporter-feedback |
Focuses: | performance | Cc: |
Description
A blog with over 30,000 unattached images was exhausting available memory on unattached attachment pages. The exhaustion occurred in previous_image_link() -> adjacent_image_link() -> get_children()
due to the large number of rows selected with post_parent=0
.
This is not the only place where get_posts()
might be called with post_parent=0
. A similar query appears in twentythirteen_the_attached_image()
but the effect is wisely mitigated by fields=ids
. Earlier, twentyten/loop-attachment.php
called get_children()
with post_parent=0
on unattached image pages which is horribly inefficient when there are many unattached images.
Attached patch depends on #24687. The change to adjacent_image_link()
creates a MySQL query which selects only the desired row. This fixes the memory exhaustion error on the blog with 30,000 unattached images. The once-only filters would be unnecessary if WP_Query accepted arbitrary extra WHERE strings.
Attachments (4)
Change History (21)
#2
@
12 years ago
- Keywords has-patch 3.7-early added
- Milestone changed from Awaiting Review to Future Release
#6
in reply to:
↑ description
@
11 years ago
Replying to andy:
The once-only filters would be unnecessary if WP_Query accepted arbitrary extra WHERE strings.
That's interesting. Are you subtly proposing this idea?
#7
follow-up:
↓ 8
@
11 years ago
About the patch
return "$where AND (...
should be
return "( $where ) AND (...
There are operators that bind less tightly, like OR
.
Is that what you meant by testing? :)
#8
in reply to:
↑ 7
@
11 years ago
I think since #26042 is closed this ticket now just needs a new patch and testing.
#10
@
9 years ago
- Keywords has-patch needs-testing added; needs-patch removed
I've updated the patch for trunk and given it a test drive. It seems to work well for me. Any chance of nudging this along?
#11
@
9 years ago
My objection above still stands, to $where
without parentheses in return "$where AND ...
#12
@
7 years ago
Found an issue with the query for retrieving the next attachment link. It will always link to the last image attached to the post, not necessarily the next one. Turns out, 'orderby' => "menu_order $order, ID $order"
will result in ORDER BY wp_posts.menu_order DESC, wp_posts.ID DESC
no matter what $order is (it'll never be ASC). This can be solved by instead passing an array into the orderby param as supported since WP 4.0.
Next up, array_pop
was also causing a PHP notice:
Notice: Only variables should be passed by reference at wp-includes/media.php:2812
Line 2812 was 'post_parent' => $post->post_parent
. This is solved by either passing a single variable into array_pop(), else avoiding using it. Since in this case get_children()
is just either going to return a single element array or an empty array, I opted for just accessing the first key directly if it exists.
Lastly, I did apply the return "( $where ) AND (
changes mentioned above, but it results in a SQL syntax error.
@
7 years ago
Adding basic unit tests for the previous_image_link() and next_image_link() functions.
#13
@
7 years ago
My objection above still stands, to $where without parentheses in return "$where AND ...
@kitchin , do you happen to have an example of code which would break by applying the patch the way it's being proposed here?
My understanding is that the WHERE clause in WP_Query::get_posts
where posts_where
filter is being applied is being built as a chain of AND
clauses. In case a plugin would be filtering posts_where
while adding OR
w/o proper parentheses, it would either have to be doing some weird string modifications (as the $where
passed to the callback starts with AND
, eg.: AND wptests_posts.post_parent = 85 AND (wptests_posts.post_mime_type LIKE 'image/%') AND wptests_posts.post_type = 'attachment' AND ((wptests_posts.post_status = 'inherit'))
and is being appended to ... WHERE 1=1
string, eg.: SELECT wptests_posts.ID FROM wptests_posts WHERE 1=1
), or would be appending OR
to the end of a string of AND
s effectively bypassing all the previous conditions.
That said, I don't think the callback proposed here should really wrap the initial part of the $where
into parenthesis, especially as it would require us replacing the leading AND
by AND (
which really feels wrong and error prone. In case something would get wrong by not using parenthesis there, it would be because some plugin code does it wrong.
This ticket was mentioned in Slack in #core by sirlouen. View the logs.
3 weeks ago
This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.
3 weeks ago
This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.
12 days ago
#17
@
12 days ago
- Focuses performance added
- Keywords has-test-info reporter-feedback added; needs-testing removed
Reproduction Report
Description
❌ This report can't validate that the issue can be reproduced.
Environment
- WordPress: 6.9-alpha-60093-src
- PHP: 8.2.28
- Server: nginx/1.27.5
- Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
- Browser: Chrome 137.0.0.0
- OS: Windows 10/11
- Theme: Twenty Fourteen 4.2
- MU Plugins: None activated
- Plugins:
- Bulk Image Generator 1.0.0
- Image Memory Exhaustion Tester 1.0.0
- Test Reports 1.2.0
Test Reproduction Suite
- First, I have created 2000 images for the test. It is easy to extrapolate results to 30K but the time it takes to do the whole process is unnecessary in the memory consumption doesn't scale.
- For this purpose I'm using my plugin for generating placeholder images of different sizes: https://raw.githubusercontent.com/SirLouen/bulk-image-generator/refs/tags/1.0.0/bulk-image-generator.php
- Secondly I'm generating a total of 2000 images for the test as I said with the previous plugin
- Finally, I had to create an ad-hoc code to test this specific scenario: https://raw.githubusercontent.com/SirLouen/image-memory-exhaustion-tester/refs/tags/1.0.0/image-memory-exhaustion-tester.php
- It's very important to increase in the PHP server, the
max_execution_time
, put a good number, like 3000 seconds or something like that. Depending on your computer running through 2000 images can take a while - Run the memory tester plugin
- ❌ No special memory exhaustion problems allocated (attached logs in Supplemental Artifacts section)
Actual Results
- ❌ Error condition is not occurring
Additional Notes
- Please review steps taken to verify if I'm not taking something into consideration, or if any of my plugins (specially the memory testing part), is doing what is being reported. Occasionally, it's very difficult to reproduce the same scenario in the same way that the reporter is running it. This is why I've been recommending for quite a while now that reporters, specially when they report any specific performance issues, to provide code samples or whatever Testing Use-Case they think could be valid
- @davidbinda @iCaleb, how were you able to test your patches?
- Given that this report is so old, maybe it was solved at some point in between.
PS: Generally I don't spend so much time creating so much code to prove something. But I found that this was a great example report I want to also add to my Testing Use-Case article in the performance section.
Supplemental Artifacts
- Bulk Image Generator plugin repo: https://github.com/SirLouen/bulk-image-generator/
- Image Memory Exhaustion Tester plugin repo: https://github.com/SirLouen/image-memory-exhaustion-tester/
=== Memory Exhaustion Test Results === Start Time: 2025-06-02 20:24:39 Initial Memory Usage: 36 MB Initial Peak Memory: 36 MB --- TEST 1: get_children() Function --- Attachments Found: 2010 Execution Time: 0.1073 seconds Memory Increase: 14 MB Peak Memory Increase: 14 MB --- TEST 2: previous_image_link() Chain (Simulating Unattached Attachment Page) --- Found 2010 unattached images to test Progress: 100 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 200 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 300 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 400 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 500 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 600 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 700 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 800 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 900 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1000 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1100 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1200 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1300 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1400 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1500 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1600 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1700 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1800 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1900 images processed, Current Memory: 52 MB, Peak: 52 MB Progress: 2000 images processed, Current Memory: 52 MB, Peak: 52 MB Previous Image Link Calls: 2010 Unattached Images Tested: 2010 Execution Time: 66.2879 seconds Memory Increase: 2 MB Peak Memory Increase: 2 MB --- TEST 3: Direct adjacent_image_link() with post_parent=0 Simulation --- Progress: 50 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 100 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 150 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 200 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 250 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 300 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 350 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 400 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 450 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 500 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 550 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 600 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 650 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 700 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 750 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 800 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 850 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 900 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 950 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1000 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1050 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1100 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1150 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1200 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1250 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1300 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1350 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1400 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1450 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1500 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1550 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1600 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1650 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1700 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1750 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1800 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1850 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1900 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 1950 attachments processed, Current Memory: 52 MB, Peak: 52 MB Progress: 2000 attachments processed, Current Memory: 52 MB, Peak: 52 MB Adjacent Image Link Calls: 4020 Attachments Tested: 2010 Execution Time: 133.7525 seconds Memory Increase: 0 B Peak Memory Increase: 0 B --- OVERALL RESULTS --- Total Execution Time: 200.1479 seconds Final Memory Usage: 52 MB Final Peak Memory: 52 MB Total Memory Increase: 16 MB Total Peak Memory Increase: 16 MB PHP Memory Limit: 256M Memory Usage: 20.31% of limit
Related: #23044