WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 11 months ago

#23044 new enhancement

adjacent_image_link() needs optimization

Reported by: markjaquith Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Query Keywords: has-patch needs-unit-tests needs-refresh
Focuses: Cc:

Description

adjacent_image_link() is really slow and stupid. It queries for all of the images attached to that post. And then it picks the previous or next one in PHP. And there's no caching, so you'll typically get this happening three times on an attachment page, (image [linked to next], prev link, next link).

We should actually just make the query grab the ONE image we want.

Attachments (3)

23044.patch (2.4 KB) - added by hardyy 5 years ago.
24688.diff (2.4 KB) - added by andy 4 years ago.
23044.diff (2.7 KB) - added by wonderboymusic 4 years ago.

Download all attachments as: .zip

Change History (24)

#1 @markjaquith
5 years ago

Talked with Nacin about this. This may end up being moot if we solve the bigger issue of determining the correct prev/next image, considering that images don't have to be in parent-post-bounded galleries.

In the meantime, this is the plugin I wrote to make it so that it doesn't query everything:

https://gist.github.com/4357848

#2 @obenland
5 years ago

#22903 was marked as a duplicate.

#3 @DrewAPicture
5 years ago

  • Cc xoodrew@… added

#4 @adamsilverstein
5 years ago

  • Cc ADAMSILVERSTEIN@… added

#5 @goto10
5 years ago

  • Cc dromsey@… added

#6 @SergeyBiryukov
5 years ago

  • Version changed from trunk to 3.5

@hardyy
5 years ago

#7 @hardyy
5 years ago

Added an optional argument in get_children() so that it returns numerical array containing only IDs of elements. Easier to return than array of objects.
Also makes it easier to search through in-built array functions.
in_array() is added to make sure loop don't skip in some cases.
If i have done any mistake, feel free to notify me.

#8 @hardyy
5 years ago

  • Cc hardyy added

#9 @westi
5 years ago

As well as the major optimisation of an improved query here there is a simple optimisation we could use to improve the performance of this on sites with a large number of images attached to a post.

If we switch from get_children to get_posts we can avoid calling update_post_cache on every attachment post for every query.

#10 @ocean90
4 years ago

  • Milestone changed from 3.6 to Future Release

@andy
4 years ago

#12 in reply to: ↑ description @andy
4 years ago

Replying to markjaquith:

We should actually just make the query grab the ONE image we want.

Newly attached patch does exactly that.

@wonderboymusic
4 years ago

#13 @wonderboymusic
4 years ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 3.7

+1 to Skelton patch, I added a few doc block things

#14 @westi
4 years ago

Andy's patch looks neat - last time I optimised this I ended up with up to 6 simple queries to populate post__in to limit the get_children call which was much more consistently scalable but still pretty crappy with regard to the number of queries required.

I think it would be good to add some simple unit tests that prove this sort correctly when there are multiple images with the same menu_order in both directions and the same as the current image.

It isn't that I doubt it does it is just that is the one thing that tripped me up before and was how I ended up with 6 queries because I failed to find a simple single query which reliably worked for all cases ;)

#15 @nacin
4 years ago

Cool.

#16 @andy
4 years ago

Before trying my patch above please read #24688. The patch depends on #24687 which was closed as a duplicate of #17065 (Independent ASC/DESC in multiple ORDER BY statement.)

#17 @nacin
4 years ago

  • Milestone changed from 3.7 to Future Release

Patch is dependent on #17065.

#18 @wonderboymusic
4 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Future Release to 3.9

#19 @wonderboymusic
4 years ago

  • Milestone changed from 3.9 to Future Release

#20 @chriscct7
2 years ago

  • Keywords needs-refresh added
  • Severity changed from major to normal

#21 @kitchin
11 months ago

return "$where AND ... should be return "( $where ) AND ...

Note: See TracTickets for help on using tickets.