Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#58457 closed enhancement (fixed)

Optimize `WP_Theme_JSON::append_to_selector`

Reported by: bor0's profile bor0 Owned by:
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.3
Component: Editor Keywords: has-patch
Focuses: performance Cc:

Description

On a basic WordPress installation, visiting the frontend page shows about 1104 calls to the function WP_Theme_JSON::append_to_selector.

We find a way to optimize the function by bringing the conditional check one level above. While it increases code verbosity, it also improves performance, as can be seen by the attached images.

Attachments (4)

before_58457.png (37.7 KB) - added by bor0 2 years ago.
Performance measurements of the current function
after_58457.png (37.7 KB) - added by bor0 2 years ago.
Performance improvements after the patch - showing nearly a 1% improvement
58457.patch (818 bytes) - added by bor0 2 years ago.
58457.2.patch (728 bytes) - added by bor0 2 years ago.

Download all attachments as: .zip

Change History (13)

@bor0
2 years ago

Performance measurements of the current function

@bor0
2 years ago

Performance improvements after the patch - showing nearly a 1% improvement

@bor0
2 years ago

#1 @bor0
2 years ago

cc @SergeyBiryukov for visibility.

#2 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.3

Nice, thanks!

#3 @spacedmonkey
2 years ago

This change will need to made in Gutenberg and ported back to core.

See this PR that have already been merged into Gutenberg

https://github.com/WordPress/gutenberg/pull/47833
https://github.com/WordPress/gutenberg/pull/50266

These will be backported as part of the WP 6.3 release.

#4 @spacedmonkey
2 years ago

@bor0 Can you rebase your PR now that [55907] is committed. I think performance issue is fixed, but worth another look. Thanks!

@bor0
2 years ago

#5 @bor0
2 years ago

Rebased. Thank you!

#6 @flixos90
2 years ago

@bor0 Thanks for opening this ticket, I hadn't seen it until now. Note that my PR https://github.com/WordPress/wordpress-develop/pull/4380 optimizes the logic around append_to_selector() in a way that no conditions will be needed anymore at all. So if we commit that, we can probably close this ticket here as well.

Also see #58193.

#7 @flixos90
2 years ago

In 55950:

Editor: Introduce WP_Theme_JSON::prepend_to_selector() to improve code quality and performance.

The WP_Theme_JSON::append_to_selector() method was previously used for both appending and prepending which violated the single responsibility principle. It resulted in additional conditionals which also came at a performance cost, particularly because the method is called over 1,000 times during a regular WordPress request.

With the new WP_Theme_JSON::prepend_to_selector() method, there are now two distinct methods for the two distinct purposes. The now useless third parameter on WP_Theme_JSON::append_to_selector() has been removed (rather than deprecated), which is acceptable given that it is a protected method on a class that is not intended for extensions.

Props bor0, costdev, flixos90, isabel_brison, oandregal, spacedmonkey.
Fixes #58193.
See #58457.

#8 @flixos90
2 years ago

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

Indirectly fixed via [55950].

#9 @SergeyBiryukov
2 years ago

  • Component changed from General to Editor
Note: See TracTickets for help on using tickets.