Make WordPress Core

Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#58810 closed enhancement (maybelater)

Allow giving priority to a block style for wp_maybe_inline_styles()

Reported by: asafm7's profile asafm7 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Script Loader Keywords:
Focuses: performance Cc:

Description

Use case:

The navigation block style is skipped in wp_maybe_inline_styles(), because, together with smaller style sheets, it exceeds the size thresholds. Unless otherwise inlined, it creates either a render-blocking issue or a CLS issue.

Change History (8)

#1 @westonruter
10 months ago

The minified stylesheet is 16,081 bytes, which is indeed quite close to the 20,000 byte limit for wp_maybe_inline_styles(). I do see on a vanilla install that the only external stylesheet is for the Navigation block's CSS (see Gutenberg source).

Note that while this is a render-blocking issue, it wouldn't be a CLS issue.

I tried increasing the styles_inline_size_limit to 31,422 bytes to allow the navigation stylesheet to be inlined.

Here are median TTFB-LCP over 10 requests with and without network throttling:

Before After Diff
No Throttle 49.05 48.75 -0.6%
Fast 3G 1201.00 1193.85 -0.6%
Slow 3G 4226.5 4223.90 -0.1%

So I don't really see the render blocking being a significant problem here.

#2 @flixos90
10 months ago

I agree with @westonruter here, I don't think there's a CLS issue? The stylesheet is still loaded in <head>, correct? If so, this is only about render-blocking, in which case I don't see a good reason to change the current heuristics, given there's no significant impact from just that.

It would only be a CLS problem if the file was otherwise loaded in the footer, but I don't think that's the case. So maybe this is a wontfix?

#3 @westonruter
10 months ago

As pointed out by @jonoaldersonwp in #core-performance:

IIRC the 20k limit on inlining was deliberately very conservative; I wonder if it might be worth bumping that up. Not a huge impact in this specific case, but if it catches a bunch of minor stuff in other scenarios, it might make a difference.

On a related note, AMP's limit for (inline) CSS was originally 50k but even that was found to be too conservative, and it was increased to 75k. I agree that it would be worthwhile revisiting the 20k threshold for styles_inline_size_limit.

#4 follow-up: @flixos90
10 months ago

@westonruter Sounds good, that's worth considering. Noting though that in AMP that limit also effectively was a limit on total CSS, while here it's only total inline CSS, so the implications are different.

I'd argue that's a separate topic than what the ticket was opened for though, so I think we should discuss that in a separate ticket and close this one.

#5 in reply to: ↑ 4 @westonruter
10 months ago

Replying to flixos90:

Noting though that in AMP that limit also effectively was a limit on total CSS, while here it's only total inline CSS, so the implications are different.

True, but note that external stylesheets for font CDNs were still allowed.

#6 @westonruter
9 months ago

I didn't see this in the description, but the title of this ticket "Allow giving priority to a block style for wp_maybe_inline_styles()" seems to suggest that there be a way to mark certain stylesheets as being more important for inlining than others.

For block themes, it could actually be possible to do this automatically. For example, any stylesheets for blocks located in the header template could be automatically prioritized for inlining, even if that causes them to go above the styles_inline_size_limit. Otherwise, if not automatically, there could be some extra flag when registering a block that its going to most likely be in the header and thus should have its stylesheet prioritized for inlining. But that seems less ideal than doing it programmatically.

Nevertheless, I didn't see any real performance benefit from the inlining. I bet that external stylesheets loaded from the same origin as the page will not a performance problem compared to stylesheets loaded from an external origin (e.g. a CDN), because the open TCP connection to the origin should allow the browser to quickly fetch the stylesheet from the same origin. This should be further tested. If this is the case, then the performance issue in this ticket would be most noticed when a site is trying to improve performance when using a CDN, in which case the performance would actually be degraded.

#7 @westonruter
9 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

This should be further tested.

I tested this by spinning up two identical vanilla sites on InstaWP, sans-external-styles.instawp.xyz and with-external-styles.instawp.xyz, with the latter also having this plugin active:

