Run this testcase: I got: > initialize: 54ms > getting innerHTML: 6ms > setting innerHTML: 513ms > after setting innerHTML: 12696ms > total: 13269ms on Nightly. but I got: > initialize: 24ms > getting innerHTML: 5ms > setting innerHTML: 15ms > after setting innerHTML: 163ms > total: 207ms It creates big hidden DOM tree at initializing. Then, getting the DOM tree as string with innerHTML. Then, setting innerHTML of an contenteditable element to the string. Finally, wait next event loop with setTimeout(()=>{}, 0).

Should this be a dupe of bug 1347525 or bug 1364861?

No. The key part in this bug is _editor_.

(In reply to Andrew Overholt [:overholt] from comment #1) > Should this be a dupe of bug 1347525 or bug 1364861? (In reply to Olli Pettay [:smaug] from comment #2) > No. The key part in this bug is _editor_. Yes. Editor may create and store a lot of nsRange instances and DOM tree change when editor has focus causes sending notifications to IME.

Was this test case representative of real world usage? We were trying to decide if this should be marked as a Quantum Flow P1 bug. Thank you.

Looks like that sync spell check (bug 1303749) makes this bad performance. https://perf-html.io/public/925e16c87180d795189212b8b6859d8964227f68/calltree/?callTreeFilters=prefix-0123456789abcdefghijk&range=3.7252_29.4471&thread=5 (In reply to Andrew Overholt [:overholt] from comment #4) > Was this test case representative of real world usage? We were trying to > decide if this should be marked as a Quantum Flow P1 bug. Thank you. Not sure. When I was working on bug 1250823, I created this testcase and I realized the slowness than Chrome.

I think what this bug is really about is scheduling of spell checking runnables. The test is just triggering some setTimeout() callbacks, and we're currently running these spell checker tasks with a higher priority than these callbacks, which seems pretty broken. We should consider dispatching them to the idle queue, I think.

We use a timeout of 1 second to ensure that spell checking will happen with some delay since it is visible to the user.

On my system, with the benchmark in comment 0, here are the before/after numbers: Before: initialize: 59 getting innerHTML: 5 setting innerHTML: 295 after setting innerHTML: 10164 total: 10523 After: initialize: 60 getting innerHTML: 6 setting innerHTML: 80 after setting innerHTML: 110 total: 256 Chrome: initialize: 25 getting innerHTML: 4 setting innerHTML: 28 after setting innerHTML: 171 total: 228 Now we're at least within the same ballpark!

I also tested this patch with https://public.etherpad-mozilla.org/p/wr-plan as a test case which requires a lot of spell checking... Without the 1000ms timeout, we wouldn't spell check much for a long time since our main thread wouldn't become idle.

Comment on attachment 8878243 [details] [diff] [review] Dispatch spell checker tasks to the idle queue Excellent!!

I also tested typing on the etherpad, especially for deleting words - much better than before! \o/ Etherpad does a bad thing when you hit backspace(delete) - it removes the whole paragraph and then inserts it back immediately, instead of just deleting character(s). This patch does help on this case and now I feels it's more responsive!

(In reply to Evelyn Hung [:evelyn] from comment #11) > Etherpad does a bad thing when you hit backspace(delete) - it removes the > whole paragraph and then inserts it back immediately, instead of just > deleting character(s). This patch does help on this case and now I feels > it's more responsive! Yeah, I do expect this patch to reduce a lot of the harm that the spell checker sync IPCs cause by moving them to happen at slightly better times hopefully.

Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/761fe192bce1 Dispatch spell checker tasks to the idle queue; r=masayuki

Backout by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7ec7da03d3dc Backout for test failures

Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2ab14fdd3af3 Dispatch spell checker tasks to the idle queue; r=masayuki

> when editor has focus causes sending notifications to IME. Hmm, do we do that also for users that do not use IME?

(In reply to Mats Palmgren (:mats) from comment #16) > > when editor has focus causes sending notifications to IME. > > Hmm, do we do that also for users that do not use IME? Yes. It's difficult to disable the notification for some some specific keyboard layout because user may change keyboard layout in the main process but we cannot notify remote process of that synchronously. So, if a keyboard layout or IME which requires text change notification or selection change notification is activated, the parent process doesn't have focused editor contents until the main process makes the remote process start observing contents and receives the result.