WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#33901 closed enhancement (fixed)

Defer embedding Customizer widget controls to improve DOM performance

Reported by: westonruter Owned by: westonruter
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.9
Component: Customize Keywords: has-patch commit
Focuses: javascript, performance Cc:
PR Number:

Description

In working on a site with a lot of widgets in the Customizer, where these widgets are heavy with lots of inputs, I've found that the Customizer locks up at DOM ready for quite a while as it tries to embed all of the widget controls into the DOM. In a test where I have 100 widgets that each have ~300 inputs each, the Customizer took 40 seconds to finish rendering the page (to allow the user to click around) even it only took PHP half a second to send the HTML for the page.

For the Menu Customizer feature merged in 4.3, there are a lot of nav menu item controls that get added to the Customizer and this introduced a very noticeable performance hit. This was addressed by only embedding the wrapper li element when the Customizer first loaded, and then to embed the rest of the nav menu item control with all of its UI once the containing menu section was expanded.

We can take this same approach for widgets. First we can defer the embedding of the widget control accordion until the sidebar section is expanded. But then we can also defer the embedding of the actual contents of the widget form until the control itself is expanded.

I have developed a proof of concept for this in a feature plugin: https://github.com/xwp/wp-customize-widgets-plus/pull/32

With the feature plugin active, what used to take 40 seconds to render the page with 100 widgets containing 300 inputs each, it now takes only 3 seconds and there is no perceived browser lockup in the UI.

Attachments (5)

Change History (18)

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


4 years ago

#2 @westonruter
4 years ago

I ran Chrome's memory usage timeline before/after the feature plugin was activated.

Before:

  • Peak JS heap size: 53MB
  • DOM node count: ~300k
  • Scripting time: 29.3 seconds
  • 336MB used by Chrome process

After:

  • Peak JS heap size: 38MB (28% savings)
  • DOM node count: ~22k (93% savings)
  • Scripting time: 1.13 seconds (96% savings)
  • 223MB used by Chrome process (34% savings)
Last edited 4 years ago by westonruter (previous) (diff)

#3 @westonruter
4 years ago

  • Keywords has-patch needs-testing added
  • Owner set to westonruter
  • Status changed from new to accepted
  • Summary changed from Defer embedding widget controls to improve DOM performance to Defer embedding Customizer widget controls to improve DOM performance
  • Version set to 3.9

#4 @celloexpressions
4 years ago

This is looking great. Definitely a good call to defer embedding these controls, even without them by JS-driven.

#5 @westonruter
4 years ago

33901.3.diff finally adds QUnit test fixtures for widgets in the Customizer, as well as tests specific to the changes introduced here.

If anyone could test out the changes and confirm that the speed improvements are present and regressions are absent, remove the needs-testing and I'll commit!

Note that the Customize Widgets Plus feature plugin implements the features in this ticket so the performance improvements can be had on 4.3 now: https://github.com/xwp/wp-customize-widgets-plus/pull/32

#6 @westonruter
4 years ago

  • Focuses javascript added

#7 @westonruter
4 years ago

Could someone do a sanity check for the patch and verify all is working as advertised? I'd like to get this committed tomorrow morning. /cc @valendesigns @celloexpressions @voldemortensen

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


4 years ago

#9 @celloexpressions
4 years ago

I don't have time to test the patch, but the code looks good to me.

#10 @westonruter
4 years ago

  • Keywords commit added; needs-testing removed

The QA team for one of my client projects tested the changes in the feature plugin, and it checked out.

#11 @westonruter
4 years ago

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

In 34563:

Customizer: Defer embedding widget controls to improve DOM performance and initial load time.

The Menu Customizer feature includes a performance technique whereby the controls for nav menu items are only embedded into the DOM once the containing menu section is expanded. This commit implements the same DOM deferral for widgets but goes a step further than just embedding the controls once the widget area's Customizer section is expanded: it also defers the embedding of the widget control's form until the widget is expanded, at which point the widget-added event also fires to allow any additional widget initialization to be done. The deferred DOM embedding can speed up initial load time by 10x or more. This DOM deferral also yields a reduction in overall memory usage in the browser process.

Includes changes to wp_widget_control() to facilitate separating out the widget form from the surrounding accordion container; also includes unit tests for this previously-untested function. Also included are initial QUnit tests (finally) for widgets in the Customizer.

Fixes #33901.

#12 @westonruter
4 years ago

In 34883:

Customizer: Ensure that wp.customize.Widgets.savedWidgetIds is defined up front.

In 4.3 the widget controls were fully initialized up front along with the sidebar controls. The sidebar control depended (unnecessarily) on the widget control to ensure that wp.customize.Widgets.savedWidgetIds was defined. So after [34563] there could be a situation where the widgets are added/removed from a sidebar before their controls are initialized (if the sidebar section is never expanded), resulting in an error attempting to get a property off of an undefined value. So this change does the right thing and defines savedWidgetIds up front.

Also changes the savedWidgetIds variable type from an array to an object, to match how it is used as a dictionary lookup.

See #33901.

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


4 years ago

Note: See TracTickets for help on using tickets.