Make WordPress Core

Opened 6 years ago

Closed 11 months ago

#43902 closed enhancement (wontfix)

Small refactoring to prevent multiple chr() calls in MO class

Reported by: likemusic's profile likemusic Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch needs-refresh close
Focuses: Cc:

Description


Attachments (1)

mo-small-optimize2.diff (3.6 KB) - added by likemusic 6 years ago.
Patch file

Download all attachments as: .zip

Change History (7)

@likemusic
6 years ago

Patch file

#1 @ayeshrajans
6 years ago

Hi @likemusic - welcome to Wordpress Trac!
I often go through performance tagged tickets and other PHP-related improvements. I'm just a regular volunteer with no commit privileges, so please take this just as an observation.

Here is a benchmark https://3v4l.org/vkjKg of the suggested change. There doesn't seem to be a noticeable performance difference between two approaches. However, it definitely looks like an improvement in code clarity, following DRY.

I believe there is some work undergoing to revamp this component with a modern approach. I'm sure someone with more understanding will comment soon.

#2 @likemusic
6 years ago

Hi @ayeshrajans !

Thanx for review. According to your test for old version php (before 7.0) using constant decrease time almost for 50%. It's good indicator and imho shoud be merged to develop branch because still many WP sites running on old php versions.

I made this fix because loading translations take most time of request handling after installing some extensions.

I looked for MO loader but can't find any other obvious way to increase translation loading performance.

Seems that it's requires to use some cache to not parse all mo files for every requests (that makes every 30 sec when admin page is opened in browser).

#3 @pento
5 years ago

  • Version trunk deleted

#4 @swissspidy
18 months ago

  • Keywords has-patch needs-refresh added
  • Milestone changed from Awaiting Review to Future Release

#5 @swissspidy
11 months ago

  • Focuses performance removed
  • Keywords close added

Looks like there is no performance gain with this change on newer PHP versions. Given that and the low number of sites running on PHP 5.6, I am removing the performance focus. Considering closing because the readability doesn't change much either IMO.

#6 @SergeyBiryukov
11 months ago

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

Hi there, thanks for the ticket!

I agree with comment:5. There is no performance benefit on PHP 7.1+ per the tests above, and with only 6.5% of sites running on PHP 5.6 or 7.0 as of May 2023, I think we should look at other ways to optimize translations instead.

Note: See TracTickets for help on using tickets.