#10886 closed defect (bug) (fixed)
WordPress should not unnecessarily query posts at page load
Reported by: | junsuijin | Owned by: | spacedmonkey |
---|---|---|---|
Milestone: | 6.0 | Priority: | normal |
Severity: | normal | Version: | 2.8.4 |
Component: | Bootstrap/Load | Keywords: | has-unit-tests has-patch needs-docs has-dev-note |
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 (11)
Change History (56)
#3
in reply to:
↑ 1
@
15 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
#5
@
15 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.
#6
@
15 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.
#7
@
15 years ago
- Keywords tested added
For reference, here is the corresponding BuddyPress ticket:
http://trac.buddypress.org/ticket/967
@
15 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?
#8
follow-up:
↓ 9
@
15 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.
#9
in reply to:
↑ 8
@
15 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.
#10
@
15 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.
#11
follow-up:
↓ 12
@
15 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.
#12
in reply to:
↑ 11
@
15 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 :-)
@
15 years ago
Adds $query_vars to the filter so that you can use that information to determine how you should return.
@
15 years ago
Adds $query_vars to the filter so that you can use that information to determine how you should return. (fixed grammatical error)
#13
@
15 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)?
#14
@
15 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.
#17
@
15 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.
#19
@
11 years ago
- Component changed from Optimization to Bootstrap/Load
- Focuses performance added
- Keywords changed from has-patch, needs-testing to has-patch needs-testing
#20
@
11 years ago
Would be nice to see this in 4.0. All workarounds, like the one posted on wp-hackers some time ago, are ugly and lead to errors.
#21
@
11 years ago
- Keywords needs-patch needs-refresh added; has-patch needs-testing removed
Patch blows up
#23
@
9 years ago
10886.4.diff still applies but needs @since doc updated when it goes in.
#24
@
9 years ago
- Keywords commit added
- Milestone changed from Future Release to 4.4
10886.5.diff updates the @since
version and fixes some docs issues.
Moving for commit consideration.
#26
@
9 years ago
Since [21163] happened in 3.5, should probably be something more like 10886.6.diff
This ticket was mentioned in Slack in #core by sergey. View the logs.
9 years ago
#28
@
9 years ago
In 10886.7.diff:
$wp->parse_request()
needs to return abool
- If it returns
false
, send headers, but don't do any querying, 404 handling, or setup of globals
If you use these filters:
add_filter( 'template_include', '__return_false' ); add_filter( 'do_parse_request', '__return_false' );
The whole request will only produce 3 db queries (no external cache):
SELECT option_value FROM wp_options WHERE option_name = 'WPLANG' LIMIT 1 SELECT * FROM wp_users WHERE user_login = 'admin' SELECT user_id, meta_key, meta_value FROM wp_usermeta WHERE user_id IN (1) ORDER BY umeta_id ASC
get_option( 'locale' )
gets called even if WPLANG
is defined in wp-config.php
or elsewhere.
#31
@
9 years ago
- Milestone changed from 4.4 to Future Release
I'll grab this later if I get the energy
This ticket was mentioned in Slack in #core by ocean90. View the logs.
9 years ago
This ticket was mentioned in PR #2032 on WordPress/wordpress-develop by pbearne.
3 years ago
#35
- Keywords has-unit-tests added
Patch refresh that:
Added boolean returns to parse_request()
Add shortcut to stop load not needed calls to main() if parse_request() returns early
Add test to make sure parse_request() return boolean
Trac ticket: (https://core.trac.wordpress.org/ticket/10886
#36
@
3 years ago
- Keywords early added
Probably safe to include this earlier in a cycle to make sure it's thoroughly tested.
#37
@
3 years ago
- Keywords needs-dev-note added
+1 on the patch. I think we should get this in early 6.0 release.
I did a quick review on plugins that use this filter. See here.Looks like return the value, so I don't see breakages here. But this needs a dev not as it may have side effects.
#41
@
3 years ago
Was there also some commit in core that demonstrates using $parsed
? For example, to optimize the infamous robots.txt loading process, or something else?
Perhaps wp() shouldn't be called at all for those pages.