1682434 - Firefox freezes for 5-7 sec when trying to highlight a long form history entry (getTokenMatches::lastIndexOf is very slow)

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[intox11]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

Go to URL bar, enter D letter (lower or capital does not matter) into it in order to get some web-sites autofilled, I am not even able to put anything after D as it freezes instantly after the input was made

Actual results:

Firefox freezes for 5-7 sec (shows Not responding message in top window bar)

Expected results:

My places.sqlite is quite huge (30 MB), so I expected every letter to cause Firefox to freeze, but I tried the whole alphabet, all other characters do not freeze Firefox even for a second

Comment 1

[intox11]

Forgot to mention, after freeze is gone, I can continue adding extra letters after the 1st "D" without any freezes

Comment 2

[intox11]

Turned off every single extension and tried SpeedyFox, did not help

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 4

[intox11]

The first suggestion if I put "d" into address bar is https://docs.google.com/, I suggested maybe it freezes Firefox for somewhat reason, cleared everything related to "docs.google" in History for the whole history period, the first suggestion is still docs.google.com
Maybe it can help. Not sure how to troubleshoot this in right way

Comment 5

[intox11]

Played around with browser.urlbar.maxRichResult , with this parameter value set to not more than 3 nothing freezes, but if it is set to 4 and up, I get the same behavior

Comment 6

[Marco Bonardo [:mak]]

Please go to about:support and try to run the Database Integrity check in the Places Database section. Once it's complete it will print a log, please post it here. Then restart. Did it improve?

Comment 7

[intox11]

it did not improve
but I noticed everything is ok with "D" letter when I open incognito window. safe mode did not help though

Task: checkIntegrity

• The places.sqlite database is sane
• The favicons.sqlite database is sane

Task: invalidateCaches

• The caches have been invalidated

Task: checkCoherence

• The database is coherent

Task: expire

• Database cleaned up

Task: originFrecencyStats

• Recalculated origin frecency stats

Task: vacuum

• Initial database size is 30720KiB
• The database has been vacuumed
• Final database size is 30720KiB

Task: stats

• Places.sqlite size is 30720KiB
• Favicons.sqlite size is 35264KiB
• pragma_user_version is 54
• pragma_page_size is 32768
• pragma_cache_size is -2048
• pragma_journal_mode is wal
• pragma_synchronous is 1
• History can store a maximum of 137971 unique pages
• Table moz_origins has 4837 records
• Table moz_places has 50177 records
• Table moz_historyvisits has 76430 records
• Table moz_inputhistory has 226 records
• Table moz_bookmarks has 73 records
• Table moz_bookmarks_deleted has 0 records
• Table moz_keywords has 0 records
• Table sqlite_sequence has 1 records
• Table moz_anno_attributes has 2 records
• Table moz_annos has 238 records
• Table moz_items_annos has 0 records
• Table moz_meta has 8 records
• Table sqlite_stat1 has 17 records
• Index sqlite_autoindex_moz_origins_1
• Index sqlite_autoindex_moz_inputhistory_1
• Index sqlite_autoindex_moz_bookmarks_deleted_1
• Index sqlite_autoindex_moz_keywords_1
• Index sqlite_autoindex_moz_anno_attributes_1
• Index moz_places_url_hashindex
• Index moz_places_hostindex
• Index moz_places_visitcount
• Index moz_places_frecencyindex
• Index moz_places_lastvisitdateindex
• Index moz_places_guid_uniqueindex
• Index moz_places_originidindex
• Index moz_historyvisits_placedateindex
• Index moz_historyvisits_fromindex
• Index moz_historyvisits_dateindex
• Index moz_bookmarks_itemindex
• Index moz_bookmarks_parentindex
• Index moz_bookmarks_itemlastmodifiedindex
• Index moz_bookmarks_dateaddedindex
• Index moz_bookmarks_guid_uniqueindex
• Index moz_keywords_placepostdata_uniqueindex
• Index moz_annos_placeattributeindex
• Index moz_items_annos_itemattributeindex

Task: _refreshUI

Comment 8

[Marco Bonardo [:mak]]

Thanks, everything there looks good.

Could you please, in addition, go to about:telemetry, open the Slow SQL section, and check if in the list there's any query taking a long time (more than 1000ms)?
It's interesting that you say it works fine in Private Windows, it must be something we skip for private browsing.
What is your default search engine?

Comment 9

[intox11]

