Make WordPress Core

Opened 7 weeks ago

Last modified 9 days ago

#59656 assigned enhancement

Merge Performant Translations (Ginger MO)

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

Description (last modified by swissspidy)

Over the past half a year or so, the core performance team spent significant time on extensively analyzing i18n performance in WordPress and finding ways to improve it. This cumulated in the Performance Translations feature plugin, which uses PHP files in favor of MO files for translations, which is much faster and benefits from OPcache as well.

This ticket tracks the suggestion of merging Performant Translations, which was originally developed by @dd32 under the name Ginger MO, into WordPress core.

View related make/core post:

Performant Translations supports multiple file formats (.mo and .php), as well as multiple text domains and locales loaded at the same time.

This means:

  • Loading .mo files as usual will be much faster and use less memory
  • If an .mo translation file has a corresponding .php file, that file will be loaded instead, making things even faster and use even less memory
  • When switching locales, we no longer have to unload the previous translations, but can instead keep them in memory, making locale switching much cheaper

A quick comparison:

Locale Scenario Memory Usage Load Time
en_US Default 14.46 MB 124.66 ms
de_DE Default 27.96 MB 173.44 ms
de_DE Performant Translations 15.62 MB 132.60 ms

As you can see, translations used to massively slow down a site, but with this new library, there is little to no additional overhead.

Since this uses a new file format for translations, some changes are required to add support for it in related places such as

Relevant companion tickets:

Everything is 100% backward compatible and if there are no PHP translation files, MO files will be loaded as usual.

Related: this is an alternative to #17268

Change History (13)

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

7 weeks ago

#2 @swissspidy
7 weeks ago

  • Description modified (diff)

#3 @flixos90
6 weeks ago

  • Priority changed from normal to high

Marking this as a high priority for the 6.5 due to the significant estimated impact this will have for localized sites' performance.

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

6 weeks ago

#5 @swissspidy
4 weeks ago

In 57083:

Build/Test Tools: Expand performance test scenarios.

Adds new tests for localized sites as well as the dashboard.
Also amends Server-Timing output to measure memory usage in all scenarios.

Props swissspidy, joemcgill, flixos90, mukesh27, mamaduka.
See #59656.
Fixes #59815.

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

4 weeks ago

#7 @joemcgill
4 weeks ago

The PR for this isn't being picked up by the GH -> Trac connection. It's linked from the ticket description, but for anyone else having trouble finding it, the PR is:

#8 follow-up: @swissspidy
4 weeks ago

@joemcgill I think it‘s because it‘s still a draft

#9 in reply to: ↑ 8 @joemcgill
4 weeks ago

Replying to swissspidy:

@joemcgill I think it‘s because it‘s still a draft

I've seen other draft PRs get picked up in tickets, like this one on #59600. I suspect the additional URLs in your PR description.

#10 @swissspidy
4 weeks ago

  • Description modified (diff)

#11 @akirk
2 weeks ago

Since this adds the ability to include a PHP file generated elsewhere, I am worried that this potentially creates a vector for putting malicious code in translation files that didn't exist before.

The contents of the PHP file is predictable, we could parse it easily using PHP's token_get_all() function. We could introduce a "secure" mode where the file would be checked before it is included.

Although, since this is about performance, we likely don't want to add a performance penalty. Thus we could try and use a checksum to ensure the file was not changed after it has been checked.

What do you think?

#12 @swissspidy
2 weeks ago

Note that the security aspect has been previously mentioned in the corresponding i18n performance analysis post.
The main takeaway is that installing translations is no different than installing a plugin or theme, which can also execute arbitrary code. And WordPress has always considered translations to be trusted.
The post does also mention a static analysis or checksum check for added safety, though doing that at runtime (vs. at install time) will negate the performance wins again. The latter would require additional infrastructure for storing and retrieving checksums too.
Right now the "secure" mode is using the translation_file_format filter to disable PHP file usage.

This ticket was mentioned in Slack in #core-i18n by swissspidy. View the logs.

9 days ago

Note: See TracTickets for help on using tickets.