WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 4 months ago

#23330 new defect (bug)

Allow autoloading all options, not just autoload = yes options

Reported by: ryan Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5.1
Component: Cache API Keywords: needs-patch
Focuses: Cc:

Description (last modified by ryan)

For 5 years on wordpress.com we have ignored the autoload field for options. In wp_load_alloptions(), autoload = yes is not part of the query. Why? Because wordpress.com has a persistent cache. Thus, the transients that constitute the bulk of autoload = no options are stored in cache, never in the options table. Querying autoload=yes makes the query slower for no reason.

Having autoload = no options also complicates notoptions caching. Core has a notoptions cache that would be unnecessary if you can assume that wp_load_alloptions() loads every option. If an option is not in alloptions, then it does not exist.

Other large sites that use a persistent cache might also like to ignore the autoload flag. Core could automatically switch to ignoring autoload when an external object cache is being used, but administrators might want to control this so they can clean out old transients from options tables and do whatever other housekeeping is desired. So, let's add a filter that allows toggling this. When autoload is being ignored the wp_load_alloptions() query does not include autoload=yes and notoptions is not used.

Change History (14)

comment:1 @ryan2 years ago

  • Description modified (diff)

comment:2 @nacin2 years ago

If combined with DB transient garbage collection, I think I could go for this being on by default (with a persistent cache in place).

comment:3 follow-up: @rmccue2 years ago

-1. I'm going to have to disagree with this. If you store custom objects in the database, those fail to load thanks to unserialize being run before your classes get redefined. There's no way to force these to be reloaded from the database once they are loaded into the object cache, so you're stuck.

(I experienced this problem yesterday and it was incredibly annoying.)

comment:4 @toscho2 years ago

  • Cc info@… added

comment:5 in reply to: ↑ 3 ; follow-up: @ryan2 years ago

Replying to rmccue:

-1. I'm going to have to disagree with this. If you store custom objects in the database, those fail to load thanks to unserialize being run before your classes get redefined. There's no way to force these to be reloaded from the database once they are loaded into the object cache, so you're stuck.

(I experienced this problem yesterday and it was incredibly annoying.)

That's a good reason for not ignoring autoload by default.

Since this can happen to autoload = yes options too, perhaps we can finagle a method for postponing unserialize for custom objects.

comment:6 in reply to: ↑ 5 @rmccue2 years ago

Replying to ryan:

That's a good reason for not ignoring autoload by default.

Since this can happen to autoload = yes options too, perhaps we can finagle a method for postponing unserialize for custom objects.

I can see this being a little complicated, but doable. I'm actually storing an array of objects, so my guess is to search for O:\d+:"\w+" and check class_exists(), then hook in at plugins_loaded to unserialize those ones.

I think it's a bit of overkill, and may have performance issues if we use regex (with a huge number of options). Possible to do with with string parsing (strspn), probably faster, but I don't think it's really necessary, since setting it to not autoload is an easy way to deal with it.

comment:7 follow-ups: @dd322 years ago

Can we do JIT unserialization instead perhaps? Don't unserialize on load, but instead do it on first-access?

comment:8 in reply to: ↑ 7 ; follow-up: @nacin2 years ago

Replying to dd32:

Can we do JIT unserialization instead perhaps? Don't unserialize on load, but instead do it on first-access?

That's what I would have figured we already do. Unserialization isn't exactly speedy and surely we woukd have made that optimization at some point.

It turns out, we only unserialize on load in wp_load_core_site_options() -- so, a couple of multisite options we probably need anyway. (The potential for accidentally double-unserializing is not major as the list of queried options is thankfully a static list.) So I'm not really sure what exactly rmccue is running into.

One point, though, is we unserialize not just on first access, but on every access. Changing that is possible but we'll need to juggle that state somehow.

Separate note: How do we use get_multi without knowing individual key names?

comment:9 in reply to: ↑ 7 @rmccue2 years ago

Replying to dd32:

Can we do JIT unserialization instead perhaps? Don't unserialize on load, but instead do it on first-access?

Hmm. I just checked the code and it looks like it does already do that. Strange... I'll investigate further.

comment:10 in reply to: ↑ 8 @rmccue2 years ago

Unable to reproduce locally and I don't have the code that was causing it originally. When I get back to my other computer I'll attempt to reproduce and track this down, but I suspect it may be unrelated (although changing autoload to no did work), so I'll file it as a separate issue.

+1 for autoloading all from me then.

Replying to nacin:

One point, though, is we unserialize not just on first access, but on every access. Changing that is possible but we'll need to juggle that state somehow.

It's off-topic, but I was thinking about this just before. Seems like another cache (or just storing it statically) may work.

comment:11 @rmccue2 years ago

Tracked it down: it's a result of APC object caching and inconsistent behaviour between update_option() and add_option(); filed as #23381. Not a result of autoloading, but rather update_option()'s broken behaviour, so +1 for autoloading all.

comment:12 @mark-k2 years ago

I don't know, AFAIK wordpress.com is a very controlled environment with well behaving themes and plugins, but what will happen to sites which installed bad behaving plugins and themes which stored a lot of unused data in the options table? how bad will the be hit by such change?

comment:13 @wonderboymusic16 months ago

  • Keywords needs-patch added

comment:14 @ashfame4 months ago

I think we should not autoload everything, this would cause issues on several sites which make use of options table as a general purpose storage table than small options. WordPress.com is obviously a very controlled environment.

Example: A lot of shipping rates plugin of WooCommerce save huge data in options table, which would slow the autoload query a lot. Also this would cause an increase in memory footprint without a persistent caching storage and possibly never-used cache entries or worse undesired evictions in case of persistent caching storage.

This impacts a lot of WordPress sites which are doing more CMS-like things.

Note: See TracTickets for help on using tickets.