WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#32125 closed defect (bug) (fixed)

Emoji script: multiple second execution time when using AngularJS to insert text

Reported by: brad989 Owned by: azaozz
Milestone: 4.3 Priority: normal
Severity: normal Version: 4.2
Component: Formatting Keywords:
Focuses: Cc:

Description

I'm building a theme using AngularJS. After updating to 4.2, I experienced a significant slowdown when viewing posts (handled by custom javascript, not php theming functions), due to the emoji script. The script execution time appears to scale with the length of the post - longer posts making the execution time several seconds, with single word posts only needing a few ms.

Chrome's JS Profiler points at the parseNode function in wp-emoji-release.min.js (I unminified to inspect)
That function was called over 2000 times for one of my articles. I think the way that angular manipulates the DOM may be triggering duplicate calls.

Attachments (4)

Screen Shot 2015-04-24 at 2.19.22 PM.png (65.4 KB) - added by brad989 6 years ago.
JS Profiler
32125.patch (1.0 KB) - added by azaozz 6 years ago.
32125.2.patch (1.3 KB) - added by azaozz 6 years ago.
32125.3.patch (1.4 KB) - added by azaozz 6 years ago.

Download all attachments as: .zip

Change History (29)

#1 follow-up: @brad989
6 years ago

I, uh, accidentally uploaded a picture of my dog instead of the chrome profiler at first and I'm not sure how to delete it. It's my first time here. :)

#2 in reply to: ↑ 1 ; follow-up: @DrewAPicture
6 years ago

Replying to brad989:

I, uh, accidentally uploaded a picture of my dog instead of the chrome profiler at first and I'm not sure how to delete it. It's my first time here. :)

I've removed it for you :-)

#3 in reply to: ↑ 2 @brad989
6 years ago

Replying to DrewAPicture:

Replying to brad989:

I, uh, accidentally uploaded a picture of my dog instead of the chrome profiler at first and I'm not sure how to delete it. It's my first time here. :)

I've removed it for you :-)

Haha, thank you!

#4 @azaozz
6 years ago

  • Milestone changed from Awaiting Review to 4.3

Seems there are two ways this can be optimized:

  1. Get the new or modified nodes textContent and do twemoji.test() before running parse(). That is still a big regex but will be much faster in case there are no emoji chars.
  2. Implement something like Underscore's _.throttle() that will execute as soon as called but delay the next execution by 100ms -- 200ms.
Last edited 6 years ago by azaozz (previous) (diff)

#5 @DrewAPicture
6 years ago

  • Component changed from General to Formatting

#6 follow-up: @iseulde
6 years ago

How many DOM manipulations are you making? Is it there anywhere we can see it? If you're using JS for the theme it might be better to remove the script and parse user generated content as strings before using it.

#7 in reply to: ↑ 6 @brad989
6 years ago

Replying to iseulde:

How many DOM manipulations are you making? Is it there anywhere we can see it? If you're using JS for the theme it might be better to remove the script and parse user generated content as strings before using it.

So, weirdly, I only experience this issue on posts where I've copy pasted text from the internet into the new post dialog. Loading a post where I've manually typed text into the new post dialog works as expected.

You can see this here: ((link deleted - i've removed the script demonstrating the bug from the site, but I'm not doing outrageous DOM manipulation))

Might also be worth noting that I'm loading the data using the WP API.

How would I go about removing the emoji script for the time being?
EDIT: Just going to use this for now. https://wordpress.org/plugins/disable-emojis/

Last edited 6 years ago by brad989 (previous) (diff)

@azaozz
6 years ago

#8 follow-up: @azaozz
6 years ago

In 32125.patch: test if any added nodes contain text with emoji chars before parsing them. That optimizes/speeds up processing of added nodes quite a bit. The testing regex in Twemoji is quite large but still much better than parsing nodes needlessly.

@brad989: can you check if this fixes it in your case.

Last edited 6 years ago by azaozz (previous) (diff)

#9 in reply to: ↑ 8 @brad989
6 years ago

Replying to azaozz:

In 32125.patch: test if any added nodes contain text with emoji chars before parsing them. That optimizes/speeds up processing of added nodes quite a bit. The testing regex in Twemoji is quite large but still much better than parsing nodes needlessly.

@brad989: can you check if this fixes it in your case.

The patch does indeed speed up the execution time for posts without special character codes quite a bit (from ~80ms to 4ms).

