WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 5 days ago

#54447 new enhancement

Site Icons on My Sites in multisite network causing performance issue

Reported by: wslyhbb Owned by:
Milestone: 6.0 Priority: normal
Severity: normal Version: 5.8
Component: Toolbar Keywords: has-patch
Focuses: multisite, performance Cc:

Description (last modified by sabernhardt)

WordPress 5.8 added displaying the site icon on the My Sites menu. When a site has an icon, it takes 0.05 seconds per get_site_icon_url, and there are three of them per site, for a total of 0.15 seconds per site. When a multisite has hundreds of sites and 100 hundred of them have site icons, this adds up and adds 15 seconds to the page load time.

The change causing performance issues: r50834

Change History (11)

#1 @sabernhardt
6 weeks ago

  • Component changed from Administration to Toolbar
  • Description modified (diff)
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement

Hi and welcome to Trac!

Thanks for the report. An early proposal on #46657 used a filter, and it still could have one to check whether the site icons should be included. Perhaps the filter could be something like this:

if ( apply_filters( 'show_site_icons_in_toolbar', true ) && has_site_icon() ) {

This ticket was mentioned in PR #2060 on WordPress/wordpress-develop by wslyhbb.


5 weeks ago

  • Keywords has-patch added; needs-patch removed

This avoids performance issue caused by having to get the site icon three times, which has a huge negative impact in multisite networks.

Trac ticket: #54447

#3 @prbot
5 weeks ago

lkraav commented on PR #2060:

Can the icons be lazy loaded instead?

I know core is not a big fan of adding arbitrary filters all over the place.

#4 @prbot
5 weeks ago

sabernhardt commented on PR #2060:

Thanks for adding a patch!

Lazy loading is a good idea, with or without the filter. The code probably should check for lazy preferences—in this context—as it does for the legacy RSS widget's feed icon.

$blavatar = sprintf(
	'<img class="blavatar" src="%s" srcset="%s 2x" alt="" width="16" height="16"%s />',
	esc_url( get_site_icon_url( 16 ) ),
	esc_url( get_site_icon_url( 32 ) ),
	( wp_lazy_loading_enabled( 'img', 'wp_toolbar_site_icon' ) ? ' loading="lazy"' : '' )
);

If a filter is added, it could be better placed before the foreach loop, plus it would need a PHP docblock to explain it.

#5 @sabernhardt
5 weeks ago

  • Milestone changed from Future Release to 6.0

#6 @prbot
9 days ago

kebbet commented on PR #2060:

Something like this?

`

$show_site_icons = true;

/

  • Filters whether to show the site icons in toolbar. *
  • Returning false to this hook is the recommended way to hide site icons in the toolbar.
  • A truthy return may have negative performance impact on large multisites. *
  • @since 6.0.0 *
  • @param bool $show_site_icons Whether site icon should be shown in the toolbar. Default true. */

$show_site_icons = apply_filters( 'show_site_icons_in_toolbar', $show_site_icons );

foreach ( (array) $wp_admin_bar->user->blogs as $blog ) {

switch_to_blog( $blog->userblog_id );

if ( true === $show_site_icons && has_site_icon() ) {

$blavatar = sprintf(

'<img class="blavatar" src="%1$s" srcset="%2$s 2x" alt="" width="16" height="16"%3$s />',
esc_url( get_site_icon_url( 16 ) ),
esc_url( get_site_icon_url( 32 ) ),
( wp_lazy_loading_enabled( 'img', 'site_icon_in_toolbar' ) ? ' loading="lazy"' : )

);

} else {

`

#7 @prbot
9 days ago

sabernhardt commented on PR #2060:

@kebbet Yes, something like that :)

Because the return value should be boolean, I think the filter could set the true default within the same line (similar to rest_jsonp_enabled and others).

$show_site_icons = apply_filters( 'show_site_icons_in_toolbar', true );

Also, numbering the placeholders is usually good, but in this case it might be a little confusing with %2$s 2x. Feel free to disagree with me :)

This ticket was mentioned in PR #2171 on WordPress/wordpress-develop by kebbet.


9 days ago

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

Alternative approach to PR #2060

#10 @prbot
5 days ago

kebbet commented on PR #2176:

The two reverted changes are reported upstream in https://github.com/WordPress/gutenberg/pull/38048

Note: See TracTickets for help on using tickets.