WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#28580 assigned enhancement

Speed up customizer by lazy-loading panels/sections/controls and settings as needed

Reported by: westonruter Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: needs-patch
Focuses: javascript, rest-api Cc:

Description (last modified by westonruter)

As more and more controls and settings get added to the customizer, it's performance is bound to take an increased hit, especially for themes that do not implement postMessage transport for controls. The Widget Customizer work showed that settings and controls can be created dynamically after the customizer loads. From the start we should consider dynamically instantiating controls, even entire sections, at the moment that they are accessed. As part of this, the JS should be cleaned up to make it a bit easier to create new controls. Right now it involves a bit of DOM manipulation to attach controls to the right section. Likewise, there is no JS API for Customize sections (accordions).

See also #27406, which introduces the ability to group sections. These would need a JS API as well.

The Customizer preview should postMessage the panels, sections, controls that it expects up to the Customizer pane and they can be created on demand; see #28709.

Attachments (3)

28580.diff (1.5 KB) - added by westonruter 3 years ago.
https://github.com/x-team/wordpress-develop/commit/4af54b5ebbea737ee1e1413b47b13268008078f1
28580.2.diff (1.8 KB) - added by celloexpressions 2 years ago.
Don't embed controls until the containing section is expanded for the first time. Works for both traditional and JS-templated controls.
28580.3.diff (894 bytes) - added by westonruter 2 years ago.
Defer ThemeControl.renderContent until containing section is expanded

Download all attachments as: .zip

Change History (36)

#1 @westonruter
3 years ago

If the specific performance problem is that the preview is taking long and longer to refresh as new settings are added, then an easy way to speed up performance would be to just keep track of each setting's dirty state, and only include settings in the preview POST request if they get mutated.

#2 @westonruter
3 years ago

@mattwiebe notes specifically that his concern is:

Slow to load. Time to the Customizer's ready event.

So tracking mutations to settings probably wouldn't help here.

#3 @celloexpressions
3 years ago

Depending on how fast the lazy-loading ends up being, we should probably do something like the following:

  • Load top-level section and panel headings from PHP.
  • Load contents of top-level sections and section headings in panels via ajax during the load process.
  • Load sub-section contents only when a panel is entered.

#4 @westonruter
3 years ago

  • Keywords has-patch added; needs-patch removed

Currently the Customizer posts all settings with every request to the Preview, and this can add a lot of time with each request if there are a lot of settings registered (e.g. when there are a lot of sidebars registered).

In 28580.diff I've greatly improved the performance for loading the preview, including the delay for refreshing the preview, by changing the Ajax request to only include data for settings that have actually been changed (those which are dirty).

(I think the _dirty property should be changed to perhaps be a dirty Value which can be watched itself.)

#5 @westonruter
3 years ago

  • Milestone changed from Future Release to 4.1

#6 follow-up: @celloexpressions
3 years ago

