Make WordPress Core

Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#59975 closed defect (bug) (fixed)

Set autoload to 'no' for previously activated themes

Reported by: mukesh27's profile mukesh27 Owned by: flixos90's profile flixos90
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch commit
Focuses: performance Cc:

Description

By default, WordPress sets the theme_mods_* options to autoload when a theme is activated. A previous ticket, #39537, successfully addressed this by ensuring that when a different theme is activated, the previous theme's autoload settings are removed.

However, we've identified an additional issue. When users activate multiple themes over time, all corresponding options remain set to autoload. Although #39537 updated autoload settings for old and new themes, there are still lingering autoload settings for other previously activated themes.

As per the discussion in https://github.com/WordPress/wordpress-develop/pull/5706#issuecomment-1827606503, it's proposed that an upgrade routine should be implemented to remove autoload for these themes' options.

Change History (13)

#1 @mukesh27
5 months ago

  • Keywords needs-unit-tests removed

This ticket was mentioned in PR #5711 on WordPress/wordpress-develop by @mukesh27.


5 months ago
#2

  • Keywords has-patch added; needs-patch removed

Trac ticket: https://core.trac.wordpress.org/ticket/59975

This PR add upgrade routine for 6.5 that set autoload to 'no' for previously activated themes.

@mukesh27 commented on PR #5711:


5 months ago
#3

@swissspidy i get Error: The theme "twentytwentyone" is not installed in Performance tests. Not sure if the PR changes affect their 🤔

@swissspidy commented on PR #5711:


5 months ago
#4

No idea why this happens just for this PR 🤷

@joemcgill commented on PR #5711:


5 months ago
#5

Seems confirmed that the Performance Test is failing due to this change for some reason as other, more recent runs, are not effected. See: https://github.com/WordPress/wordpress-develop/actions/workflows/performance.yml?query=

We need to debug this before committing in order to not introduce failures into trunk.

@mukesh27 commented on PR #5711:


5 months ago
#6

The Performance tests going fails if we update DB version. Check PR: #5716 not sure but seems there is bug Performance tests setup.

cc. @felixarntz @joemcgill @swissspidy

@swissspidy commented on PR #5711:


5 months ago
#7

Oooh I know what's up!

Because of the version bump and the following checkout of the previous commit, WordPress is showing the "Database upgrade required" screen, so you first have to run the db upgrade. I tested this hypothesis in https://github.com/WordPress/wordpress-develop/pull/5717, where you can see that tests now pass :)

@mukesh27 commented on PR #5711:


5 months ago
#8

Great @swissspidy, it's fascinating how we can encounter unforeseen issues while focusing on different tasks. I suggest we commit that PR initially without altering the DB version. Afterward, we can rebase this PR, making it ready for the final commit.

@mukesh27 commented on PR #5711:


5 months ago
#9

Performance tests ✅

#10 @flixos90
5 months ago

  • Keywords commit added
  • Owner changed from mukesh27 to flixos90
  • Status changed from assigned to reviewing

#11 @flixos90
5 months ago

In 57153:

Themes: Avoid autoloading the previous theme's theme mods when switching to another theme.

This reduces unnecessarily autoloaded data from inactive themes, which can contribute to slow database performance as part of excessive autoloading.

Props mukesh27, rajinsharwar, igmoweb, joemcgill, swissspidy, westonruter, flixos90.
Fixes #59537.
See #59975.

#12 @flixos90
5 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 57155:

Themes: Clean up inactive themes' theme mods from being autoloaded as part of upcoming 6.5 upgrade routine.

Alongside [57153], this reduces unnecessarily autoloaded data from inactive themes, which can contribute to slow database performance as part of excessive autoloading. This changeset specifically resolves the issue for existing sites.

Props mukesh27, joemcgill, swissspidy, westonruter, flixos90.
Fixes #59975.
See #39537.

Note: See TracTickets for help on using tickets.