Opened 6 years ago
Last modified 12 hours ago
#42441 assigned defect (bug)
Disable autoload for large options
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Options, Meta APIs | Keywords: | has-patch has-unit-tests 2nd-opinion |
Focuses: | performance | Cc: |
Description
A frequent issue I encounter with WordPress sites is extreme slowdown due to a huge option that is being autoloaded. Sometimes this is some sort of cache option that a careless plugin has let be autoloaded, and has grown to tens of megabytes.
Having an option this large be autoloaded not only slows downs the options loading query (when that option may not be required on every load), but on sites with object caching, it causes a lot of wear and tear on the alloptions cache. Combine a large option with a frequently updated option and you have a recipe for a very sluggish site.
We should consider preventing options from being autoloaded if they grow to a certain size. Off the top of my head, 100kb sounds like a reasonable upper bounds. We could monitor option settings/updates and peek at the size of the option going in. If it's above a certain (filterable) bounds, we can force autoload to be off.
Change History (25)
#1
@
5 months ago
- Milestone changed from Awaiting Review to 6.4
- Owner set to spacedmonkey
- Status changed from new to assigned
This ticket was mentioned in PR #4881 on WordPress/wordpress-develop by @spacedmonkey.
5 months ago
#2
- Keywords has-patch has-unit-tests added
#4
@
4 months ago
- Keywords needs-refresh added
Per my feedback on the PR, I think the approach it takes is too complicated and puts too many responsibilities into a single function. So I think this needs an iteration.
@spacedmonkey commented on PR #4881:
3 months ago
#5
Something to consider is if we should change these values to default-
#6
@
3 months ago
- Keywords 2nd-opinion added; needs-refresh removed
@markjaquith @boonebgorges Any chance you can take a look at the PR https://github.com/WordPress/wordpress-develop/pull/4881? Would be great to get your thoughts on the direction.
This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.
2 months ago
This ticket was mentioned in Slack in #core-performance by flixos90. View the logs.
2 months ago
#9
@
2 months ago
- Milestone changed from 6.4 to 6.5
This ticket was discussed in the WordPress core performance chat today. It was agree this ticket needs more time to bake and for feedback from the community. Punting to WP 6.5
This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.
2 months ago
#11
@
8 weeks ago
- Owner spacedmonkey deleted
I am unable to work on this ticket in WP 6.5, removing myself as owner.
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
4 weeks ago
This ticket was mentioned in PR #5671 on WordPress/wordpress-develop by @pbearne.
3 weeks ago
#14
Code refresh
This ticket was mentioned in PR #5671 on WordPress/wordpress-develop by @pbearne.
3 weeks ago
#15
Code refresh
#16
@
3 weeks ago
I have refreshed the code
I found an issue with the tests failing
The update_option() function was clearing the cache correctly
This is ready to merge
This ticket was mentioned in Slack in #core-performance by pbearne. View the logs.
3 weeks ago
This ticket was mentioned in Slack in #core-performance by pbearne. View the logs.
2 weeks ago
@flixos90 commented on PR #4881:
2 weeks ago
#19
Closing in favor of #5671, which iterates on this.
#20
@
2 weeks ago
I just reviewed the new PR by @pbearne, and it looks very good to me functionally, despite needing a few more changes.
Separately, I would like to bring the conversation about the new database values for the autoload
column back to this ticket, to decouple from the specific PR implementation.
Including the new values (regardless of whether they're called default-yes
/default-no
or something else) in the database is not only a "side effect" of this change. It is a requirement to make the thresholds for the maximum size of an option work reliably. Here's why:
- Options don't always maintain the same size. An option may be added with some very small starter/placeholder value, but later grow much larger depending on how the option is used and how the end user interacts with the relevant functionality.
- Because of that, it is crucial that we not only perform the maximum size check when adding the option, but also when updating it.
- For that, unless the developer explicitly passes a value to
update_option()
(which is not exactly intuitive to do when you just want to update the option value), we need to see whether the autoload value previously stored is an explicit one chosen by the developer (yes|no
) or not (default-yes|default-no
). In the latter case, we must re-check the option size. But in the first case, we must not do that, as we need to respect the developer's explicit choice. - In other words, being able to determine between explicitly set autoload values and "chosen by WordPress" autoload values requires this differentiation to be made in the database.
The only alternative to that which I can think of is completely doing away with the $autoload
parameter of all those functions and instead handle it as part of either setting registration or a separate way to register an option's autoload value independently. That however would have much wider implications and affect far more plugins than the currently proposed implementation that adds support for two more database values. Only plugins that implement their own functionality around autoloading options (e.g. certain database optimization plugins) will require an update with the current proposal, while a broader shift towards autoloading registration would be far more disruptive and affect plugins of all kinds.
#21
@
2 weeks ago
Or we can just block all large options from autoloading
Even this can worked round by adding a late filter to set the limit high enough
Add a hard limit of 150kb of option size. Also add a new default value of autoload to default.
Trac ticket: https://core.trac.wordpress.org/ticket/42441