<?php
/**
 * Plugin Name: Load styles from sans-external-styles.instawp.xyz
 */

add_filter(
        'style_loader_src',
        static function ( $src ) {
                $origin = parse_url( home_url(), PHP_URL_HOST );
                if ( parse_url( $src, PHP_URL_HOST ) === $origin ) {
                        $src = str_replace( "//{$origin}", '//sans-external-styles.instawp.xyz', $src );
                }
                return $src;
        }
);

Without Network Throttling

$ npm run research -- benchmark-web-vitals -u https://sans-external-styles.instawp.xyz/ -n 100 -o csv

URL,https://sans-external-styles.instawp.xyz/
Success Rate,100%
FCP (median),181.2
LCP (median),181.2
TTFB (median),122.6
LCP-TTFB (median),61.4
$ npm run research -- benchmark-web-vitals -u https://with-external-styles.instawp.xyz/ -n 100 -o csv

URL,https://with-external-styles.instawp.xyz/
Success Rate,100%
FCP (median),205.8
LCP (median),205.8
TTFB (median),137.15
LCP-TTFB (median),64.35

Here the site without the external stylesheet has a LCP-TTFB that is 4.58% faster.

With Network Throttling

$ npm run research -- benchmark-web-vitals -u https://sans-external-styles.instawp.xyz/ -n 100 -c "Slow 3G" -o csv

URL,https://sans-external-styles.instawp.xyz/
Success Rate,100%
FCP (median),2250.4
LCP (median),2250.4
TTFB (median),328.45
LCP-TTFB (median),1921.8
$ npm run research -- benchmark-web-vitals -u https://with-external-styles.instawp.xyz/ -n 100 -c "Slow 3G" -o csv

URL,https://with-external-styles.instawp.xyz/
Success Rate,100%
FCP (median),2247.75
LCP (median),2247.75
TTFB (median),126.1
LCP-TTFB (median),2123.05

Here the site without the external stylesheet has a LCP-TTFB that is 9.48% faster.

With All Styles Inlined

Lastly, I re-tested when all stylesheets are inlined such as via the following plugin:

<?php
add_filter( 'styles_inline_size_limit', static function () {
        return 1000000000000;
} );

Without Network Throttling

$ npm run research -- benchmark-web-vitals -u https://with-external-styles.instawp.xyz/ -n 100 -o csv

URL,https://with-external-styles.instawp.xyz/
Success Rate,100%
FCP (median),186.3
LCP (median),188.95
TTFB (median),124.75
LCP-TTFB (median),62

The LCP-TTFB above with the external stylesheet was 64.35ms, and here it is 62ms when inlined. So that is 3.65% faster. However, when the stylesheet was not externalized the LCP-TTFB was 61.4ms, so in this case the LCP-TTFB actually increased by ~1%.

With Network Throttling

$ npm run research -- benchmark-web-vitals -u https://with-external-styles.instawp.xyz/ -n 100 -c "Slow 3G" -o csv

URL,https://with-external-styles.instawp.xyz/
Success Rate,100%
FCP (median),2269.85
LCP (median),2303.45
TTFB (median),120.9
LCP-TTFB (median),2179.85

The LCP-TTFB above with the external stylesheet was 2123.05ms, and here it is 2179.85ms when inlined, which is actually slower but a negligible difference. However, when the stylesheet was not externalized (and not inlined) the LCP-TTFB was 1921.8ms which is 11.84% more than when it was inlined, puzzlingly.


In short, I can confirm that if a block's stylesheet is coming from an external origin (such as via a CDN) then it is definitely better to inline the stylesheet. Nevertheless, I'm confused still why inlining the stylesheets seems actually to be slower than when stylesheets are not external. In any case, loading block stylesheets from external origins is not something that core does. It is the domain of plugins. Therefore, it seems it should it be the responsibility of plugins to inline such stylesheets.

For this reason, I'm going to close this as maybelater.

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


9 months ago

Note: See TracTickets for help on using tickets.