Make WordPress Core

Opened 13 years ago

Closed 8 months ago

#17268 closed enhancement (wontfix)

Use native gettext library when available

Reported by: linushoppe's profile linushoppe Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch needs-refresh needs-unit-tests dev-feedback
Focuses: performance Cc:

Description

Here you say that the GNU gettext-Framework is used. Exactly, "pomo" (file: wp-includes/l10n.php) is a complete own php-implementation of the gettext-program.

I've added a patch to solve this problem. Maybe it is not very good, but it works. On my wordpress-sites, the used php-memory returns from about 65% to about 12% and the site is running much faster when patching wp-includes/l10n.php.

I know that gettext is not available on every wordpress-installation, but when it's available, it should be used.

Sorry for my bad english, I'm german.

Attachments (10)

wp_gettext.patch (4.1 KB) - added by linushoppe 13 years ago.
Patch to reduce the used php-memory
wp_gettext_v2.patch (4.3 KB) - added by linushoppe 13 years ago.
wp_gettext_v3.patch (4.0 KB) - added by linushoppe 13 years ago.
Patch against /wp-includes/l10n.php in WP 3.2.0
native.php (6.6 KB) - added by linushoppe 13 years ago.
native gettext-support php-file
wp-settings.php (9.7 KB) - added by linushoppe 13 years ago.
new wp-settings.php for native gettext-support
native_gettext.patch (7.9 KB) - added by linushoppe 13 years ago.
patch against WP 3.3.1, corrects the $Domain-Bug
17268.diff (28.6 KB) - added by Mte90 7 years ago.
includes native support and dynamic-mo-loader
17268.1.diff (28.7 KB) - added by Mte90 7 years ago.
it was missing a require
17268.2.diff (29.3 KB) - added by Mte90 7 years ago.
working on unit tests
17268.3.diff (36.5 KB) - added by Mte90 7 years ago.
improving code also for unit tests

Download all attachments as: .zip

Change History (125)

@linushoppe
13 years ago

Patch to reduce the used php-memory

#1 @duck_
13 years ago

Related: #17128

#2 @johnbillion
13 years ago

  • Cc johnbillion@… added

#3 @mwidmann
13 years ago

After applying this patch I was able to handle a substancial amout more requests in the same amount of time.

Running the same command command in all cases:

siege -c5 -t30s -b http://localhost

Results:

Version trans/secs  succ. transactions
3.1.2 tagged 2.35 69
with patch from #17128 2.92 86
with just this patch 3.40 100
with this and the other patch 3.50 102

This change is a huge winner if gettext is installed. Would be nice to have this in 3.2.

#4 follow-up: @scribu
13 years ago

Looks nice, but I'm wondering if it wouldn't be even faster if we had two definitions of each WP translation function:

if (function_exists ('dgettext')) {
  require_once('translate-gettext.php');
} else {
  require_once('translate-pomo.php');
}

translate-pomo.php would contain the current implementation in WP, while translate-gettext.php would contain the more efficient, gettext based definitions.

#5 @westi
13 years ago

It looks like this patch doesn't handle translation with context at all and also I don't understand the need for all that copying of files.

I think this would be better implemented as an alternative implementation of a Translations class - probably could even be done in a plugin then maybe

#6 @Philipp15b
13 years ago

  • Cc hebipp1@… added

#7 in reply to: ↑ 4 @linushoppe
13 years ago

Replying to scribu:

Looks nice, but I'm wondering if it wouldn't be even faster if we had two definitions of each WP translation function:

if (function_exists ('dgettext')) {
  require_once('translate-gettext.php');
} else {
  require_once('translate-pomo.php');
}

translate-pomo.php would contain the current implementation in WP, while translate-gettext.php would contain the more efficient, gettext based definitions.

Very nice idea. I just wrote this patch to make WP faster, I had no time to make the patch "good-looking" and useful.

#8 @linushoppe
13 years ago

  • Cc linus.hoppe@… added

We've created a new version of the patch. The new one (patch against l10n.php of Wordpress 3.1.3) can handle php contexts (msgctxt) via $context. We hope that it helps you to integrate this patch into one of the next wordpress versions in order to reduce the used php-memory.

#9 @aphrodite
13 years ago

Hi !

Be carefull on Cpanel servers, the folder creation and chmods generates a mkdir error. Added a @mkdir on line 377, and on some servers the folder creation may be mannual...

