update all update_option calling in core to set the autoload option

[pbearne]

Let's be set the autoload option so it is clear what is autoloadered
Let's not auto load option only needed in WP-admin or very often to reduce the number always loaded

[joemcgill]

Adding the performance keyword for visibility.

[swissspidy]

Why is this needed? Aren't the autoload flags for all these options already set correctly by default anyway? Is there a concern that the flag somehow changes so we have to fix it again?

[pbearne]

I feel it good practice to set the default to make clear that choice was made

And I feel that some should not be autoload eg admin email and maybe widgets

I have replaced the on/off with booleans

As I have gone through these I have set any option that looks like it is wp-admin only to not autoload this should help the front-end load a little bit

[pbearne]

I thought it might help to list the options I propose, not to autoload, that are currently not unset in core.

update_option( 'auto_plugin_theme_update_emails', $past_failure_emails, false );
update_option( 'recently_activated', $recently_activated, false );
update_option( '_wp_suggested_policy_text_has_changed', $state, false );
update_option( $lock_option, time(), false );
update_option( 'dashboard_widget_options', $widget_options, false );
update_option( 'ftp_credentials', $stored_credentials, false );
update_option( 'recently_edited', $oldfiles, false );
update_option( 'adminhash', $new_admin_email, false );
update_option( 'nav_menu_options', $nav_menu_option, false );
update_option( 'active_plugins', $current, false );
update_option( 'uninstall_plugins', $uninstallable_plugins, false );
update_option( 'wp_force_deactivated_plugins', array(), false );
update_option( 'delete_blog_hash', $hash, false );
update_option( 'allowedthemes', $allowed_themes, false );
update_option( 'admin_email', $new_admin_detailsnewemail?, false );
update_option( 'recently_activated', $recent, false );
update_option( 'https_detection_errors', $support_errors->errors, false );
update_option( 'fresh_site', '0', false );
update_option( 'upload_path', UPLOADBLOGSDIR . "/$blog_id/files", false );
update_option( 'admin_email', , false );
update_option( 'admin_email_lifespan', time() + $remind_interval, false );

src/wp-includes/class-wp-paused-extensions-storage.php
return update_option( $option_name, $paused_extensions, false );

src/wp-includes/class-wp-recovery-mode-key-service.php
return update_option( $this->option_name, $keys, false );

this is one that we may not want to set false
update_option( 'widget_' . $id_base, $widget, false );

Please review this list and comment if any of these need to be autoload (as needed by the front-end)
I hope we can get this change in, as this will free some memory.

[swissspidy]

I feel it good practice to set the default to make clear that choice was made

I thought it might help to list the options I propose, not to autoload, that are currently not unset in core.

Can we achieve this with a one time database migration instead of (or in addition to) changing all update_option calls.

[pbearne]

Not all of them can be done, as some are dynamic
But we could do most of them

I will work on the SQL/ update function to do this

[pbearne]

I have added an upgrade
I am not sure how to set the db version

[mukesh27]

Hi there!

This ticket discussed in today's performance scrub.

Moving to 6.7 for early consideration.

Additional props: @joemcgill

[pbearne]

Note to self: use wp_prime_site_option_caches() for all the options not been autoloaded to be loaded in WP-admin

Could you fix your bot that keeps mentioning me? I'm getting dozens of notifications a month from this project. ​https://github.com/WordPress/wordpress-develop/pulls?q=sort%3Aupdated-desc+%22%40paul%22

Could you fix your bot that keeps mentioning me? I'm getting dozens of notifications a month from this project. ​https://github.com/WordPress/wordpress-develop/pulls?q=sort%3Aupdated-desc+%22%40paul%22

@pbearne Are you able to look at the details in your git config so the bot stops pinging the wrong Paul. It's currently set to paul bearne <paul> which is causing the bot to be confused.

If you add pbearne@git.wordpress.org to your GitHub profile and set that as the email address for WordPress-Develop the commits will be assigned correctly.

If you add pbearne@git.wordpress.org to your GitHub profile and set that as the email address for WordPress-Develop the commits will be assigned correctly.

updated

[SergeyBiryukov]

Replying to swissspidy:

Can we achieve this with a one time database migration instead of (or in addition to) changing all update_option calls.

Yeah, converting a few options to be non-autoloaded seems fine, but changing every update_option() call in core to specify the default value seems redundant to me.

[pbearne]

Replying to SergeyBiryukov:

Replying to swissspidy:

Can we achieve this with a one time database migration instead of (or in addition to) changing all update_option calls.

Yeah, converting a few options to be non-autoloaded seems fine, but changing every update_option() call in core to specify the default value seems redundant to me.

This new sniff would require them all to be set ​https://github.com/WordPress/WordPress-Coding-Standards/issues/2473

[flixos90]

@swissspidy

Can we achieve this with a one time database migration instead of (or in addition to) changing all update_option calls.

