#26638 closed enhancement (wontfix)
Performance Increase in l10n (9-12%)
Reported by: | dshafik | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.9 |
Component: | I18N | Keywords: | has-patch |
Focuses: | performance | Cc: |
Description (last modified by )
While doing some writing on profiling I choose WordPress as my example application and wrote a small patch that in my testing boosts performance of index.php by 9-12% (with the dataset from my 6 year old blog, ~250 posts, lots of plugins and extra stuff).
This performance increase is found by making a small change to the way translate()
works, effectively cutting out the use of the NOOP_Translates
class.
Patch is attached.
Attachments (4)
Change History (25)
#2
in reply to:
↑ 1
@
11 years ago
Replying to nacin:
In this case, when using the domain default
— which is the only time I skip it — only the NOOP_Translations
is used. Note that I'm still calling the gettext
filter, just with the same text that would get returned by NOOP_Translations
.
These numbers are from xhprof profiling, I've done it on a couple of machines now, several months apart, we're saving 1500+ function calls on a standard index.php alone (I'm using a based WP install, just my DB), and the instantiation of the object too.
#3
follow-up:
↓ 4
@
11 years ago
The domain default
is the WordPress domain. See also load_default_textdomain(). If you downloaded, say, the Spanish language pack from es.wordpress.org, nothing will get translated.
The instantiation of the object only happens once, and it's light, so that's not a big deal. I agree that avoiding the function calls would be nice. If it consulted $l10n directly instead of calling get_translations_for_domain() and then did if ( $l10n['default'] instanceof NOOP_Translations )
(and if true, bypassed the second function call), I think we'd get most of the benefits here without sacrificing the translation aspects.
#4
in reply to:
↑ 3
@
11 years ago
Replying to nacin:
instanceof
can be expensive, it would be better to set it to (and check for) false
. I can make this change, re-run the profiling to compare and update the patch.
#5
@
11 years ago
We could probably do something like 26638.patch.
is_textdomain_loaded()
is basically the same check as in get_translations_for_domain()
:
tags/3.8/src/wp-includes/l10n.php#L667.
#6
@
11 years ago
Benching all three, I see the following:
- l10n.patch - 9% improvement
- l10n-false.patch - 8% improvement
- 26638.patch - 6% improvement
I think the latter is the most comprehensive patch though...
#7
@
11 years ago
The final patch will break all other uses of get_translations_for_domain(), such as _n (translate_plural()), etc. It will also change the return value of get_translations_for_domain(), which is not ideal (but something we can work around if we found a change to the data structure was beneficial).
#9
in reply to:
↑ 8
@
11 years ago
Replying to SergeyBiryukov:
26638.2.patch avoids a function call.
I think in the interest of readability the previous patch (26638.patch) is better.
#11
@
11 years ago
- Focuses performance added
- Summary changed from [Patch] Performance Increase in l10n (9-12%) to Performance Increase in l10n (9-12%)
#12
@
10 years ago
Will this be landing in 4.0? Between this, and #26640 that would yield a 20-23% performance improvement.
#14
@
10 years ago
It's to late for 4.0 but I don't mind to test this on some of my sites. So what is the patch that is now ready to be tested?
#15
@
10 years ago
@markoheijnen it's attached as 26638.patch, or a less readable suggestion (IMO) 26638.2.patch
#16
follow-up:
↓ 17
@
10 years ago
- Keywords has-patch 4.1-early added
- Milestone changed from Awaiting Review to Future Release
#17
in reply to:
↑ 16
@
9 years ago
Replying to SergeyBiryukov:
Im curious when this be implemented since we're already on 4.2. The same goes for #26640.
#19
@
8 years ago
- Keywords reporter-feedback dev-feedback added
Now that we have _load_textdomain_just_in_time()
(#34114), the approach in the latest patch doesn't seem viable anymore.
@dshafik If you're still interested in this, it would be great to hear your opinion on how improvements could be made now. Also, how did you measure the performance? Makes testing/sharing patches easier.
Also pinging @ocean90.
#20
@
6 years ago
- Description modified (diff)
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from new to closed
Going to close this as a wontfix for now since the patches don't comply with how core loads translations anymore. See #34114 for history.
Feel free to reopen if someone has a solution which doesn't copy most of get_translations_for_domain()
into each translate function and still has a noticeable performance improvement.
#21
@
2 years ago
- Keywords needs-testing reporter-feedback dev-feedback removed
Found this ticket again while looking at #26640.
It looks like the NOOP_Translations
usage was improved in [36538] / #21319, where we save an instance of the class to a static variable instead of creating a new instance every time, as originally implemented in [12080] / #10971.
Out of curiosity, I've experimented a bit more with something along the lines of:
if ( isset( $l10n[ $domain ] ) ) { $translations = $l10n[ $domain ]; } else { $translations = get_translations_for_domain( $domain ); } if ( $translations instanceof Translations ) { $translation = $translations->translate( $text ); } else { $translation = $text; }
But that did not provide any noticeable performance improvement, at least not on PHP 8.0.x. So I guess these particular micro-optimizations are no longer relevant here.
Thanks for the patch! NOOP_Translations is used when a domain is "loaded" but no translations actually exist for that domain. It is a dead-simple class, essentially functioning as a no-op (as the name says). What would need to happen is get_translations_for_domain() would still need to be called, as this would break translation usage as written. While we could avoid calling NOOP_Translations, we'd still need to check whether the object returned is NOOP_Translations before avoiding the function call. I've looked at trying to squeeze performance out of this before — is it really that much overhead? How did you calculate these numbers?