Moreover, translations are not directly applied. This is a little issue, but can be confusing for users. Must refresh the pages to have the translation applied ;)

Last edited 13 years ago by aphrodite (previous) (diff)

#10 @scribu
13 years ago

WP has a helper function for that called wp_mkdir_p().

Last edited 13 years ago by scribu (previous) (diff)

#11 @xknown
13 years ago

The are two problems that may need to be addressed:

  • setlocale only works if the locale name matches a system locale.
  • two different .mo files may not be used when they contain translations of the same domain. For example, "$locale.mo" and "ms-$locale.mo" (both referenced in the core).

#12 @linushoppe
13 years ago

What do we have to change to see this patch in one of the next WP-releases?

@linushoppe
13 years ago

Patch against /wp-includes/l10n.php in WP 3.2.0

#13 @wet
13 years ago

  • Cc r.wetzlmayr@… added

#14 @pavelevap
13 years ago

  • Cc pavelevap@… added

#15 @kurtpayne
13 years ago

  • Cc kurtpayne added

#16 @ocean90
13 years ago

  • Cc ocean90 added
  • Component changed from General to I18N
  • Type changed from defect (bug) to enhancement

#17 @MHagemeister
13 years ago

  • Cc MHagemeister added

#18 @MHagemeister
13 years ago

Love this patch! I get similar performance improvements like mwidmann.

#19 @scribu
13 years ago

  • Summary changed from Using pomo for translations is very inefficient to Use native gettext library when available

Changing title to avoid confusion with #17128

#20 @scribu
13 years ago

  • Component changed from I18N to Performance

#21 @scribu
13 years ago

  • Keywords early added
  • Milestone changed from Awaiting Review to Future Release

#22 @scribu
13 years ago

  • Milestone changed from Future Release to 3.4

#23 @sirzooro
13 years ago

  • Cc sirzooro added

#24 @Bueltge
13 years ago

  • Cc frank@… added

#25 @swissspidy
13 years ago

  • Cc hello@… added

#26 @mikeschinkel
13 years ago

  • Cc mikeschinkel@… added

#27 @nacin
13 years ago

I think it would be interesting to see this abstracted a bit cleaner. Perhaps, as westi indicated, a separate class.

For example, we already have Translations and NOOP_Translations. We should introduce a new _Translations class.

In the process, we should probably split the classes into three files.

Ideally all of the relevant code should occur in wp-includes/pomo, and none of it in l10n.php. That is simply our API layer.

#28 @scribu
13 years ago

  • Keywords needs-patch added; has-patch removed

Agreed. This should be just another translation implementation, like we have multiple implementations for HTTP requests.

#29 @westi
13 years ago

  • Milestone changed from 3.4 to Future Release

Moving back to Future Release until we have the scope discussion for 3.4

@linushoppe
13 years ago

native gettext-support php-file

#30 @linushoppe
13 years ago

For WordPress 3.3 we did a complete rewrite of the patch to make wordpress
work with PHP's native gettext-support.

The Patch now emulates the PoMo-API and creates an alias from the old class
named "MO" to itself - you just have to make sure that its loaded *before*
pomo is loaded.

File that were modified:

wp-settings.php (new verson attached to the bug report)

Files that were added

wp-includes/pomo/native.php (attached to the bug report)

I've added a patch to create the new native.php and to modify the wp-settings.php.

@linushoppe
13 years ago

new wp-settings.php for native gettext-support

#31 @xeno010
13 years ago

Hallo,
I've written a plugin -> WP-Performance-Gettext-Patch

I use a modified version of your patches:

  • Add Cacheing-Control
  • other minor changes

Benchmark:
Linux with Plugin 3x faster

#32 @linushoppe
13 years ago

xeno010, can you please explain what "other minor changes" your plugin has and why you've added caching? As far as I know, gettext itself has an own cache-implementation.
Besides, installing a plugin is not as powerful as patching the wordress-sourcecode and only some users would profit from the changes.

#33 @nacin
13 years ago

I closed http://wordpress.org/extend/plugins/wp-performance-gettext-patch/ for download as it applies a patch to core, which is dangerous and an all-around bad idea. Its source is still viewable and accessible in SVN.

#34 @pavelevap
13 years ago

I tested mentioned plugin and encountered great memory gains. My WP website with installed plugin used 28 MB memory and without this plugin more than 34 MB. This problem is real pain for localized versions, because there are more and more strings with every new release...

