Make WordPress Core

Opened 3 weeks ago

Closed 4 days ago

Last modified 3 days ago

#58321 closed defect (bug) (fixed)

`_load_textdomain_just_in_time()` firing too often for en_US sites

Reported by: swissspidy's profile swissspidy Owned by: swissspidy's profile swissspidy
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.2
Component: I18N Keywords: has-patch has-unit-tests
Focuses: performance Cc:


_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:

  1. In WP_Textdomain_Registry, change this ! empty() check to an isset() check, to distinguish between absent (null) and retrieved values but missing translation files (false): This way, _load_textdomain_just_in_time() bails earlier
  2. 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 (9)

#1 @swissspidy
2 weeks ago

Probably also happens for a localized site where the translation files are missing

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

2 weeks ago

  • 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.


  • [ ] Fix tests
  • [ ] Add new tests covering the new desired behavior

Trac ticket:

#3 @mukesh27
9 days 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.

#4 @swissspidy
9 days ago

  • Milestone changed from Future Release to 6.3

#5 @swissspidy
7 days ago

  • Priority changed from high to normal

#6 @swissspidy
4 days ago

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

In 55865:

I18N: Improve _load_textdomain_just_in_time() logic when there are no translation files.

Fixes a performance issue where the JIT logic is invoked for every translation call if the there are no translations in the current locale. With this change, the information is cached by adding Noop_Translations instances to the global $l10n array. This way, get_translations_for_domain() returns earlier, thus avoiding subsequent _load_textdomain_just_in_time() calls.

Props swissspidy, johnbillion, ocean90.
Fixes #58321.

#8 @swissspidy
3 days 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 @flixos90
3 days 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. 👍

Note: See TracTickets for help on using tickets.