1761464 - Browser freezes after enabling/disabling languages for spell checking

Status: VERIFIED FIXED

(Core :: Spelling checker, defect)

Description

[Hani Yacoub, Desktop QA]

Attachment: aboutsupport.txt

Affected versions

Firefox 100.0a1

Affected platforms

Windows 10 x64: Please check aboutsupport attached
macOS 11.6:
MacBook Pro (13-inch, 2019, Four Thunderbolt 3 ports)
Processor 2,4 GHz Quad-Core Intel Core i5
Memory 16 GB 2133 MHz LPDDR3

Steps to reproduce

1. Open Firefox.
2. Go to bugzilla.mozilla.org or any other websites.
3. Right click on the search field to open the context menu.
4. From the context menu make sure check spelling is enabled -> Languages.
5. Enable/Disable any of the languages.
6. Try right away to open the context menu or to switch to another tab.

Expected result

Browser shouldn't freeze after enabling/disabling languages for spell checking.

Actual Result

Browser freezes after enabling/disabling languages for spell checking.

Note

On the OS's mentioned in the affected platform the freeze lasted for 4-5 seconds.
On a Dell G5 laptop with processor Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz and 16GB RAM the freeze laster for 1 second.

Comment 1

[Hani Yacoub, Desktop QA]

Attachment: 2022-03-25_11h40_34.mp4

Comment 2

[Dan Minor [:dminor]]

This reproduces for me, but only with 5 or 6 dictionaries installed. It was fine, even on older machines, with two dictionaries. Changing the UI code to not wait on changing dictionaries makes things better on my machine. I'm going to check that we're not reloading all of the dictionaries when adding a single dictionary, or something similar, before changing the UI code because I'm surprised that the number of dictionaries installed would make a difference in this case.

Comment 3

[Dan Minor [:dminor]]

Hani collected a profile on an affected machine here: https://profiler.firefox.com/public/2wgmaw9g0z4mnsw4x5xtp28zce7qjg25wecp8yr/flame-graph/?globalTrackOrder=awe0w9&hiddenGlobalTracks=1w7&hiddenLocalTracksByPid=9808-0w3~4908-0~15460-0~10084-0~9488-01~12444-0~17936-0w9&localTrackOrderByPid=9808-40w3~4908-0~15460-0~10084-0~9488-10~12444-0~17936-a0w9&search=LdrpDispatchUserCallTarget&thread=0&timelineType=cpu-category&transforms=df-302~ff-1435~mf-1128~mf-1002&v=6

It looks like the time is being spent in loading the dictionary.

The async changes I mentioned in Comment 2 seem to help on some machines, but more testing is needed.

Comment 4

[Dan Minor [:dminor]]

It appears that this also happened in Firefox 99, https://profiler.firefox.com/public/87v44f6nc4x1a0cxnsytwrjtnsmsec6g5rjws9g/stack-chart/?globalTrackOrder=bwf0wa&hiddenGlobalTracks=1w9&hiddenLocalTracksByPid=14076-0w4~3800-0~11660-0wa&localTrackOrderByPid=14076-0w4~3800-0~11660-0wa&thread=0&timelineType=cpu-category&v=6

Comment 5

[Dan Minor [:dminor]]

It seems like there was always jank while loading a dictionary, but now that we're potentially loading 5 or 6 dictionaries at once, the jank lasts 5 or 6 times as long. It would be nice to move loading the dictionaries to a background thread, but we may be stuck keeping this on the main thread. In that case, we could try chaining promises together, so the main thread is not blocked for the full time it takes to load the dictionaries.

Comment 6

[Dan Minor [:dminor]]

So it looks like we can only use RLBoxHunspell from the main thread [1], so the best we can do here is to try to break up the dictionary loading so it doesn't block the main thread for several seconds in a row.

[1] https://searchfox.org/mozilla-central/rev/1ca8ea11406642df4a2c6f81f21d683817af568d/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#43

Comment 7

[Dan Minor [:dminor]]

QA has checked that the issue doesn't reproduce with the changes in this try job: https://treeherder.mozilla.org/jobs?repo=try&revision=685f80ae05ac05da508b84c140d43622c378e0a1. I'll be pushing it for review shortly.

Comment 8

[Dan Minor [:dminor]]

Attachment: Bug 1761464 - Return promise from InlineSpellChecker.selectDictionaries; r=gijs!

This returns a promise from selectDictionaries rather than using async/await
to synchronize sending events.

Comment 9

[Dan Minor [:dminor]]

Attachment: Bug 1761464 - Don't free dictionaries when they are no longer in use; r=smaug!

This adds an mEnabled flag which is used to hide dictionaries from the
spellchecker rather than freeing them. Loading dictionaries is an expensive
process which must occur on the main thread. By retaining dictionaries in
memory, we avoid having to reload them. If the number of dictionaries in use
exceeds a set number (10), we'll attempt to free any disabled dictionaries to
avoid memory use growing indefinitely.

Depends on D142556

Comment 10

[Dan Minor [:dminor]]

Attachment: Bug 1761464 - Load dictionaries lazily; r=smaug!

Loading dictionaries is time consuming and must occur on the main thread. By
loading them lazily, we can hopefully spread these loads across multiple calls
rather than loading them all at once, which results in noticeable jank on
slower systems.

Depends on D142557

Comment 11

[Dan Minor [:dminor]]

Attachment: Bug 1761464 - Pass nsCString to RLBoxHunspell::Create; r=smaug!

Depends on D142557

Comment 12

[Phabricator Automation]

Comment on attachment 9270234 [details]
Bug 1761464 - Return promise from InlineSpellChecker.selectDictionaries; r=gijs!

Revision D142556 was moved to bug 1762629. Setting attachment 9270234 [details] to obsolete.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/262849952d57
Don't free dictionaries when they are no longer in use; r=smaug
https://hg.mozilla.org/integration/autoland/rev/34396045f417
Pass nsCString to RLBoxHunspell::Create; r=smaug
https://hg.mozilla.org/integration/autoland/rev/0a103b6714e3
Load dictionaries lazily; r=smaug

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/262849952d57
https://hg.mozilla.org/mozilla-central/rev/34396045f417
https://hg.mozilla.org/mozilla-central/rev/0a103b6714e3

Comment 15

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 100.0a1 (2022-04-04) on Windows 10 x64, Ubuntu 20.04 x64 and on macOS 11.6.