WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 7 months ago

#42558 accepted enhancement

Reduce the number of capability checks performed by the Customizer

Reported by: johnbillion Owned by: johnbillion
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch needs-testing
Focuses: performance Cc:

Description

On a vanilla installation of WordPress running the Twenty Seventeen theme, the Customizer performs over 400 user capability checks via current_user_can().

The majority of these are duplicate checks performed for each customizer setting. This includes:

  • ~140 calls to current_user_can( 'customize' )
  • ~220 calls to current_user_can( 'edit_theme_options' )
  • ~20 calls to current_user_can( 'manage_options' )

The main offenders are:

  • WP_Customize_Manager->unsanitized_post_values()
  • WP_Customize_Setting->check_capabilities()
  • WP_Customize_Section->check_capabilities()
  • WP_Customize_Panel->check_capabilities()

Calls to current_user_can() are quite cheap, but they're certainly not free, especially on sites which have filters hooked onto the map_meta_cap and user_has_cap filters, and sites with extra Customizer controls registered.

With a static property cache in each of the WP_Customize_Manager, WP_Customize_Section, and WP_Customize_Setting classes, the 400 capability checks are reduced to fewer than 30.

Patch coming up soon, after some more testing.

Attachments (1)

42558.patch (4.7 KB) - added by johnbillion 7 months ago.

Download all attachments as: .zip

Change History (9)

#1 @johnbillion
7 months ago

  • Owner set to johnbillion
  • Status changed from new to accepted

#2 @westonruter
7 months ago

Related: a setting's sanitize and validate methods currently get called every time that its dirty value is read. If a given changeset (post) value for a setting hasn't changed, then the value returned should be whatever was previously sanitized/validated. This could yield a much higher performance improvement than reducing the cap checks. See #41271.

Some previous work on caching was done in #32103.

#3 @westonruter
7 months ago

  • Milestone changed from Awaiting Review to 5.0

@johnbillion
7 months ago

#4 @johnbillion
7 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

42558.patch is a first pass at a patch.

#5 @westonruter
7 months ago

@johnbillion why static?

#6 @johnbillion
7 months ago

The property is static so its value is shared between instances. This means the value is set once per class instead of once per instance.

#7 @westonruter
7 months ago

@johnbillion yes I know, but use it here? None (almost) of the other member vars are static. Using static makes it harder to unit test. Maybe too there also there could be a case where you want to instantiate WP_Customize_Manager and other classes later with another current user active, but the the caps in the static var could leak to other such Customizer sessions.

#8 @westonruter
7 months ago

What if there was a new $wp_customize->current_user_can() method that all of the customizer classes could then call as part of their manager? Then they wouldn't need to store their own caches of the cap checks, as the manager could have common set shared by all for that instance.

Note: See TracTickets for help on using tickets.