Making WordPress.org

Opened 9 months ago

Closed 5 months ago

#7296 closed enhancement (fixed)

Add PHP files to language packs

Reported by: swissspidy's profile swissspidy Owned by:
Milestone: Priority: normal
Component: Translate Site & Plugins Keywords: has-patch has-unit-tests
Cc:

Description

Context:

The WordPress core performance team is working on a Performant Translations feature plugin that uses a new approach to handle translation files in WordPress, making localization blazing fast. This plugin helps to make localized WordPress sites faster by replacing the traditional MO translation files with PHP files, which are much faster to parse.

Learn more at https://make.wordpress.org/core/2023/09/05/call-for-testing-performant-translations/

The goal is to merge this new feature into WordPress 6.5, but nothing is set in stone yet. I merely want to get a headstart on making all the infra changes.

Anyway, once that lands, translate.wordpress.org will need to serve PHP files as part of language packs.

-> Here is the PR adding PHP support to GlotPress

I do have a patch for the \WordPressdotorg\GlotPress\Customizations\CLI\Language_Pack class to enable this for plugins and themes. But I couldn't find where the language packs are generated for core. Any help would be appreciated.

Another open question is if and how PHP files should be added conditionally. If a site is running WordPress 6.0, the language pack (either for core or a plugin/theme) shouldn't need to include the PHP files. Though it doesn't really hurt either, as the files are rather small (smaller than the MO file).

Attachments (2)

7296.diff (2.7 KB) - added by swissspidy 9 months ago.
POC patch for the Language_Pack command class
7296.2.diff (4.5 KB) - added by swissspidy 9 months ago.
Updated patch that also exports PHP files for core

Download all attachments as: .zip

Change History (21)

@swissspidy
9 months ago

POC patch for the Language_Pack command class

#1 @mihdan
9 months ago

This is a great idea, hopefully this approach will be in WordPress core soon!

@swissspidy
9 months ago

Updated patch that also exports PHP files for core

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


9 months ago
#2

  • Keywords has-patch has-unit-tests added

Notable changes from the plugin

  • Does not automatically convert any MO files to PHP files upon reading.
    • This avoids any FS interaction on regular requests, avoiding any unexpected results.
    • Makes the logic much simpler.
    • This additional feature can stay in the Performant Translations plugin for those who wish to use it.
  • No integration with Language_Pack_Upgrader to automatically convert MO files to PHP files.
    • This was only needed because WordPress.org didn't yet serve PHP files in language packs.
    • If someone is using a custom translation platform like Traduttore, it is up to them to provide PHP files as well (if they want to)

Related

To-do

  • [ ] Merge in latest changes from plugin

Trac ticket:

#3 @Otto42
8 months ago

Can we get some statistics on why this improvement is better? And maybe a simplified explanation of how it works?

#4 @swissspidy
8 months ago

@Otto42 Sure, please see https://make.wordpress.org/core/2023/07/24/i18n-performance-analysis/ for an in-depth analysis of various solutions including the PHP file approach, covering extensive benchmarks and comparisons. See also https://make.wordpress.org/core/2023/09/05/call-for-testing-performant-translations/ as linked in the ticket description.

tl;dr: instead of MO files, WP will load PHP files returning arrays containing the translations, which is much faster and benefits from OPcache as well.

We‘re targeting a 6.5 merge with all the necessary blog posts etc. This ticket here is for bringing this on the radar for meta early on.

@ocean90 commented on PR #5306:


7 months ago
#5

If an MO file has a corresponding PHP file, that file is loaded instead.

To keep the PR even "simpler" can we remove this part from this PR for now too? So basically as a first step just let add the foundation (replacement of POMO) and then extend that with the PHP format. This should give us more confidence that the replacement is working as expected with just MO which will still likely be the primary use case for a while.

@swissspidy commented on PR #5306:


7 months ago
#6

@ocean90 What would be your proposed timeline for that? For gradual adoption I would recommend not removing the part but simply changing the default format in the translation_file_format filter to mo. Later on we can then simply switch the default to php. Given the benefits of PHP I would suggest doing that sooner rather than later. Alternatively, keep the default but don't ship PHP translation files yet (so control it via the translation API)

@swissspidy commented on PR #5306:


7 months ago
#7

btw, yes I am aware of the failing tests. And no, leaky core tests are not fun.

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


5 months ago

@swissspidy commented on PR #5306:


5 months ago
#9

@westonruter For the return types I wasn't actually sure whether they are allowed in core, which is why I removed them at one point.

But seems like they are: https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#type-declarations - just the docs need a little updating now that PHP 7 is the required minimum

This ticket was mentioned in PR #191 on WordPress/wordpress.org by @swissspidy.


5 months ago
#10

#13 @swissspidy
5 months ago

This just got merged into core in https://core.trac.wordpress.org/changeset/57337!

@dd32 @ocean90 Would appreciate your review on https://github.com/WordPress/wordpress.org/pull/191 so that we can add this new file format to language packs. The last piece of the puzzle 🧩🚀

@swissspidy commented on PR #5306:


5 months ago
#14

@mukeshpanchal27 thanks for the feedback. just fyi it's easy to miss feedback on closed PRs, I almost didn't see your comments because I didn't get any notification. Best to ping separately.

@dd32 commented on PR #191:


5 months ago
#15

I haven't reviewed this, but it looks like it'll work.

GlotPress will need to make an alpha release before we can merge this though; as the php format isn't yet on https://plugins.trac.wordpress.org/browser/glotpress/trunk/gp-includes/formats

@mukesh27 commented on PR #5306:


5 months ago
#16

Yes, but it's nit-pick feedback, so I missed pinging you.

@swissspidy commented on PR #191:


5 months ago
#17

GlotPress will need to make an alpha release before we can merge this though; as the php format isn't yet on plugins.trac.wordpress.org/browser/glotpress/trunk/gp-includes/formats

This is now complete & the new format is available ✅

@Amieiro commented on PR #191:


5 months ago
#18

I have committed this PR to Meta, so I close it.

#19 @Amieiro
5 months ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.