Make WordPress Core

Opened 20 months ago

Closed 17 months ago

Last modified 17 months ago

#58682 closed defect (bug) (fixed)

wp_trim_excerpt parses and renders blocks twice

Reported by: spacedmonkey's profile spacedmonkey Owned by: flixos90's profile flixos90
Milestone: 6.4 Priority: normal
Severity: normal Version: 5.0
Component: Posts, Post Types Keywords: has-patch dev-feedback has-unit-tests commit
Focuses: performance Cc:

Description

The function wp_trim_excerpt can called when a excerpt is not saved in the database and use the post content to fill the missing field. To do this, it strips unsupported blocks out using excerpt_remove_blocks. However, as the filter the_content is applied to the text, this means that blocks and parsed again.

wp_trim_excerpt should be improved not to call parse block multiple files and only call when there are blocks present.

Change History (25)

This ticket was mentioned in PR #4767 on WordPress/wordpress-develop by @spacedmonkey.


20 months ago
#1

  • Keywords has-patch added

#2 @spacedmonkey
20 months ago

Tested with 1000 runs on 2017 theme with theme data.

Trunk PR
Response Time (median) 141.55 138.13
wp-load-alloptions-query (median) 1.49 1.49
wp-before-template (median) 28.63 28.03
wp-before-template-db-queries (median) 3.56 3.48
wp-template (median) 106.35 103.12
wp-total (median) 135.13 131.38
wp-template-db-queries (median) 17.18 16.38

Blackfire profile - https://blackfire.io/profiles/compare/8a452033-8a00-4552-ae1e-337b565737d5/graph

#3 @joemcgill
20 months ago

  • Component changed from Editor to Posts, Post Types
  • Keywords 2nd-opinion added
  • Priority changed from high to normal

I agree that there's definitely something worth addressing here, but I'm not sure if this is the right place to address it. Particularly when thinking about block themes, the_content filter seems to be potentially called at multiple different levels, creating unexpected behavior or unnecessarily duplicating the same logic. This also became apparent in #56588. Prior to block themes, this filter was only ever intended to be called once per post being displayed on a page (e.g., once on a singular template, and once for every post shown in a post list on home or archive pages).

I think it would worth an investigation into whether this filter behavior could be improved to avoid this entire class of issues in a future release. To start, it would be helpful to document the current behavior for when blocks are parsed and the_content filter is run.

Resetting the priority as "normal" for now, since this isn't a release priority at this point. We may decide to move it back to "high" while determining goals for 6.4.

#4 @spacedmonkey
20 months ago

  • Milestone changed from Future Release to 6.4
  • Owner set to spacedmonkey
  • Status changed from new to assigned

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


18 months ago

#6 @oglekler
18 months ago

  • Keywords dev-feedback added

This ticket was discussed during a bug scrub.

@spacedmonkey and @joemcgill, can you please investigate what is the solution should look like and if this can and should be covered with unit tests as well.

Add props to @mukesh27

#7 @spacedmonkey
18 months ago

@mukesh27 do you want to take this ticket over?

#8 @spacedmonkey
17 months ago

  • Owner spacedmonkey deleted

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


17 months ago

This ticket was mentioned in PR #5183 on WordPress/wordpress-develop by @thekt12.


17 months ago
#10

  • Keywords has-unit-tests added

#11 @spacedmonkey
17 months ago

  • Owner set to thekt12

This ticket was mentioned in Slack in #core-performance by thekt12. View the logs.


17 months ago

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


17 months ago

@mukesh27 commented on PR #5183:


17 months ago
#14

@kt-12, I'm curious to know which theme you use in your benchmark testing. Could you please share the data for both block and classic themes?

This ticket was mentioned in Slack in #core-performance by thekt12. View the logs.


17 months ago

#17 @flixos90
17 months ago

  • Keywords commit added; 2nd-opinion removed
  • Owner changed from thekt12 to flixos90
  • Status changed from assigned to reviewing

@flixos90 commented on PR #4767:


17 months ago
#18

Closing in favor of #5183.

#19 @spacedmonkey
17 months ago

@flixos90 Do you plan to commit this? Is not, I am more than happy to commit this one.

#20 @flixos90
17 months ago

@spacedmonkey Yes, I'm on it :)

#21 @flixos90
17 months ago

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

In 56560:

Posts, Post Types: Avoid unnecessarily parsing blocks twice in wp_trim_excerpt().

All blocks relevant for the excerpt are already being parsed in excerpt_remove_blocks(). Therefore running do_blocks() on the post content only to create the excerpt is unnecessary and wasteful from a performance perspective.

Props thekt12, spacedmonkey, mukesh27, joemcgill.
Fixes #58682.

#24 @SergeyBiryukov
17 months ago

In 56561:

Docs: Fix typo in a comment in wp_trim_excerpt().

Includes removing redundant @covers tags. There is already an existing annotation for the whole test class, following the PHPUnit recommendation:

This annotation can be added to the docblock of the test class or the individual test methods. The recommended way is to add the annotation to the docblock of the test class, not to the docblock of the test methods.

Follow-up to [56560].

See #58682.

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


17 months ago

Note: See TracTickets for help on using tickets.