Make WordPress Core

Opened 13 months ago

Closed 12 months ago

Last modified 11 months 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 13 months ago.
Performance measurements of the current function
after_58457.png (37.7 KB) - added by bor0 13 months ago.
Performance improvements after the patch - showing nearly a 1% improvement
58457.patch (818 bytes) - added by bor0 13 months ago.
58457.2.patch (728 bytes) - added by bor0 12 months ago.

Download all attachments as: .zip

Change History (13)

@bor0
13 months ago

Performance measurements of the current function

@bor0
13 months ago

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

@bor0
13 months ago

#1 @bor0
13 months ago

cc @SergeyBiryukov for visibility.

#2 @SergeyBiryukov
13 months ago

  • Milestone changed from Awaiting Review to 6.3

Nice, thanks!

#3 @spacedmonkey
13 months 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
12 months ago

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

@bor0
12 months ago

#5 @bor0
12 months ago

Rebased. Thank you!

#6 @flixos90
12 months 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
12 months 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
12 months ago

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

Indirectly fixed via [55950].

#9 @SergeyBiryukov
11 months ago

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