#35 @noah.williamsson
13 years ago

  • Cc noah.williamsson added

I ran into the l10n performance issue initially reported in #17128 on an Ubuntu 10.04 LTS (PHP 5.3.2, APC 3.1.3) system running Wordpress 3.3.1.

I did some testing and meassured the response time for serving a single post page.
Here are my numbers for Wordpress 3.3.1.

Feature Response time Performance hit
Stock 3.3.1, WPLANG not set 139ms 0%
WPLANG set to 'sv_SE' 255ms 45% slower
WPLANG set + patch from #17268 145ms 4% slower

As can be seen, the patch (native_gettext.patch) improved page response times by 43% compared to a stock Wordpress 3.3.1 installation where WPLANG is used.

I would like to see this patch merged for 3.4.

#36 @scribu
13 years ago

I just noticed that native_gettext.patch has the following comment:

@license http://creativecommons.org/licenses/by-sa/3.0/de/ Creative Commons Attribution-Share Alike 3.0 Germany

This license is not compatible with the GPL license and therefore can not be included in WordPress Core.

Please consider changing the license to GPL or removing that comment altogether, in which case the GPL is implied.

Last edited 13 years ago by scribu (previous) (diff)

#37 @ryan
13 years ago

Any native support we build in should fallback to the current pomo setup if the desired locale is not fully supported. The presence of the gettext extension does not mean that any given locale can be served. Type "locale -a" on your Mac OS or Linux box and see which ones you have. Some hosts install very few locales, often just C, POSIX, and en_US*. Different environments can have different names for the same locale. Also, the directory structure required by gettext is not compatible with what we use now.

#38 @Chouby
13 years ago

  • Cc frederic.demarle@… added

#39 @gr0b1
13 years ago

FYI I get a 25% memory decrease and I if i benchmark my homepage: I go from around 18 req/s to 30+ req/s with this patch.
There is a typo on line 217: "$domain" should be "$Domain". (causes � bug)
Anyway thanks. And please push this out soon, even if turned off by default.

Last edited 13 years ago by gr0b1 (previous) (diff)

#40 @nacin
13 years ago

  • Milestone changed from Future Release to 3.4

Not promising anything, but we may just end up doing this.

@linushoppe
13 years ago

patch against WP 3.3.1, corrects the $Domain-Bug

#41 @linushoppe
13 years ago

I've added a new version of the patch. This patch was created against Wordpress 3.3.1. It fixes the $Domain-Bug mentioned.
More information: http://oss.tiggerswelt.net/wordpress/3.3.1/

#42 @linushoppe
13 years ago

By the way, the patch ist GPL now.

#43 @scribu
13 years ago

  • Keywords has-patch added; needs-patch removed

#44 @pavelevap
13 years ago

I tried to test latest patch with WP 3.3.1 and received following error:

Warning: Cannot redeclare class MO in .../wp-includes/pomo/native.php on line 242

#45 @Devstorm
13 years ago

  • Cc Devstorm added

#46 @nacin
13 years ago

  • Milestone changed from 3.4 to Future Release

Not going to happen for 3.4. Let's see what we can continue to cook up. If we can come up with a sturdy patch that doesn't affect installs without gettext, and handles issues related to LC_* values, I think we'd be open to reconsidering it.

#48 @tszming
12 years ago

+1 for making this patch merged into the core as soon as possible.

(Have done a quick profiling and the those *textdomain* functions really used too many CPU cycles)

#49 @knutsp
12 years ago

  • Cc knut@… added

#50 @meloniq
12 years ago

  • Cc meloniq@… added

#51 @mbijon
12 years ago

  • Cc mike@… added

#52 @tszming
11 years ago

I have been using the patch for years, any chance to get it merged?

#53 @nacin
11 years ago

  • Component changed from Performance to I18N
  • Focuses performance added

#54 @greencp
11 years ago

linushoppe, I included your gettext implementation into my new plugin WP Performance Pack and added you to the list of contributors. It can be used without any chances to core files by using override_load_textdomain.

The plugin needs some more checks if native gettext can be used, but it works for me. Only changes I made to your implementation were adding checks for NOOP_Translations in merge_with and merge_originals_with.

#55 @wonderboymusic
10 years ago

  • Keywords needs-refresh added

This causes fatal errors in unit tests

#56 @benjamin4
10 years ago

