Make WordPress Core

#56815 closed defect (bug) (fixed)

Massive (potential) performance issues within get_default_block_editor_settings

Reported by: janthiel's profile janthiel Owned by: flixos90's profile flixos90
Milestone: 6.1.1 Priority: normal
Severity: major Version: 6.0
Component: Editor Keywords: has-patch commit has-unit-tests
Focuses: multisite, performance Cc:

Description

Hi there,

we currently investigate some performance issues within the frontend of some larger sites.
We figured out that there is a potentially bad design decision within the Block editor causing unnecessary slowdowns within the frontend.

The issue is as the following:
Take a single site within a multisite, quota enabled, with many media files and a large amount. We revisit the already optimized dirsize calculation here which currently is in the best possible shape WP core can deliver - imho.
Now add a Theme with the block editor (theme.json present) and hit a state where the dirsize cache was cleared. After a media upload for example. Or delete the 'dirsize_cache' transient.
Opening the Frontend(!) as a non-loggedin user now becomes super slow as the frontend request triggers the recursive dirsize calculation.

Tracing the issue on trunk leads to the following call chain:

script-loader.php::_wp_theme_json_webfonts_handler
WP_Theme_JSON_Resolver::get_merged_data()
WP_Theme_JSON_Resolver::get_theme_data()
block-editor.php::get_default_block_editor_settings
media.php::wp_max_upload_size()
#filter(upload_size_limit)#
ms-functions.php::upload_size_limit_filter()
ms-functions.php::get_upload_space_available()
ms-function.php::get_space_used()

The latter then triggers the dirsize calculation and does real-time calculation in case the cache isn't there for whatever reasons. Which can take any arbitrary time blocking the frontent.

This callstacks happens on each and every pageload. I do not see why the information about the max_upload_size is required within this context. An unauthenticated request does not need this information.
Before this code was introduced all calls to dirsize / quota relevant features were limited to authenticated wp-admin requests or API requests leading to media upload or deletion. And I believe it should be this way.

My suggestion would be to remove this piece of information from the default editor settings in get_default_block_editor_settings() as a quick fix and only add it to a context where the information is really required.

But I would ask for some more pair of eyes on this who know the reasoning behind this code in the block editor in the first place.

Thanks and best regards,

Jan

Change History (23)

#2 @SergeyBiryukov
20 months ago

  • Milestone changed from Awaiting Review to 6.1

Thanks for the ticket! Moving to 6.1 to get more eyes on this.

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


20 months ago

#4 @audrasjb
20 months ago

  • Summary changed from [Performance][Multisite][Block Editor] Massive (potential) performance issues within get_default_block_editor_settings to Massive (potential) performance issues within get_default_block_editor_settings

#5 @audrasjb
20 months ago

Thanks for raising this issue @janthiel. As per today's bug scrub, we'll keep it in the milestone for now, but it looks like the related code wasn't introduced in 6.1.
Needs further investigation.

#6 @janthiel
20 months ago

@audrasjb This issue is around since the code was added as you discussed in the bug scrub on slack. So no, this was not introduced with 6.1. It is also around with 6.0.x. That's where we found it initially as we profiled WP 6.0.2.
The block-editor.php code was refactored in 6.1 so the call stack is different there than in 6.0. But this doesn't change anything about the issue. It's still present on trunk.

We haven't had any issues with WP 5.x. though.

But the trigger of the issue with us might be as well a combination of our theme (Enfold) adding block support or something like that triggering that code path just recently. Or a massive increase in media files making the dirsize calculation an issue performance-wise and we didn't really notice before.

#7 @desrosj
19 months ago

  • Milestone changed from 6.1 to 6.1.1
  • Version changed from trunk to 6.0

The Version field is for the first version of WordPress affected by a problem. While it does exist in trunk, the important piece of information is where it was introduced. I've updated the field to 6.0 as that's the version you've described above.

While it is unfortunate, this is being raised too late in the release cycle to be addressed in 6.1. The Release Candidate phase is only for addressing regressions introduced during the current release cycle.