+1 for 28580.diff, although I'm not sure that this is the right ticket for it. I think we'll definitely want to lazy-load controls and settings initially at some point as this ticket proposes (which #29572 starts to build a framework for), but that's very doubtfully 4.1 material, and in the meantime we should only post dirty settings to the preview with this patch.

#7 in reply to: ↑ 6 ; follow-up: @westonruter
3 years ago

Replying to celloexpressions:

+1 for 28580.diff, although I'm not sure that this is the right ticket for it. I think we'll definitely want to lazy-load controls and settings initially at some point as this ticket proposes (which #29572 starts to build a framework for), but that's very doubtfully 4.1 material, and in the meantime we should only post dirty settings to the preview with this patch.

I think maybe this ticket should be rewritten to just be about improving performance, because now I'm not sure that lazy-loading is really the core need.

#8 in reply to: ↑ 7 @ocean90
3 years ago

Replying to westonruter:

I think maybe this ticket should be rewritten

I prefer a new ticket for 28580.diff so that we can get it in.

#9 @celloexpressions
3 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 4.1 to Future Release

#29983 for now. We can always wontfix this if we decide it's not needed.

#10 @westonruter
3 years ago

  • Description modified (diff)

#11 @ocean90
3 years ago

In 30102:

Improve/introduce Customizer JavaScript models for Controls, Sections, and Panels.

  • Introduce models for panels and sections.
  • Introduce API to expand and focus a control, section or panel.
  • Allow deep-linking to panels, sections, and controls inside of the Customizer.
  • Clean up accordion.js, removing all Customizer-specific logic.
  • Add initial unit tests for wp.customize.Class in customize-base.js.

https://make.wordpress.org/core/2014/10/27/toward-a-complete-javascript-api-for-the-customizer/ provides an overview of how to use the JavaScript API.

props westonruter, celloexpressions, ryankienstra.
see #28032, #28579, #28580, #28650, #28709, #29758.
fixes #29529.

#12 @kuus
2 years ago

Hello, I was just wondering if is there still interest in this ticket. I'm definitely interested.
Especially in this idea:

Load sub-section contents only when a panel is entered.

To keep a light DOM tree, would be great to actually attach and detach the DOM of a control only when its section is open / closed. You think is possible with the current API (4.1) to get this behavior?

#13 @westonruter
2 years ago

Yes, it should be possible.

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


2 years ago

@celloexpressions
2 years ago

Don't embed controls until the containing section is expanded for the first time. Works for both traditional and JS-templated controls.

#15 follow-up: @celloexpressions
2 years ago

  • Keywords has-patch added; needs-patch removed

28580.2.diff is a functional proof of concept for waiting to insert controls into the DOM until their containing section is opened for the first time. Needs refinement around edge cases, and fallbacks to ensure that a control is always embedded in the DOM if it's supposed to be visible (need to check what happens with custom sections that override onChangeExpanded and behavior if an unembedded control is moved to a previously-content-embedded section). But potentially a big performance improvement especially with things like the Customizer Theme Switcher.

#16 in reply to: ↑ 15 @westonruter
2 years ago

Replying to celloexpressions:

28580.2.diff

In this patch, autofocusing on controls is broken, for instance: /wp-admin/customize.php?autofocus[control]=blogname.

@westonruter
2 years ago

Defer ThemeControl.renderContent until containing section is expanded

#17 @westonruter
2 years ago

@celloexpressions: in 28580.3.diff the ThemeControl's renderContent method gets deferred until the containing section is expanded, meaning that the screenshots won't load until the Theme Switcher panel is opened. Given the lateness of the release cycle, I think such a Theme Switcher-specific loading optimization should be what is in scope. /cc @mattwiebe

#18 @westonruter
2 years ago

Let's continue the discussion regarding Theme Switcher performance improvements to the ticket specific to it, as it is the key one for 4.2: #31793

#19 follow-up: @westonruter
22 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Future Release to 4.4
  • Owner set to westonruter
  • Status changed from new to accepted

Customizer performance is going to be a key focus for 4.4

#20 in reply to: ↑ 19 @adamsilverstein
22 months ago

Yea! Very excited to work on improving performance in the customizer!

Replying to westonruter:

Customizer performance is going to be a key focus for 4.4

#21 @westonruter
21 months ago

For nav menu items and widget instances, it's possible that we could postpone the fetching of the settings data until they are actually used. When a nav menu section or widget area (sidebar) section is expanded into view, this could kick off an Ajax request to load the settings data for the given menu items and widget instances. The request for widgets would also need to fetch the markup for each of the controls, since widgets still rely on PHP for all validation and form generation. (Aside, I'm proposing in #33507 a new implementation of widget controls that rely entirely on JS for form generation and data validation in the UI.)

This ticket was mentioned in Slack in #core by westonruter. View the logs.


21 months ago

#23 @westonruter
21 months ago

  • Milestone changed from 4.4 to Future Release

I've identified a couple more impactful wins rather than full lazy-loading of setting and controls. I'm focusing on these for 4.4 instead:

#33898: Reduce Customizer peak memory usage by JSON-encoding settings and controls separately
#33901: Defer embedding widget controls to improve DOM performance

#24 @westonruter
20 months ago

I consider #33052 to be a dependency for lazy-loading widget settings, since the initial expanding of the widgets panel would be trigger for lazy-loading the settings and controls. We'd also need to lazy-load widgets when the widgets panel, a widget area section, or a widget control is autofocused; likewise, the nav menu items would need to get loaded when expanding the nav menu panel.

Last edited 20 months ago by westonruter (previous) (diff)

This ticket was mentioned in Slack in #core by westonruter. View the logs.


20 months ago

This ticket was mentioned in Slack in #core by westonruter. View the logs.


20 months ago

#27 @westonruter
15 months ago

Lazy-loading widgets have some special performance considerations of their own. At the moment, when a pre-existing widget is instantiated dynamically, an update-widget Ajax request is kicked off to obtain the rendered form. This issue will be irrelevant to JS-driven widgets (#33507), when they are implemented, because the widget control will be constructed entirely client-side from a JS template. For legacy widgets that aren't JS-driven, we should batch together the update-widget Ajax calls (similar to how selective refresh requests are now batched in 4.5) to prevent slamming the server with dozens (or hundreds) of HTTP requests for (very) large sidebars.

Last edited 15 months ago by westonruter (previous) (diff)

#28 @westonruter
6 months ago

See also discussion on #38889 related to dynamically fetching settings for controls in a given section when it is expanded.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


5 months ago

#30 @westonruter
5 months ago

  • Focuses rest-api added
  • Owner westonruter deleted
  • Status changed from accepted to assigned
  • Summary changed from Speed up customizer by lazy-loading controls and settings as needed to Speed up customizer by lazy-loading panels/sections/controls and settings as needed

I suggest that this ticket be a key use case for implementing REST API endpoints for accessing panels, sections, controls, and customize settings on demand rather than having them all exported up front during the initial customizer load.

#31 @celloexpressions
4 months ago

We need to be careful not to make the experience worse. Often when implementing single page apps or single page JS-driven sites that rely heavily on Ajax requests, the experience actually feels slower than a normal pageload because users don't get the traditional browser feedback that a new page is loading.

I remain extremely skeptical of loading controls "as-needed" since users could easily navigate to a section and then end up waiting for something to load. If we go that route, we'll need to benchmark the load time for accessing individual elements versus loading everything up front and prove that the change is a usability improvement for a majority of cases. Individual Ajax calls can be very slow in many places regardless of how much data we're loading, to the extend that loading everything at once is often better for UX.

Rather than lazy-loading everything, we could have much more impactful changes if the lazy-loading is used in more refined situations such as:

  • Loading everything via Ajax when the customizer is bootstrapped from the front end, rather than loading pieces more granularly
  • Menu item and widget controls and settings could be loaded when the associated panels are first expanded, so that they don't impact initial load but are ready by the time a user navigates (note that menu item controls aren't rendered to the DOM until the associated menu section is expanded, which is a great balance in terms of speed/performance and scales easily to hundreds of menu items).
  • Theme controls don't need to be loaded until the themes panel is expanded (which is how #37661 handles things).
  • It may be reasonable to wait to load panel controls until the panel is expanded, but you'd want the sections there already.

Generally, my concern is that "just-in-time" loading will really end up being "just-too-late" and usability would suffer as a result. We also need to think about how this would impact things like visible edit shortcuts, partials, and autofocusing. Loading things on demand sounds shiny but isn't necessarily the best approach for users (Calypso suffers horrifically here on poor internet connections).

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


3 months ago

#33 @westonruter
3 months ago

Next step for this is a feature plugin to prototype and user test.

Note: See TracTickets for help on using tickets.