Note that the gettext cache isn't flushed until the restart of the web server [1], but it should be cached after any update. Restarting the web server isn't really an option in shared hosting :)

[1] There seem to be workarounds:
https://stackoverflow.com/questions/13625659/how-to-clear-phps-gettext-cache-without-restart-apache-nor-change-domain
http://www.php.net/manual/en/function.gettext.php#58310

#57 follow-up: @nacin
10 years ago

What if we went to an associative PHP array for our strings?

<?php
return array( 'English string' => 'translated string' );

How would performance look? We'd take a bit of a hit for loading the file, but it'd avoid the overhead of parsing a MO file, could be opcode-cached, etc.

ryan, nikolay, and I have discussed this as an idea over the years, but we've never tried it out.

#58 @greencp
10 years ago

I just saw, I didn't mention it in my last comment: I wrote an alternative MO file Reader (read only) that is faster and uses less memory than the current implementation. That's how I started my plugin. http://wordpress.org/plugins/wp-performance-pack/ With caching enabled performance hit is almost minimal compared to not translated WordPress setups.

#59 @tszming
10 years ago

@benjamin4 , the patch by linushoppe check the md5sum of the file everytime and assign to the domain, so it won't be a problem for shared hosting. (We have changed to filemtime to improve performance.)

#60 @markjaquith
10 years ago

Okay. To advance this ticket we need someone to explore Nacin's static array mapping idea. Any volunteers?

#61 follow-up: @luciole135
9 years ago

I use this hack for 3 years without any problems on my web site.
I have not tested it for a multisite.

To do this I added to wp-config.php file the following lines:

/* Utilisation de gettext pour l'internationalisation */
require( ABSPATH . '/wp-includes/pomo/native.php');

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


8 years ago

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


8 years ago

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


8 years ago

#65 @francescobagnoli
8 years ago

is there any news...

#66 in reply to: ↑ 61 @francescobagnoli
8 years ago

Replying to luciole135:

I use this hack for 3 years without any problems on my web site.
I have not tested it for a multisite.

To do this I added to wp-config.php file the following lines:

/* Utilisation de gettext pour l'internationalisation */
require( ABSPATH . '/wp-includes/pomo/native.php');

After some test on multisite, it work with a speed improvment of 0.5 seconds.

#67 @luciole135
8 years ago

With WordPress 4.6, there is a problem when we want to add a plugin from the dashboard of WordPress: « An unexpected error occurred. Something seems not to work with WordPress.org or configuring the server. If you continue to have problems, please try the support forums. »

#68 follow-up: @swissspidy
8 years ago

@luciole135 100% sure this has nothing to do with this ticket. That error message is quite common and usually happens when your server temporarily cannot access wordpress.org.

If you continue to see this error, please post your issue in the support forums along with some details about your site, server, etc. Trac is not for support.

#69 in reply to: ↑ 68 @luciole135
8 years ago

Replying to swissspidy:

@luciole135 100% sure this has nothing to do with this ticket. That error message is quite common and usually happens when your server temporarily cannot access wordpress.org.

You are right on my site online there is a problem with this hack, but not on my local test site. So it is the server configuration is in question.
I apologize for the inconvenience.

Last edited 8 years ago by luciole135 (previous) (diff)

#70 @ottok
7 years ago

This idea will improve WP load times by over 50% on any site not using the default English language. Please core developers adopt the patch and merge it into core. The original developer seems to have given up after trying for 3 years to get some love for this. Now it is over 6 years since the idea was initially presented..

#71 @Mte90
7 years ago

As I can see the patch don't require so much work on update (only the reference on settings.php that is changed the line but I need to investigate).
I think that use the native module only if available is a good option to improve the performance and be more standard.

#72 @Mte90
7 years ago

There is also https://github.com/aucor/dynamic-mo-loader that is a new implementation with caching.

#73 @koenHuybrechts
7 years ago

What should we do to get this finally moving? There are more than 7000 languages, other than English.

#74 @greencp
7 years ago

Guys, 3 years ago I commented to this ticket mentioning my plugin in which I implemented native gettext support based in parts on this ticket not needing any core patches. I also commented about my own MO reader implementation that doesn't require gettext extension and using cache is almost as fast as native gettext, also included in the plugin. That implementation, by the way, is used in the dynamic-mo-loader mentioned by @Mte90, and is currently awaiting review to be added as stand alone plugin on wordpress.org. Both, native gettext and new MO Reader, are working fine on different sites of mine since then.

