#58321 closed defect (bug) (fixed)
`_load_textdomain_just_in_time()` firing too often for en_US sites
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | 6.2 |
Component: | I18N | Keywords: | has-patch has-unit-tests needs-dev-note |
Focuses: | performance | Cc: |
Description
_load_textdomain_just_in_time
is supposed to run once on the first translation call to see if translations need to be loaded.
This works great on a localized site. However, I found out now that this doesn't work on a bare en_US site.
At first glance, things to improve:
- In
WP_Textdomain_Registry
, change this! empty()
check to anisset()
check, to distinguish between absent (null
) and retrieved values but missing translation files (false
): https://github.com/WordPress/wordpress-develop/blob/f375b68447d392ca7c189ebfadb6b908931d3812/src/wp-includes/class-wp-textdomain-registry.php#L100 This way,_load_textdomain_just_in_time()
bails earlier - In
get_translations_for_domain()
(or maybe elsewhere), set$l10n[ $domain ] = &$noop_translations;
so that_load_textdomain_just_in_time
is only called once. Needs some testing with locale switching to see if that actually works.
Not sure if this is a regression from 6.2, 6.1, or even from 4.6.
Change History (10)
This ticket was mentioned in PR #4463 on WordPress/wordpress-develop by @swissspidy.
21 months ago
#2
- Keywords has-patch has-unit-tests added; needs-patch removed
Fix reported issue by setting Noop_Translations
instances on the $l10n
global.
There is one failing test right now, test_unload_textdomain_non_existent_file
, but it passes in isolation. So probably some other test is not cleaning up.
Help with that test appreciated.
To-do:
- [ ] Fix tests
- [ ] Add new tests covering the new desired behavior
Trac ticket: https://core.trac.wordpress.org/ticket/58321
#3
@
21 months ago
- Priority changed from normal to high
As a member of the Performance team, I am going to set the priority to high
as the major website is in en_US
, and these changes will help improve performance for all of them.
@swissspidy commented on PR #4463:
21 months ago
#7
Committed in https://core.trac.wordpress.org/changeset/55865
#8
@
21 months ago
Tested this change using npm run research benchmark-server-timing -- -n 100 -c 2 -u http://localhost:8889/
using en_US locale.
Before this change:
wp-total
(median): 397.24
After this change:
wp-total
(median): 387.65
So that's a ~2.5% improvement
#9
@
21 months ago
For reference, in my test runs with TT3 and en_US locale, WP_DEBUG
and other debugging constants all set to false
(npm run research -- benchmark-server-timing -u http://localhost:8889 -n 100 -p
), I got:
- 157.24ms with this change, 157.89ms without (~1% faster)
- 157.06ms with this change, 158.02ms without (~1.6% faster)
- 159.08ms with this change, 161.14ms without (~2.3% faster)
While the difference oveerall is small enough to potentially be due to variance between test runs, I consistently see better results with the commit applied. It's a relatively small win, but I think it's safe to say it is a win. 👍
Probably also happens for a localized site where the translation files are missing