Opened 3 years ago
Last modified 6 days ago
#57548 accepted enhancement
Stop concatenating scripts and stylesheets in wp-admin and retire load-scripts.php and load-styles.php
| Reported by: | azaozz | Owned by: | westonruter |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | Script Loader | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: | performance |
Description
This functionality was added 14 years ago, #8628. At the time it made the loading of wp-admin quite faster by reducing the number of HTTP requests by more than 50%. The downside is that each wp-admin page has it's own set of concatenated scripts and stylesheets making them harder for caching in browser/network caches. Nevertheless the benefits of the reduced number of requests was greater than the insufficient caching.
This changed when HTTP/2 became more popular. Scripts and stylesheets are loaded much more efficiently there and concatenation is generally not needed. This allows for better browser/network caching and for eventual prioritization. In that terms disabling concatenation of scripts and stylesheets in wp-admin would reduce complexity and improve performance in many cases.
Change History (27)
#2
@
3 years ago
I think this is worth pursuing. Once we have a working solution, we should gather some performance comparisons to make a comparison between the two approaches.
This ticket was mentioned in Slack in #core-editor by mamaduka. View the logs.
3 years ago
#4
follow-up:
↓ 8
@
3 years ago
The current implementation (in WordPress 6.3) is currently causing the asset generation and concatenation to fail with an HTTP 503 error if less than 1.5 Gb RAM is assigned to the site. See https://github.com/WordPress/gutenberg/issues/54294 for fuller details.
This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.
2 years ago
#8
in reply to: ↑ 4
@
11 months ago
Replying to markhowellsmead:
The current implementation (in WordPress 6.3) is currently causing the asset generation and concatenation to fail with an HTTP 503 error if less than 1.5 Gb RAM is assigned to the site. See https://github.com/WordPress/gutenberg/issues/54294 for fuller details.
#47322 is a ticket with a simple patch that likely reduces peak memory usage for wp-admin/load-styles.php.
#9
@
11 months ago
My understanding is that there are still benefits from concatenation with HTTP/2, particularly on servers configured with compression.
While it is true that assets can be downloaded in parallel, each file will have it's own compression dictionary. For example, each CSS file will have entries for color, background, font, font-size, etc.
By concatenating the requests WordPress is reducing the number of dictionaries that need to be transferred across the wire. Harry Roberts documents the results in a post on HTTP performance.
Rather than rely on instinct, it would be good to get some data on the benefits for common screens: the dashboard, the post list, the post editor.
As WP is all-or-nothing at the moment with concatenation, another comparison point is to check limiting the number of script in a single concatenated request to N files.
#10
follow-up:
↓ 11
@
11 months ago
@peterwilsoncc We should test this indeed.
You're referring to transferring dictionaries, but Compression Dictionaries aren't implemented by load-scripts.php, right? Also, I don't see that there is even any compression at all (e.g. GZip). Maybe I'm not seeing something.
The additional latency required to boot up PHP to concatenate the scripts should be considered as well. Serving static files would be a much faster TTFB.
We can get some additional benefits by preloading key scripts and styles used on in the editor when the user lands on the dashboard, for example.
#11
in reply to: ↑ 10
@
11 months ago
Replying to westonruter:
@peterwilsoncc We should test this indeed.
You're referring to transferring dictionaries, but Compression Dictionaries aren't implemented by
load-scripts.php, right? Also, I don't see that there is even any compression at all (e.g. GZip). Maybe I'm not seeing something.
Yes, that's what I'm referring to.
WordPress doesn't do anything, I'm thinking of the compression modules used by web servers & CDNs.
On the two sites I can readily use live servers to run experiments with, adding define( 'CONCATENATE_SCRIPTS', true ); increased the transfer size of the CSS while the JS dropped a little.
#12
@
11 months ago
I shared this ticket with some trusted performance folks @sergiomdgomes and @josephscott, I'll summarize their feedback here.
Most results seem to indicate that concatenation remains beneficial in general. It hasn't been made obsolete by HTTP/2.
A few additional links discussing the topic:
There's also "tight mode" discussed in this talk around 11 minutes:
Chrome loads resources in 2 phases. “Tight mode” is the initial phase and constrains loading lower-priority resources until the body is attached to the document (essentially, after all blocking scripts in the head have been executed). In tight mode, low priority resources are only loaded if there are less than 2 in-flight requests at the time that they are discovered.
It seems conclusive that concatenation is usually better for performance. Does anyone have evidence to the contrary?
To move this forward, a different case needs to be made because it doesn't seem like "no performance benefit from concatenation" is valid.
Do the benefits of not concatenating outweigs the drawbacks of degraded performance. Or in the contexts where concatenation is used (like the customizer view) is degraded performance irrelevant? Or is concatenation so buggy that it doesn't make sense to support it?
#13
@
11 months ago
- Focuses performance added
Thank you, @jonsurrell!
Sorry for not joining the discussion earlier, but I'd like to add a few comments inline now that I'm here :)
Browser module loading - can we stop bundling yet?
I just wanted to add a note that my article is really old, and most of the findings may no longer apply. Still, it mentions the sort of issues one can reasonably expect.
In general, I haven't seen conclusive evidence that concatenation no longer has a benefit.
As @peterwilsoncc mentions, the "small files" compression problem is still relevant, and there isn't a cross-browser solution to address it. Even though WordPress doesn't handle compression itself (whether gzip or brotli), the expectation is that a competent host will enable it at the HTTP server level, and that it will have a meaningful impact on performance, so it's something we do need to consider.
The additional latency required to boot up PHP to concatenate the scripts should be considered as well. Serving static files would be a much faster TTFB.
This would be good to test, but we have to remember that the WordPress ecosystem is extremely heterogenous, and it's difficult to make broad assertions that are valid everywhere.
In this case, since we're thinking outside of WordPress and at the level of serving infrastructure, we have to consider the impact of edge caches as well. It's reasonable to assume that the concatenated files would also be edge cached, potentially negating the effect of any boot-up cost in concatenation at scale.
We can get some additional benefits by preloading key scripts and styles used on in the editor when the user lands on the dashboard, for example.
Preloading is a rather forceful technique that should be used with extreme caution, as it has the potential to make things worse. It's generally preferable to leave prioritisation to the browser, and at most provide hints in the form of fetchpriority or by rearranging things so that the resource is discovered earlier.
In any case, if there is any benefit to be had here, it may be worth exploring independently of concatenation.
Don't get me wrong, there are definitely some downsides to concatenation. Beyond the security issues mentioned, it makes debugging harder, and it can worsen problems with scripts, in that script-breaking bugs now break all the concatenated scripts, not just the one script.
But in terms of performance, the evidence I've seen so far is that there's still an improvement.
This ticket was mentioned in Slack in #core by welcher. View the logs.
10 months ago
#15
@
10 months ago
- Milestone 6.9 → Future Release
With 6.9 beta 1 happening in ~2 weeks and this ticket still be discussed, I am going to move this to Future Release.
#16
@
9 months ago
Related: #58302 (Deprecate and disable the unused compression_test() and wp_ajax_wp_compression_test())
This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.
7 months ago
This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.
7 months ago
#20
@
6 months ago
- Milestone 7.0 → Future Release
- Owner set to
- Status new → accepted
I haven't gotten around to this, so I'm punting given we're a couple weeks from beta1. But I'm self-assigning as a reminder to investigate.
#21
@
5 months ago
Coming in from #64087 as the ghost of WordPress past.
Here's a good reason to get rid of this system: It's riddled with PHP warnings that have been intentionally suppressed.
At the top of load-styles.php and load-scripts.php is the following code:
/*
* The error_reporting() function can be disabled in php.ini. On systems where that is the case,
* it's best to add a dummy function to the wp-config.php file, but as this call to the function
* is run prior to wp-config.php loading, it is wrapped in a function_exists() check.
*/
if ( function_exists( 'error_reporting' ) ) {
/*
* Disable error reporting.
*
* Set this to error_reporting( -1 ) for debugging.
*/
error_reporting( 0 );
}
It's there because without it, any call to load-styles.php will generate multiple warnings, especially because the noop.php zeroes out all filters to return null, which triggers PHP8 warnings for things like file_exists() that don't accept null. Thanks to @westonruter for helping me figure that part out.
I stumbled on to this weird detail because on our live server we have an alternate error handler that doesn't respect error_reporting() and effectively always runs in E_ALL, so the warnings were being triggered after we updated to PHP 8.
As I said in the other ticket, the particular warnings I noticed don't even seem worth fixing, because the whole system assumes there are warnings/errors and intentionally ignores them.
If there's no great argument to keep this system, I vote to nuke it from space. I've been running in define( 'CONCATENATE_SCRIPTS', false ); mode for months now and it works fine and resolves all the warnings.
This ticket was mentioned in PR #11577 on WordPress/wordpress-develop by @sukhendu2002.
3 months ago
#22
- Keywords has-patch has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/57548
## Use of AI Tools
#23
@
3 weeks ago
- Milestone Future Release → 7.1
Another issue has been raised regarding this: #65515.
When a script is in strict mode (where "use strict" appears at the beginning of the script), and this script is the first to get concatenated, then all remaining concatenated scripts are put into strict mode. This causes errors in ThickBox and potentially other scripts.
Conversely, if we worked around this and forced the concatenated script to be in "sloppy mode", then this can cause other errors because there are semantic differences, as pointed out by @siliconforks.
I think the only reasonable way out of all this is to deprecate and remove concatenation. We can use an alternate strategy for improving performance, such as preloading the individual scripts and styles. By preloading individual assets, they'll be better cached given that they'll be outside of the current concatenation bundles.
This ticket was mentioned in Slack in #meta by westonruter. View the logs.
3 weeks ago
#26
@
6 days ago
- Milestone 7.1 → 7.2
Punting this because we have a workaround for the most recent issue identified above.
For this to move forward, we need to not only remove the concatenation logic but also add the appropriate preload links (e.g. from the login screen) so that when a user lands on the dashboard and post editor they'll have a primed cache.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
And I will add that this proposal will reduce the attack perimeter in terms of WordPress security.
Many security plugins propose to reinforce the parameters of calls to these 2 URLs, because possible source of DDOS attack.