In all those years I got no feedback on my comments or plugin here.

Every once in a while a new comment to this ticket pops up but nothing happens. I'm curious if this will ever be addressed even though there are many working solutions out there. Meanwhile I'll keep working on my own solution.

#75 @mwidmann
7 years ago

It's funny how I was reminded about this ticket in a talk about WordPress performance at WordCamp Europe last week. Too bad the video of the talk by @ottok is not online yet.

It would be great to finally have a good solution for any non-English sites as loading the MO files is the main performance killer in the WordPress bootstrap.

@nacin 's idea of using hashs instead of the POMOs could be a good one for performance, but there would be the need for some kind of functionality to automatically convert the translated text-domains for each and all plugins on plugin activation (or on first access) to this hash because otherwise one single update would break all i18n in all plugins that haven't been updated yet.

This ticket was mentioned in Slack in #core by mte90. View the logs.


7 years ago

#77 follow-up: @swissspidy
7 years ago

It seems to me that comment #56 and comment #57 haven't been fully addressed/explored yet.

As for reading and parsing MO files, I think there was once a plan to integrate https://github.com/dd32/ginger-mo.

There's also https://github.com/aucor/dynamic-mo-loader. So, the question for that would be: which one do we choose?

Whatever way we decide to go, this needs some new performance tests with various situations (default language, localized site, switching locales multiple times, etc.).

#78 in reply to: ↑ 77 ; follow-up: @dd32
7 years ago

Replying to swissspidy:

As for reading and parsing MO files, I think there was once a plan to integrate https://github.com/dd32/ginger-mo.

There's been a few optimisations in core, and a bunch of questions raised around which/what format of JSON to support (if needed) which made the multiple-format in GingerMo not needed..
GM could be simplified again, and re-benchmarked, or some features stripped out of POMO and could probably be just as fast.

I don't think using the PHP Gettext library is a realistic option for WordPress (It's not exactly the best PHP API at all) and using object caches isn't good for i18n either (although benchmarks may show a benefit, it'll have consequences under heavy load).

#79 in reply to: ↑ 78 @ocean90
7 years ago

Replying to dd32:

I don't think using the PHP Gettext library is a realistic option for WordPress (It's not exactly the best PHP API at all) and using object caches isn't good for i18n either (although benchmarks may show a benefit, it'll have consequences under heavy load).

I second that. It's not a secret that Gettext isn't the best i18n library out there, We had a lot of chatter about that at WCEU. So maybe we should take this opportunity to look at alternatives like MessageFormat.

This ticket was mentioned in Slack in #core by selvambe23. View the logs.


7 years ago

#81 @ottok
7 years ago

MessageFormat is a completely new format. Migration to it would require to rewrite current i18n tools, Polyglot, po-editors etc. I strongly advice against doing drastic changes here.

The whole issue here is about low-hanging fruit that has been pending for core devs attention for some years.

I suggest you simply adopt the usage of native gettext when available, and then benefit from all optimizations native gettext provides. I agree that upstream gettext isn't that actively maintained (having a patch of mine waiting for review already over a year..) and maybe PHP gettext isn't the latest and coolest either, but they are at least mature and the whole ecosystem around .po/.mo file handling works well.

There is even a patch attached to this issue already. Don't expand the scope here, please. Just take in usage gettext as an improvement to the current situation and close this issue.

Optimizations in step 2 and 3 can be tracked in other issues.

#82 @Mte90
7 years ago

So probably actually we can do:

  • Continue to use Po/Mo actual system for l10n files
  • Implement the support for the PHP native po/mo php module (we need to improve the performance in any case and improve the actual platform for l10n with this support)
  • Later when this new implementation will be available evaluate what to do for new formats for WordPress

So actually we have to evaluate what is the best option between dynamic-mo-loader and ginger-mo.
I will try to do performance tests with them and comparison with the help of xDebug.

#83 @Mte90
7 years ago

Seems that Ginger-Mo is not a plugin so is not possible to test on WordPress pretty easily without rewriting.

I have done a test with 4.9-alpha-40870-src, on VVV on Discussion Options on the WordPress backend with the Italian language.
My installation had not plugin installed or activated so is a pure WordPress installation.
In this screen you can see the difference on the two pages with the dynamic-mo-loader and not with a difference of 300 ms with this plugin http://imgur.com/a/4k9oi