Can't find "Slow SQL" section, where is it? https://prnt.sc/w3e5fw
(In reply to Marco Bonardo [:mak] from comment #8)

Thanks, everything there looks good.

Could you please, in addition, go to about:telemetry, open the Slow SQL section, and check if in the list there's any query taking a long time (more than 1000ms)?
It's interesting that you say it works fine in Private Windows, it must be something we skip for private browsing.
What is your default search engine?

Comment 10

[intox11]

My default search engine is Google, I already tried to switch it to something else, I tried setting Amazon as a default search engine, did not take any effect there

(In reply to Marco Bonardo [:mak] from comment #8)

Thanks, everything there looks good.

Could you please, in addition, go to about:telemetry, open the Slow SQL section, and check if in the list there's any query taking a long time (more than 1000ms)?
It's interesting that you say it works fine in Private Windows, it must be something we skip for private browsing.
What is your default search engine?

Comment 11

[Marco Bonardo [:mak]]

Thanks, it's possible there were no slow SQL, so the problem could be elsewhere.

We have a performance profiler that may help figuring out what's up here, you can find instructions at https://profiler.firefox.com/, this allows to start a registration before reproducing the problem, and stop it after, and should give us useful information about what is taking so much time.
If you could capture a profile and post the link to it here, hopefully it would show the problem.
Thank you a lot for helping us figuring this out.

Comment 12

[intox11]

Ok sir here you go. I started with browser.urlbar.maxRichResult = 10 with opened blank tab, then put "d" into Address bar, then waited for freeze to be gone, then stopped logging
https://share.firefox.dev/3r03X9B

Comment 13

[intox11]

For browser.urlbar.maxRichResult = 3 (no freeze)
same actions - stated on opened new tab, put "d", stopped logging
https://share.firefox.dev/3ni3AVH

Comment 14

[intox11]

For browser.urlbar.maxRichResult = 5 just in case you might want it
https://share.firefox.dev/3gRE6vY

Comment 15

[intox11]

For browser.urlbar.maxRichResult = 10 and letter "a" instead of "d"
https://share.firefox.dev/3mr7ha9

Comment 16

[Marco Bonardo [:mak]]

I think the problem is some really long past search you made, and we try to match "d" in that long string to properly highlight it.
That would explain why this doesn't reproduce in private browsing mode, since Search Suggestions are disabled there.

Could you please try setting browser.urlbar.maxHistoricalSearchSuggestions to 0 in about:config and tell me if it helps?

Comment 17

[intox11]

For browser.urlbar.maxRichResult = 10 and browser.urlbar.maxHistoricalSearchSuggestions = 0 and letter "d", without touching mouse, just to eliminate excessive mouse input records (used CRTL SHIFT 1/2 to record)
https://share.firefox.dev/38aZW9A

Comment 18

[Marco Bonardo [:mak]]

so, did setting maxHistoricalSearchSuggestions to 0 help or not? I see a long pause in the first profile that I can't find here.
I see the long pause in comment 12 and comment 14.

Comment 19

[intox11]

yes it did help no freeze at all(In reply to Marco Bonardo [:mak] from comment #18)

so, did setting maxHistoricalSearchSuggestions to 0 help or not? I see a long pause in the first profile that I can't find here.
I see the long pause in comment 12 and comment 14.

Comment 20

[Marco Bonardo [:mak]]

Thank you, we identified the problem and will try to have a fix for Firefox 85. You can keep that setting to 0 for now, and revert it once this fix hits release, I'm sorry for this issue.

Comment 21

[intox11]

that is alright! thank you, so it is not because of my history is too huge?
(In reply to Marco Bonardo [:mak] from comment #20)

Thank you, we identified the problem and will try to have a fix for Firefox 85. You can keep that setting to 0 for now, and revert it once this fix hits release, I'm sorry for this issue.

Comment 22

[Marco Bonardo [:mak]]

No, it's a bug in Firefox, not due to your history.

Comment 23

[intox11]

Thank you
just in case you want record for this issue without mouse inputs and letter "d"
browser.urlbar.maxRichResult = 10 and browser.urlbar.maxHistoricalSearchSuggestions = 2
https://share.firefox.dev/3qUlNLj
(In reply to Marco Bonardo [:mak] from comment #22)

No, it's a bug in Firefox, not due to your history.

Comment 24

[Marco Bonardo [:mak]]

Attachment: Bug 1682434 - Don't store long strings in search history. r=Standard8

Comment 25

[Marco Bonardo [:mak]]

Attachment: Bug 1682434 - Limit textruns and substring matching for search suggestions. r=adw

Depends on D99954

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ffb755b259c
Don't store long strings in search history. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/4fb5a850c536
Limit textruns and substring matching for search suggestions. r=adw

Comment 27

[Dorel Luca [:dluca]]

Backed out 2 changesets (bug 1682434) for Browser-chrome in browser/components/search/test/browser/browser_contentSearchUI.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=324948041&repo=autoland&lineNumber=2539

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=TSJm8Vy0RYWCi27ESEikUA.0&resultStatus=pending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&revision=4fb5a850c53620fcea2309921b340d2bd701c0fa\

Backout:
https://hg.mozilla.org/integration/autoland/rev/e8e0ae9ae6d98094213f1535e9f063a79cc64bfd

Comment 28

[Marco Bonardo [:mak]]

This test is just too slow, because autocomplete is extremely slow with long textruns... not sure how to solve that, I could have to investigate the autocomplete performance, since content is still using the old code.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/231589dd6888
Don't store long strings in search history. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/92ebbdb83e7e
Limit textruns and substring matching for search suggestions. r=adw

Comment 30

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/231589dd6888
https://hg.mozilla.org/mozilla-central/rev/92ebbdb83e7e

Comment 31

[Marco Bonardo [:mak]]

Comment on attachment 9193632 [details]
Bug 1682434 - Limit textruns and substring matching for search suggestions. r=adw

Beta/Release Uplift Approval Request

• User impact if declined: If the user somehow executed a very long search (for example searching for a data uri, even if by mistake), the next time they type a letter contained in that search the address bar may hang for seconds.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: This requires to add an entry from the browser console using FormHistory, because the other patch in this bug prevents from adding too long strings to search history.
  Execute from the console:
  await UrlbarUtils.addToFormHistory({formHistoryName: "searchbar-history"}, "made-up-very-long-value");
  On my system the string must be at least 15k chars to start causing slowdowns, but this may vary on a system basis.
  Things to check:

1. Before the patches values longer than 255 chars can be added to search history, just by searching for them. Note some engines may fail on too long strings. after the patch it should not be possible to add to search history strings longer than 255 chars
2. if search history contains a very long string, typing text matching it in the urlbar should not cause major slowdowns

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): minor changes to already existing code, covered by tests.
• String changes made/needed:

Comment 33

[Julien Cristau [:jcristau]]

Comment on attachment 9193631 [details]
Bug 1682434 - Don't store long strings in search history. r=Standard8

approved for 85.0b5

Comment 34

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4e1e5de25c16
https://hg.mozilla.org/releases/mozilla-beta/rev/b7cdec5f7ff4