Make WordPress Core

Opened 11 years ago

Last modified 5 years ago

#23044 new enhancement

adjacent_image_link() needs optimization

Reported by: markjaquith's profile markjaquith Owned by:
Milestone: 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 11 years ago.
24688.diff (2.4 KB) - added by andy 11 years ago.
23044.diff (2.7 KB) - added by wonderboymusic 11 years ago.

Download all attachments as: .zip

Change History (24)

#1 @markjaquith
11 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
11 years ago

#22903 was marked as a duplicate.

#3 @DrewAPicture
11 years ago

  • Cc xoodrew@… added

#4 @adamsilverstein
11 years ago

  • Cc ADAMSILVERSTEIN@… added

#5 @goto10
11 years ago

  • Cc dromsey@… added

#6 @SergeyBiryukov
11 years ago

  • Version changed from trunk to 3.5

@hardyy
11 years ago

#7 @hardyy
11 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
11 years ago

  • Cc hardyy added

#9 @westi
11 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
11 years ago

  • Milestone changed from 3.6 to Future Release

@andy
11 years ago

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

Replying to markjaquith:

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

Newly attached patch does exactly that.

#13 @wonderboymusic
11 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
11 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
11 years ago

Cool.

#16 @andy
11 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
11 years ago

  • Milestone changed from 3.7 to Future Release

Patch is dependent on #17065.

#18 @wonderboymusic
10 years ago

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

#19 @wonderboymusic
10 years ago

  • Milestone changed from 3.9 to Future Release

#20 @chriscct7
9 years ago

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

#21 @kitchin
7 years ago

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

Note: See TracTickets for help on using tickets.