That plugin not use the PHP native modules but only caching and loading only of text domain used on the page so I think that add this code and support also the native modules can speed up more our amazing CMS :-)

#84 @ottok
7 years ago

@Mte90 thanks for taking the time to test dynamic-mo-loader. Your result 500 vs 800 milliseconds is in the same range as my measurements. See for example slide 44 at https://www.slideshare.net/ottokekalainen/improving-wordpress-performance-with-xdebug-and-php-profiling

I don't have a benchmark done on the use of native gettext, but I am sure it is much faster than the custom implementation in PHP in WordPress. Smart caching is good, and having native gettext functions that are quick to start with is even better. Whatever the solution will be, WP core developers should not neglect this issue for another 6 years since even simple benchmarks prove there is a 30+ % speed improvement to achieve quite easily.

@Mte90
7 years ago

includes native support and dynamic-mo-loader

#85 @Mte90
7 years ago

This patch include the native gettext support and dynamic-mo-loader.
Probably is not the better way to add external libraries in WordPress but to do performance analysis and debug I think that is good enough.

The native patch code became an alias to the gettext php version and this class is used from WordpPress or dynamic-mo-loader.
Using my patch with WordPress with the native (php7.1-intl package on VVV's ubuntu) show a difference of 300ms (using Yoast SEO plugin on the settings page) without dinamyc-mo-loader.
With dynamic-mo-loader there is another boost of 50ms so 350ms instead the gettext php version (probably because of the cache enabled in this library there is this boost).

I created two filter for dynamic-mo-loader:

<?php
add_filter( 'load_mo_dynamic', '__return_true' ); //there is another parameter that is $domain
add_filter( 'load_mo_cache_mo_dynamic', '__return_true' ); //to enable the cache

Both the filter are true on default so you can do all the test that you want.
In any case dynamic-mo-loader contain few little improvements on the code https://github.com/aucor/dynamic-mo-loader/pull/6 by me

@Mte90
7 years ago

it was missing a require

#86 @greencp
7 years ago

@Mte90 I added your changes to my code and further improved performance by omitting use of POMO_FileReader (which added unnecessary overhead). See here for latest version: https://plugins.trac.wordpress.org/browser/wp-performance-pack/trunk/modules/l10n_improvements/class.wppp_mo_dynamic.php

#87 @Mte90
7 years ago

Cool but in case can you give me a diff? It will be more easy to improve my patch and also dynamic-mo-loader in case (doing a pr) :-)

This ticket was mentioned in Slack in #core by koenhuybrechts. View the logs.


7 years ago

#89 @johnbillion
7 years ago

  • Keywords needs-unit-tests added

This will need unit tests which demonstrates that the existing Translations class and any other classes that are introduced all function identically.

I'd also like to see some up to date performance figures for memory usage and execution time.

#90 @Mte90
7 years ago

I tried that patch with PHPunit and as I can see there are this errors that are caused by the php native module:

There were 2 failures:

1) Tests_POMO_MO::test_nplurals_with_backslashn
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'%d foro'
+'%d forum'

/home/www/VVV/www/wordpress-develop/public_html/tests/phpunit/tests/pomo/mo.php:117

2) Tests_POMO_MO::test_load_pot_file
Failed asserting that true matches expected false.

/home/www/VVV/www/wordpress-develop/public_html/tests/phpunit/tests/pomo/mo.php:141

As I can see the native module/native.php doesn't handle the malformed po/pot file like the PHP version. I will investigate :-)

@Mte90
7 years ago

working on unit tests

#91 @Mte90
7 years ago

Seems that the native code is missing few methods and other stuff from the MO code that I am backporting on that to fix the unit tests.
The work is not finished but I hope to finish soon :-)

@Mte90
7 years ago

improving code also for unit tests

#92 @Mte90
7 years ago

I spent few hours working on this days and the last patch fix many unit tests but showed many issue between the native module and the POMO implementation in WordPress.
First of all they work in a different way, while in WordPress you can have the access to all the strings/entries in an array with the native is not possible because it is everything in the memory so many unit tests are not useful for native.
Also the headers on gettext are not available but the unit tests use them and the only way to get that values is to parse the text.
So many methods used in the unit tests are not available but I backported a few about the export that the native doesn't include so probably will be better to split up the code to avoid a duplication between different classes.
Debug with gettext is very difficult because actually my patch is not working anymore and I don't understand why in any case the code now is more clean and organized.
The native method use also the cache from the webserver so there can be issue with that when someone update the files.
Again the unit tests doesn't use a textdomain but check only the methods so probably the native methods needs a specific suite of unit tests.
In any case to cover the needs of unit tests is required a parse of the mo files that there is the issues on performance (without consider the cache).