But, investigating this is important. So I am moving the ticket to 6.1.1 for continued visibility.

#8 @spacedmonkey
19 months ago

I have this is a duplicate of #56974 or #45171.

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


19 months ago

#10 @Clorith
19 months ago

This seems to be a different issue from #56974, since it focuses on a known heavy function now being called on the front-end of a site.

Although not introduced in 6.1, if a patch can land within the next few days, this could be included in 6.1.1, but if not, it will have to be moved to a future release instead, as 6.1.1 is a short-cycle release.

#11 @janthiel
19 months ago

@Clorith I am happy to provide a patch on short notice if someone has a suggestion on how to actually fix this.
I cannot say where this information is actually required in Gutenberg and thus don't have enough insight on how to fix this.
The goal has to be to remove that call in the Frontend and / or in unauthenticated requests.

Simply adding a check for the upload ability of the current user should be a proper fix - in theory. But that requires knowledge on how that information is used.

Can someone with that knowledge assist?
Question is: Is the upload size used for anything else than actually uploading media in Gutenberg?

Last edited 19 months ago by janthiel (previous) (diff)

This ticket was mentioned in PR #3569 on WordPress/wordpress-develop by JanThiel.


19 months ago
#12

  • Keywords has-patch added; needs-patch removed

Only retrieve upload related information for the theme.json resolver when user can actually upload

Trac ticket: [](https://core.trac.wordpress.org/ticket/56815)

#13 @janthiel
19 months ago

I added a patch suggestion based on the assumption that the max_upload_size is only relevant for calls which can actually upload files. For any other context the max_upload_size should be 0 as in "not allowed to upload anyway".

If this is a proper fix, I am more than happy. If not, this should be a fair starting point for a discussion about why the information would be needed in other contexts as well ;-).

#14 @spacedmonkey
19 months ago

Another workaround is the following.

add_filter('pre_site_option_upload_space_check_disabled', '__return_true');
$max_upload_size = wp_max_upload_size();
remove_filter('pre_site_option_upload_space_check_disabled', '__return_true');

As a workaround, is will hotwire upload_size_limit_filter function ( filter ) to not run the space check.

#15 @mxbclang
19 months ago

  • Owner set to flixos90
  • Status changed from new to assigned

#16 @spacedmonkey
19 months ago

  • Keywords commit added

After core performance chat, the PR #3569 looks good to commit.

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


19 months ago
#17

  • Keywords has-unit-tests added

This is basically the same as #3569, only with a unit test added.

Trac ticket: https://core.trac.wordpress.org/ticket/56815

#18 @flixos90
19 months ago

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

In 54769:

Editor: Improve frontend performance for get_default_block_editor_settings().

The wp_max_upload_size() function can be expensive to call, especially for large sites or multisites. For the frontend usage of get_default_block_editor_settings() knowing the allowed upload size is typically unnecessary.

This changeset adds a condition so that wp_max_upload_size() is only called if the current user can actually upload_files. It keeps the data present when it is actually needed while avoiding the execution overhead when it is not needed.

Props janthiel, Clorith, flixos90, spacedmonkey.
Fixes #56815.

#21 @flixos90
19 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport to 6.1. branch.

#22 @flixos90
19 months ago

  • Keywords 2nd-opinion removed

#23 @flixos90
19 months ago

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

In 54772:

Editor: Improve frontend performance for get_default_block_editor_settings().

The wp_max_upload_size() function can be expensive to call, especially for large sites or multisites. For the frontend usage of get_default_block_editor_settings() knowing the allowed upload size is typically unnecessary.

This changeset adds a condition so that wp_max_upload_size() is only called if the current user can actually upload_files. It keeps the data present when it is actually needed while avoiding the execution overhead when it is not needed.

Props janthiel, Clorith, flixos90, spacedmonkey.
Merges [54769] to the 6.1 branch.
Fixes #56815.

Note: See TracTickets for help on using tickets.