However, when the posts include special character codes that aren't emojis (’ for apostrophes, for instance), I'm still getting 1000ms+ execution times when angular tries to compile it.


#10 follow-up: @Otto42
6 years ago

Related: #32197

More info: The problem I posted there only occurs when the "Enable full-height editor and distraction-free functionality." is enabled and you're in Text mode, not in Visual mode. Same profiler results.

#11 @azaozz
6 years ago

In 32335:

Emoji: before parsing added nodes test if the text in them contains emoji chars. That speeds up the processing quite a bit.
See #32125.

#12 in reply to: ↑ 10 ; follow-up: @azaozz
6 years ago

Replying to Otto42:

Related: #32197

Yes, suspecting this is the exact same problem with Chrome and probably WebKit. Note that the Twemoji regex is used for both testing and replacing the chars. It is a huge difference, according to the fiddle over 2800 times slower! @brad989, could you confirm the performance in Firefox and IE11.

This will eventually be fixed in the affected browsers, not sure how we can mitigate it until then. Perhaps we can try to make a shorter/faster regex to test for emoji chars, even if it is not as precise?

Last edited 6 years ago by azaozz (previous) (diff)

#13 @Otto42
6 years ago

FYI, I reported this issue to Chromium and created a jsfiddle to demonstrate the issue:

https://code.google.com/p/chromium/issues/detail?id=482998

https://jsfiddle.net/7vo8wteu/

Try that jsfiddle in Firefox and a few other browsers to see a marked difference. I'm convinced this has something to do with the v8 irregexp code, but no idea what. Maybe it has a different code path there for unicode optimization or something.

(for those who may have missed it in the other ticket)

Last edited 6 years ago by Otto42 (previous) (diff)

#14 @Otto42
6 years ago

@azaozz: Results for that jsfiddle on my Windows 8.1 laptop:

Firefox 37.0.2: 2 ms and 65ms. Results very consistent. Almost no variation.

IE 11.0.18: 525ms and 482ms. The results vary quite a bit, but always hover around 500ms for both tests. No appreciable difference. The unicode case appears to be slightly faster on most runs.

Chrome 43.0.2357.45 beta-m (64-bit): 2ms and 2880ms. Almost no variation.

#15 @azaozz
6 years ago

Yep, pretty much the same results here. Chrome 43/Win7 1ms and 2840ms...

Going to try to make a shorter regex, perhaps using ranges? Maybe will get better results in Chrome then.

#16 in reply to: ↑ 12 @brad989
6 years ago

Replying to azaozz:

@brad989, could you confirm the performance in Firefox and IE11.

Yep, I only experience the big delay on Chrome. Firefox, IE, and Safari are all much quicker.

@azaozz
6 years ago

#17 @azaozz
6 years ago

Came up with 32125.2.patch. Includes all lower emoji (apparently there are quite a few of them, see http://www.unicode.org/Public/emoji/1.0/emoji-data.txt), the "Supplementary characters" and the "Private Use Area # 2".

#18 @azaozz
6 years ago

Actually we can probably reduce the second regex to /[\uDC00-\uDFFF]/ (test for the second half of the surrogate pair) as we only need to check if there are emoji chars in the string, not match them.

@azaozz
6 years ago

#19 follow-up: @azaozz
6 years ago

In 32125.3.patch: added couple of missing chars to the singles, reduced the pair regex to test only for the second half of the surrogate pairs.

Tested with http://www.unicode.org/Public/emoji/1.0/emoji-style.html. Detects 1251 emoji, skips "U+00A9 copyright sign" and "U+00AE registered sign" (do we want to replace these with images too?). Needs more testing :)

#20 in reply to: ↑ 19 @brad989
6 years ago

Replying to azaozz:

32125.3.patch

This fixed it for me! Down to ~3ms for all posts.

#21 @azaozz
6 years ago

In 32347:

Emoji: add shorter/faster regex for testing for emoji chars in added nodes.
See #32125.

#22 @obenland
6 years ago

  • Owner set to azaozz
  • Status changed from new to assigned

#23 @Otto42
5 years ago

FYI: Chromium has now made a change to fix this issue, which will likely hit stable in a couple months:

https://code.google.com/p/chromium/issues/detail?id=482998

#24 @obenland
5 years ago

@azaozz, anything left to do here? Can this be closed?

#25 @obenland
5 years ago

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