What are the next steps? Someone else that try the various patch (the 17268.2.diff is wrong is incomplete) and give their feedbacks or help me on fix the various issue because alone is a difficult task.

#93 @johnbillion
7 years ago

  • Milestone changed from Future Release to 5.0

Let's take a look at this early in the 5.0 cycle so it gets a good amount of testing.

#94 @ottok
6 years ago

Related gettext optimization using lazy loading functions (=get's evaluated only if the string actually is displayed somewhere during the page load) is implemented in ticket https://core.trac.wordpress.org/ticket/41305 - thanks @schlessera!

#95 @pento
6 years ago

  • Milestone changed from 5.0 to 5.1

This ticket was mentioned in Slack in #polyglots by websupporter. View the logs.


6 years ago

#97 @pento
6 years ago

  • Milestone changed from 5.1 to Future Release

This needs more testing and feedback.

This ticket was mentioned in Slack in #core by mte90. View the logs.


6 years ago

#99 @desrosj
5 years ago

Related: #48116

#100 @Mte90
5 years ago

I was thinking to refresh this patch with the usual integration of dynamic-mo-loader that in meantime was updated.
Before to do I appreciate having feedback on what I already did in the previous comments in these tickets, also because this ticket is followed by 50 people, so I think that there are interests on moving on.

This ticket was mentioned in Slack in #core-test by sergey. View the logs.


3 years ago

#102 @TeemuSuoranta
3 years ago

As a maintainer of dynamic-mo-loader I would advice looking into DynaMO that the creators of Polylang have just released: https://wordpress.org/plugins/dynamo/

This is far better documented and structured than dynamic-mo-loader ever was.

#103 @colinleroy
3 years ago

Hello,
Just a word to say that I've read the entire history of this ticket and seen that it seems hard to get native gettext in Core. I understand why, especially the part where it only works with the system-supported locales.
I've seen that WP Performance Pack seems unmaintained, I've tested that DynaMO gives less performance improvement than dgettext(), so I've made my own minimal plugin that does dgettext(). I've made it available for other interested people : https://fr.wordpress.org/plugins/native-gettext/

Hope that helps.

#104 @mukesh27
2 years ago

  • Keywords dev-feedback added; early removed
  • Milestone changed from Future Release to 6.1

Hi there!

For the last 11 years, folks have been discussing the implementation of this workflow. We have more than 50% other locales than English. @Mte90 provided patches and improved them frequently in the last few years.

Do you have the latest updated patch against the trunk (latest version)? As performance is the key part of any website, we should consider this ticket and push the work so other locales' users get the benefit of it and see some improvement in their websites without the use of additional plugins.

Also, more than 60 folks are following this ticket. I'm going to add this ticket to the next milestone 6.1 for visibility so we get some input from the folks.

Feel free to update Milestone to Feture Release if there is no progress.

#105 @Mte90
2 years ago

Hi, this is a good news, but until I don't see any real further action I don't believe that this change will be implemented.

As it is now my old patches are kind of useless after all those years also because in the meantime there are various implementation that improve differently this change.

I moved the discussion to the performance team with a github ticket https://github.com/WordPress/performance/issues/171

So to me, the next steps to take:

  • Define what library/implementation use
  • Integrate in WP
  • Test the test suite (years ago this was one of the issues, as there are tests specific for our implementation that aren't gettext standard)

Looking also the PHP-FIG started discussing a standard (I am in the Discord server) about localization files but there isn't anything more about it.
We have to remember that https://konstantin.blog/2021/php-benchmark-include-vs-file_get_contents/ for performance reasons is more fast to read files from HDD/SSD but our big issue is that we have to parse a binary file by PHP at every page requests multiplied for any language file.

So probably a solution that save it using object cache or generating PHP/JSON files it is better and more similar to other PHP frameworks.

In conclusion, this is a work that require more than 1 single person discussing (not on this ticket that has a bigger history) and doing some benchmark and tests (that is the reason why I abandoned the working on this).

#106 @mukesh27
2 years ago

Thanks for the additional information regarding the progress.

Let's wait for other folk's responses who can help us with this ticket.

#107 @itmapl
2 years ago

Hey

@mukesh27 I proposed yet another solution for that problem here #56051 (see last comment). Would be nice to have whatever was proposed ;-) A lot of solutions have been put on the table and nothing happened for many many years.

Fixing that we speed up a WP instance by a lot so it matters actually.

Last edited 2 years ago by SergeyBiryukov (previous) (diff)

#108 @mxbclang
2 years ago

Related GH issue in the Performance Lab repo: https://github.com/WordPress/performance/issues/171

Discussed in the 3 Aug performance bug scrub and this warrants further coordination as part of the Performance team's work, so we plan to discuss further in a future Performance weekly chat. I'll leave another update here and on the linked issue once we have that discussion scheduled so folks can attend if they'd like.

#109 @adamsilverstein
2 years ago

  • Milestone changed from 6.1 to Future Release

#110 in reply to: ↑ 57 @swissspidy
19 months ago

What if we went to an associative PHP array for our strings?

<?php
return array( 'English string' => 'translated string' );

How would performance look? We'd take a bit of a hit for loading the file, but it'd avoid the overhead of parsing a MO file, could be opcode-cached, etc.

ryan, nikolay, and I have discussed this as an idea over the years, but we've never tried it out.

If anyone's curious, I've started experimenting with that over at the Preferred Languages feature project: https://github.com/swissspidy/preferred-languages/pull/659

Initial memory consumption and performance results are positive.

#111 follow-up: @swissspidy
14 months ago

The core performance team recently conducted an in-depth i18n performance analysis, which confirmed the performance penalty localized WordPress sites pay today. The blog post presented and compared multiple solutions to this problem—including the native gettext extension, but also different approaches like using raw PHP files for translations.

The latter turned out to be the most promising approach and now we want to test it at a wider scale using a dedicated plugin, appropriately named Performant Translations.

I would love for you all to first read the performance analysis and then check out the feature plugin on your site(s).

Download the plugin

#112 in reply to: ↑ 111 @colinleroy
14 months ago

Replying to swissspidy:

Hi,

The latter turned out to be the most promising approach and now we want to test it at a wider scale using a dedicated plugin, appropriately named Performant Translations.

Thanks for working on this. I have given Performant Translations a look and it's promising. (I'm the author of Native Gettext and I'd love to be able to drop it).

However, profiling shows that Performant translations is still much more CPU-intensive than Native Gettext is. On a reference page, I get

  • 78M cycles in mo.php
  • 21M cycles in Performant Translations' translatable-object.php
  • 3.5M cycles in NativeGettext's class-native-mo.php (it does count cycles spent in the native PHP extension)

I think Performant Translations is much better than what actually exists in the core, and should replace the existing core's translation system. But I also think it is not fast enough, and as each cycle is energy, it is our responsability as web developers working on The Engine That Powers 40% of the internet to do the best we can. This inefficiency probably translates to terawatts-hour.

Reference profiles available at (I couldn't find how to attach files):
https://colino.net/tmp/no-translation-plugin.cachegrind
https://colino.net/tmp/performant-translations.cachegrind
https://colino.net/tmp/native-gettext.cachegrind

Hope this helps!

#113 follow-up: @swissspidy
14 months ago

Thanks @colinleroy, appreciate the feedback!

Could you perhaps share your findings on GitHub? I'd love to take a closer look after my vacation, making sure it's as fast and efficient as possible.

FWIW, translatable-object.php is a file from Polylang, not Performant Translations. You might wanna double check your results.

#114 in reply to: ↑ 113 @colinleroy
14 months ago

Replying to swissspidy:

Thanks @colinleroy, appreciate the feedback!

FWIW, translatable-object.php is a file from Polylang, not Performant Translations. You might wanna double check your results.

Oh, you're absolutely right, and I had a brain fart! In reality the difference is less, it's 12M cycles not 21 :)

#115 @swissspidy
8 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Just circling back here: WordPress 6.5 is shipping with a new, faster localization system as announced a bit earlier. See https://make.wordpress.org/core/2024/02/27/i18n-improvements-6-5-performant-translations/

It easily outperforms other methods that were tried in the past, and shines with good developer experience too.

With this new system in place, there is now no longer a reason to look into adopting native gettext. Hence I am closing this one as a wontfix, because, again, the new system is very fast :)

Note: if you find any issues with this new system or have any enhancement suggestions, please open a new ticket.

Note: See TracTickets for help on using tickets.