I'm not sure. I guess the way that core uses options, in most cases update_option() truly just updates the option (rather than inserting it), so in those cases it's fine to not pass it. But for new options it should be passed. For all the options that are prepopulated during WP installation and never deleted, a once-off migration would work.

@SergeyBiryukov

Yeah, converting a few options to be non-autoloaded seems fine, but changing every update_option() call in core to specify the default value seems redundant to me.

Per the above, I think it depends. I think the problem is when update_option() falls through to add_option(). For those cases, preferably the $autoload value should always be explicitly specified.

@pbearne

This new sniff would require them all to be set ​https://github.com/WordPress/WordPress-Coding-Standards/issues/2473

FWIW, this would be in WordPress-Extra so wouldn't necessarily apply to WordPress core. Most plugins use options differently than core: Core inserts almost all the options with their defaults upon install, and then update_option() calls truly update them. Plugins on the other hand often don't "pre-populate" them in the database, but then use update_option() as "update or add the option if it's not there yet". For the latter, passing $autoload is more important.

[flixos90]

I just left a thorough review on the PR in ​https://github.com/WordPress/wordpress-develop/pull/6465#pullrequestreview-2261711999, where I reviewed each potentially modified option individually.

Summarizing here:

• We should only pass an explicit $autoload of true|false to update_option() if the option is not added to the database as part of the WordPress installation (and therefore always present when calling update_option().
• For any arbitrary update_option() calls where $option is a variable that could be anything, we should not provide a value either since we have no idea about whether or not to autoload the option. The default value null works best in such cases.
• The following options (which are not part of WordPress installation) are set by the PR to no longer be autoloaded, which make sense to me because all of them are only used in one specific admin screen or one specific process that doesn't run on regular page loads:
  • recently_activated
  • _wp_suggested_policy_text_has_changed
  • {upgradeLock}.lock
  • dashboard_widget_options
  • ftp_credentials
  • adminhash
  • nav_menu_options
  • wp_force_deactivated_plugins
  • delete_blog_hash
  • allowedthemes
  • {sessionId}_paused_extensions
  • recovery_keys
  • https_detection_errors
  • fresh_site

There are a few other options in the PR that are set to no longer autoload, which I think is risky and should therefore not be included. More complicated options can still be discussed in separate tickets for whether it makes sense to stop autoloading them.

I think once those changes are made, the PR will be in a good state to land.

[flixos90]

In 58945:

Options, Meta APIs: Stop using 'yes' and 'no' for autoload parameter in favor of recommended boolean.

This changeset does not modify any behavior, it only updates the code to use the recommended type for the $autoload parameter as of WordPress 6.6. The old values 'yes' and 'no' are only maintained in certain tests that are explicitly about these backward compatibility values.

Props flixos90, joemcgill, mukesh27.
Fixes #61939.
See #61103, #61929.

[flixos90]

In 58949:

Options, Meta APIs: Soft-deprecate use of 'yes' and 'no' as $autoload parameter.

WordPress 6.6 option autoload enhancements included discouraging the use of 'yes' and 'no' to indicate whether to autoload an option when calling add_option() or update_option(). Instead, a boolean should be used.

This changeset brings the newer autoload related functions wp_set_option_autoload_values(), wp_set_options_autoload(), and wp_set_option_autoload() in line with those changes. Additionally, it soft-deprecates the values more formally, as they should no longer be used. No PHP warnings will be emitted though as this is not a hard deprecation. This change is purely about documentation.

Props flixos90, joemcgill, jrf, mukesh27.
Fixes #61929.
See #61103, #61939.

[flixos90]

In 58975:

Options, Meta APIs: Explicitly pass $autoload parameter to when potentially adding new options.

It is recommended that for every option it is explicitly set whether to autoload it or not. This changeset updates relevant update_option() and add_option() calls.

Note that the $autoload parameter is only needed for update_option() if the option is potentially not present yet, i.e. the call will pass through to add_option(). Since WordPress core adds the majority of its options to the database during installation, only update_option() calls for dynamically added options need to be modified, which is what this changeset does.

As part of revisiting the autoload values for dynamically added WordPress core options, this changeset modifies some options to no longer be autoloaded, since they are only accessed in a few specific places that are not relevant for a regular request. These options are:

• recently_activated
• _wp_suggested_policy_text_has_changed
• {upgradeLock}.lock
• dashboard_widget_options
• ftp_credentials
• adminhash
• nav_menu_options
• wp_force_deactivated_plugins
• delete_blog_hash
• allowedthemes
• {sessionId}_paused_extensions
• recovery_keys
• https_detection_errors
• fresh_site

An upgrade routine is present as well that sets those options to no longer autoload for existing sites.

Props pbearne, flixos90, mukesh27, swissspidy, SergeyBiryukov, joemcgill, adamsilverstein.
Fixes #61103.

Committed in https://core.trac.wordpress.org/changeset/58975