1867474 - Bookmark tab... | Add bookmark | Location extremely slow (reloads all bookmarks every time?)

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[technosaur]

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

Steps to reproduce:

1. Have lots of bookmarks, or you won't notice it (I have more than 300k)
2. Right-click a tab
3. In the pop-up menu, click "Bookmark tab..."
4. In the "Add bookmark" dialog, under "Location", click "Choose"
5. Watch Firefox become unresponsive and your hard drive activity spike for (in my case) almost a minute
6. Close the "Add bookmark" dialog, go back to step (2) and do it again. Same thing again, no matter how often you do it.

Actual results:

Firefox becomes unresponsive and hard drive activity spikes, apparently reloading the entire bookmark database every time.

Profiler data:
https://share.firefox.dev/47WZIk0

Expected results:

Until yesterday's update, selecting a bookmark location in "Bookmark tab..." only took a few seconds at most. That was acceptable.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox::Bookmarks & History' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Marco Bonardo [:mak]]

I assume you were just updated to Firefox 121 beta 5?

300k bookmarks is a lot more than I'd expect, also in edge cases. Did an add-on create those?
Could you please attach a log from about:support (there's an Attach New File button above).

I suspect this is a regression of bug 1861811, though there's only limited amount of things we can do here, either we optimize for the most common case or for the edge case.

I'll try to build up a database with these many bookmarks...
can you tell me more about the structure of them, are there many folders, or is most bookmarks in Other Bookmarks? do you have many tags too?

Comment 3

[technosaur]

Attachment: Troubleshooting Information (about:support | Copy raw data to clipboard)

Comment 4

[Marco Bonardo [:mak]]

So far, I tried generating a 200k bookmarks + 10k tags, while I see slowdowns, they are not as severe, unless I fill-up all the memory. If I read correctly you have 4GB memory, that may partially explain this.
The new query is surely more memory intensive, while the old one was disk intensive.

I have an idea to optimize some of the views, so I may ask you to try a custom build.

Comment 5

[technosaur]

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

Firefox 121 beta 5?

Looks right, Help | About Firefox reports 121.0b5 (64-bit)

300k bookmarks [...] Did an add-on create those?

No, I've just been collecting them for a few decades.

Could you please attach a log from about:support (there's an Attach New File button above).

Done.

I suspect this is a regression of bug 1861811, though there's only limited amount of things we can do here, either we optimize for the most common case or for the edge case.

Without digging into the code, my impression is that the whole bookmark database is being reloaded every time I choose a location.

I can understand a tradeoff between slow application startup (if the database is loaded upfront) and slow response on first use, but not repeatedly reloading the database in the same session.

I'll try to build up a database with these many bookmarks...
can you tell me more about the structure of them, are there many folders,

A moderate amount of folders (less than 400) some nested down to a depth of 5 or 6. A few monster folders contain tens of thousands of bookmarks (I think one is over 50k).

or is most bookmarks in Other Bookmarks?

My Other Bookmarks is empty. Everything is under All Bookmarks | Bookmarks Menu.

do you have many tags too?

None at all. (I like tags, but this collection started out in simpler times. One day I'll restructure it, or more likely let a script do it for me.)

Hope this helps. Thanks for looking into it.

Comment 6

[technosaur]

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

So far, I tried generating a 200k bookmarks + 10k tags, while I see slowdowns, they are not as severe, unless I fill-up all the memory. If I read correctly you have 4GB memory, that may partially explain this.

I just tried exercising "Add bookmark" with Task Manager | Performance | Memory on screen, and it doesn't look like it. I barely budged, and even with Thunderbird and a couple other applications open, I have 1.5 GB free.

Makes sense, even if every bookmark was 1 kB, 300k of them would only be 300 MB.

Comment 7

[Marco Bonardo [:mak]]

Before doing anything, please backup your profile folder. I'd like to avoid any kind of dataloss, in case of mistakes.

Here is a test build for Windows 64bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/GRWGmeQmTaqiCTff2cQiYw/runs/0/artifacts/public/build/target.zip
Launch it with the -P command line option to open the Profile Manager, and create a new profile for it. You don't want to use your Beta profile.
Then copy over places.sqlite from your Beta profile to the new profile, the profile folder is easily findable using the Open Folder button in about:support.
It's better to do the file copy when Firefox is closed.

Please let me know if this build improves things.

Comment 8

[technosaur]

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

Please let me know if this build improves things.

This one runs really well! Can I keep it? :)

Comment 9

[Marco Bonardo [:mak]]

(In reply to technosaur from comment #8)

This one runs really well! Can I keep it? :)

Thank you.
Don't worry, the patch should be in the next Beta, and in Nightly in a few days, I'll attach it and ask for review now.
Using the temporary build may not be the best as it won't auto-update nor it has build optimizations.

Comment 10

[Marco Bonardo [:mak]]

Attachment: Bug 1867474 - Don't collect bookmark tags for ExcludeItems() Places queries. r=daisuke

Fetching tags for excludeItems queries doesn't make much sense as they only show
folders and queries, and in certain cases it may slow down bookmarks panels
considerably.

Comment 11

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

Set release status flags based on info from the regressing bug 1861811

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/143b93a3e77c
Don't collect bookmark tags for ExcludeItems() Places queries. r=daisuke,places-reviewers

Comment 13

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/143b93a3e77c

Comment 14

[Marco Bonardo [:mak]]

Attachment: Bug 1867474 - Don't collect bookmark tags for ExcludeItems() Places queries. r=daisuke

Fetching tags for excludeItems queries doesn't make much sense as they only show
folders and queries, and in certain cases it may slow down bookmarks panels
considerably.

Original Revision: https://phabricator.services.mozilla.com/D195115

Comment 15

[Phabricator Automation]

Uplift Approval Request

• Code covered by automated testing: no
• Is Android affected?: no
• Fix verified in Nightly: yes
• Risk associated with taking this patch: Low
• String changes made/needed: none
• User impact if declined: Users with many bookmarks may hit jank when opening a bookmark dialog
• Steps to reproduce for manual QE testing: While it would be great to reproduce this, I couldn't find a specific setup to make it obvious (creating 200k bookmarks wasn't sufficient). The user having the original problem reported builds with this fix to be working much better.
• Needs manual QE test: no
• Explanation of risk level: With this optimization we are avoiding some work that is not necessary for the specific bookmarks view.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9366725 [details]
Bug 1867474 - Don't collect bookmark tags for ExcludeItems() Places queries. r=daisuke

Approved for 121.0b7.

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/8833c90233aa