WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 3 months ago

#10886 new defect (bug)

WordPress should not unnecessarily query posts at page load

Reported by: junsuijin Owned by: junsuijin
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8.4
Component: Bootstrap/Load Keywords: has-patch needs-testing
Focuses: performance Cc:

Description

For plugins like BuddyPress, with pages that have no WordPress post content, a query for posts at load time creates unnecessary overhead. I've tested this patch (with WordPress MU) and it removes 3 queries on all pages when the 'NO_QUERY' constant is defined. A very quick scan of page load times also suggests about 20% in time savings on average.

Other plugins like shopping carts and any others that needn't query the WordPress posts table can benefit from this as well.

My previous means of achieving this effect was to simply return null to the 'posts_request' filter, which removes only 2 queries per page.

The devised method uses suggestions from azaozz and sivel, and allows for setting the definition of 'NO_QUERY' using the 'init' action hook. It also still allows for further querying of the posts at a later point if necessary.

I welcome further suggestions about the patch, and any other ways this could be accomplished more effectively.

Attachments (6)

lighter-load.patch (660 bytes) - added by junsuijin 5 years ago.
lighter-load.2.patch (739 bytes) - added by sivel 5 years ago.
New patch that replaces the constant with a filter, and skips wp() as well.
lighter-load.3.patch (643 bytes) - added by junsuijin 5 years ago.
filter used, wp() circumvented, 'wp' hook left intact, commented
10886.diff (601 bytes) - added by westi 5 years ago.
I don't think we should be calling the wp hook from two different places like that. I think I prefer this patch. Does this work for you?
10886.2.diff (563 bytes) - added by sivel 5 years ago.
Adds $query_vars to the filter so that you can use that information to determine how you should return.
10886.2.2.diff (562 bytes) - added by junsuijin 5 years ago.
Adds $query_vars to the filter so that you can use that information to determine how you should return. (fixed grammatical error)

Download all attachments as: .zip

Change History (25)

junsuijin5 years ago

comment:1 follow-up: ryan5 years ago

Perhaps wp() shouldn't be called at all for those pages.

comment:2 usermrpapa5 years ago

  • Cc steve@… added

comment:3 in reply to: ↑ 1 westi5 years ago

  • Cc westi added

Replying to ryan:

Perhaps wp() shouldn't be called at all for those pages.

Possibly a good idea.

Whatever I think a filter to which you return false to disable stuff may be better here than a define.

We don't want to create something that people put into wp-config.php by accident defines should be reserved for things configured there IMHO

sivel5 years ago

New patch that replaces the constant with a filter, and skips wp() as well.

comment:4 sivel5 years ago

  • Cc matt@… added
  • Keywords tested removed

comment:5 sivel5 years ago

It seems I may have misunderstood Ryan's suggestion. I have talked to junsuijin and he should be doing some testing and adding a new patch.

comment:6 junsuijin5 years ago

Building on ryan, westi, and sivel's work/suggestions, this new patch seems to further improve page loading times. More testing indicates that extra queries are caused by setting the 'wp_main' filter to false when site-wide post requests are made later on. This seems a case of benefitting the majority over the minority though, since the people that find the 'wp_main' filter set to false on a page where they want to include something like a site-wide posts widget, can just reset the filter to true.

comment:7 junsuijin5 years ago

  • Keywords tested added

For reference, here is the corresponding BuddyPress ticket:
http://trac.buddypress.org/ticket/967

junsuijin5 years ago

filter used, wp() circumvented, 'wp' hook left intact, commented

westi5 years ago

I don't think we should be calling the wp hook from two different places like that. I think I prefer this patch. Does this work for you?

comment:8 follow-up: sivel5 years ago

I'd like to have the filter pass $query_vars also so that the $query_vars could be used as a way to determine if you should return false or not. I don't have a current use for this, but I see a possibility of it being helpful.

comment:9 in reply to: ↑ 8 westi5 years ago

Replying to sivel:

I'd like to have the filter pass $query_vars also so that the $query_vars could be used as a way to determine if you should return false or not. I don't have a current use for this, but I see a possibility of it being helpful.

+1

That sounds very sensible.

comment:10 sivel5 years ago

  • Keywords tested removed

The patch as it stands now causes the admin to say there are the correct number of posts/pages/attachments but "hides" them from view in the edit lists. I'm not saying this is bad or good. Just wondering what everyone else has to say about it. Should it be up to the developer to make sure they only filter for the front end by checking if is_admin() is true? Or do we want to assume that we always want to be able to see this in the admin? Or should we add a second filter for the admin? So we would have run_wp_main_front and run_wp_main_admin or something like that.

comment:11 follow-up: sivel5 years ago

Sorry, I should have also stated that this is the result when running the filter using a plugin or in a themes functions.php without context to what is actually being called/loaded.

comment:12 in reply to: ↑ 11 westi5 years ago

Replying to sivel:

Sorry, I should have also stated that this is the result when running the filter using a plugin or in a themes functions.php without context to what is actually being called/loaded.

I think if someone does this in a theme/plugin without checking the context then they deserve to be eaten by those dragons :-)

sivel5 years ago

Adds $query_vars to the filter so that you can use that information to determine how you should return.

junsuijin5 years ago

Adds $query_vars to the filter so that you can use that information to determine how you should return. (fixed grammatical error)

comment:13 junsuijin5 years ago

I know that BuddyPress hooks to 'wp' in several places. Not doing the 'wp' action simply because WordPress has not run its initial post-grabbing queries, will also break any other plugins that rely on that action hook.
Which is less desirable:
1) "[not] calling the wp hook from two different places?"
2) forcing those who choose to return false to the 'run_wp_main' filter to do_action_ref_array( 'wp', array( (object) array() ) ) (likely in whatever function they use to set the 'run_wp_main' filter to false)?

comment:14 westi5 years ago

I don't know what BuddyPress uses the wp hook for but I think the following is what we should achieve.

  • We are trying to stop something running so we should just stop that.
  • We should not duplicate do_actions or expect plugins to run them if they use the new hook - If we do this I think we have failed.

Therefore it looks like we need to understand if wp is the hook BuddyPress should be using on these pages and we need to move the new filter into the wp.main() function or whether BuddyPress should be hooked in to something else.

comment:15 ryan4 years ago

  • Milestone changed from 2.9 to Future Release

comment:16 mtdewvirus4 years ago

  • Cc mtdewvirus@… added

comment:17 dd324 years ago

See Also: #12256

robots.txt requests cause WordPress to query the database for all the homepage posts. It'd be good if there was perhaps a query var to short circuit the internal querying and instead perhaps fake a page template... Or probably better to just offer an appropriate hook.

comment:18 Denis-de-Bernardy4 years ago

  • Cc Denis-de-Bernardy added

comment:19 nacin3 months ago

  • Component changed from Optimization to Bootstrap/Load
  • Focuses performance added
  • Keywords changed from has-patch, needs-testing to has-patch needs-testing
Note: See TracTickets for help on using tickets.