Opened 7 years ago
Closed 7 years ago
#41501 closed defect (bug) (invalid)
Emoji: Short-circuit wp_staticize_emoji() when there are no emoji to staticise.
Reported by: | pento | Owned by: | pento |
---|---|---|---|
Milestone: | Priority: | high | |
Severity: | major | Version: | 4.9 |
Component: | Emoji | Keywords: | has-patch needs-testing |
Focuses: | performance | Cc: |
Description (last modified by )
After [41043] (for #35293), there seem to be some circumstances where the new regex is noticeably slower, particularly with PHP < 7, and PCRE < 8.39.
This isn't noticeable for most sites, but does become a problem for sites with a particularly high volume of HTML email. (Looking at you, WP.com.)
To help mitigate that, we can short circuit the process with early checks for emoji - if there are none, we can return before running the regex.
Attachments (6)
Change History (17)
#3
@
7 years ago
Running performance tests here: https://travis-ci.org/pento/test-41501
#4
@
7 years ago
Thank you for the additional patch, @umangvaghela123!
Unfortunately, 41501.patch doesn't quite do the right thing - I've created 41501.2.diff, to show a little clearer what's going on.
It can return early, if the following conditions are met:
- It only contains ASCII. This is tested by
mb_check_encoding()
, if available, or thepreg_match()
, if it isn't. - AND, it doesn't contain the string '&#x', which possibly indicates a HTML encoded entity that we need to test further.
#5
@
7 years ago
@pento Thank you for your response.
If we check ASCII so it is fine to check check_ascii() which is already defined in wp-include/wp-db.php file.
#7
follow-up:
↓ 8
@
7 years ago
- Milestone changed from 4.8.1 to 4.9
Punting this to 4.9, as I can't find a good improvement, and 4.8.1 needs to go ahead.
#8
in reply to:
↑ 7
@
7 years ago
Replying to pento:
Punting this to 4.9, as I can't find a good improvement, and 4.8.1 needs to go ahead.
Seeing as we now know the new code has issues on large content should we not remove the existing changes to the RegEx from 4.8.1 so that we don't potentially cause issues on other sites with this Maintenance Release?
I'm going to do some performance tests with this patch, any further suggested changes would be appreciated.