Open Bug 1695129 Opened 3 years ago Updated 10 months ago

OOM crash while moving bookmarks

Categories

(Firefox :: Bookmarks & History, defect, P3)

Firefox 88
defect

Tracking

()

People

(Reporter: bced991, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: perf, Whiteboard: [snt-scrubbed][places-performance])

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

Steps to reproduce:

Firefox Nightly crashed while i was moving ~ 12,000 bookmarks (cut and paste) from bookmarks toolbar, or a certain menu category, to.. "Other Bookmarks", using the Bookmarks Manager window.

This should be possible, i have 16GB RAM and a well optimized PC. It went fine for a few minutes, had moved like 30% already (I selected all bookmarks a few times to see progress, the number in each directory) and then I selected main bookmarks overview ("All Bookmarks") to wait out the completion. It just crashed in the background at some point next.

This is a common operation of organizing bookmarks to your personal preference after importing (I imported HTML export from Chrome bookmarks) so it should be technically possible, even with such a huge amount of bookmarks. I believe it went fine in the past, although maybe back then i moved it (drag and drop) instead of "Cut" after selecting the ones to move.

Actual results:

Crash occured, the relevant crash was reported at: https://crash-stats.mozilla.org/report/index/1308e767-997c-4e1d-8fa9-adf5d0210226 (sent by me)

I guess something triggered a memory leak, and that this should be the focus of this bug report.

Expected results:

Firefox should be able to successfully move a significant amount of bookmarks, using cut and paste in the bookmarks manager. At least, given that the PC has enough RAM available when the aforementioned process commences.

It turns out that after relaunching Firefox following the crash, the whole operation of moving all bookmarks has completed after all, without intervention since the crash. That indicates it is possible the memory leak started, or continued, after it had already completed moving all bookmarks - as clearly, it had completed at the moment it crashed and thus any growth in memory usage should have ceased or even be released following completion (rather than crash).

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

Component: Untriaged → Bookmarks & History

We are aware of performance issues in the Bookmarks & History views, as visible from bug 734643. We plan to do some more analysis of the problem after some re-architecture work we are currently doing.

Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf
Priority: -- → P3
See Also: → 734643

I think I should write this here instead of opening a new bugreport:

I have 4795 tabs open.
I selected all with right click select all tabs, then tried to put them into a bookmark folder by rightclicking on a tab and selecting the option for it.

After around 3 mins the window where i can decide the bookmark folder name popped up. Obviously the program is still very slow, but I can click on the save button, which eventually makes the window disappear.

In the background, firefox starts to occupy more and more memory. I have 16GB of installed RAM, but obviously windows can use much more virtual memory using my SSD.

I tried this twice - the first time, firefox just crashed by disappearing without anything else; the second time i closed some other background programs first, and firefox still crashed, but at least this time it opened up a crash report window.

So according to task manager I started out with around 7GB (or maybe 9~10?) occupied memory on the second try (didn't monitor the first one), and firefox hiked it up to around 64GB memory, after which it crashed, which took about 10 mins after I clicked on the save button on the window where i can decide the folder name.

Event viewer says for my second try that firefox.exe (5160) used 42523848704 bytes, firefox.exe (17188) used 18207039488 bytes, and firefox.exe (19232) used 549019648 bytes.

Both tries did create a new bookmark folder with 4603 entries (selecting all in the folder with ctrl+a in the bookmark manager ctrl+shift+o), so having 1-2GB more memory doesn't seem to affect the number of saved bookmarks.
According to my manual count by scrolling through the tabs, I have about 29 blank pages ("new tab"), which don't seem to get saved into bookmarks, but that still leaves around 160 pages that are missing.
If firefox doesn't save link duplicates with this bookmarking method, that might account for the missing pages, but is in my opinion not intended behaviour.

Could there please be an option to save only the links into bookmarks, and not anything else? Clearly firefox does some other unnecessary bullshit in the background, as 5000 single-line links wouldn't take up 60GB+ memory on their own.

Thanks!

I noticed that Mayank Bansal subbed to this issue #, and because that might be indicative of upcoming work for fixing the underlying "performance issues in the Bookmarks & History views" (bug 734643) ..

I'd like to use being reminded of this issue to post a Web Archive version of the "Mozilla crash stats" details page, as it obviously expired its storage period:
https://web.archive.org/web/20210302234026/https://crash-stats.mozilla.org/report/index/1308e767-997c-4e1d-8fa9-adf5d0210226

Hope it's useful

Today i was trying to import my bookmarks .html (nearly 20,000 entries) again, and this hasn't been possible in a normal fashion due to insane RAM usage (27GB all the way to filling up my 32GB, causing a crash):

https://i.imgur.com/RBhXebP.png of my task manager

So this proves the issue is still present, even though the 'history' ticket #734643 got closed, it doesn't cover this situation so is there still plans to optimize this as well? With bookmarks? @mak

Flags: needinfo?(mak)

The problem is likely similar to the history one, when importing bookmarks the Library is open, and that alone causes a lot of work. It could be optimized in a similar way as the history case, so when we receive a large amount of bookmark-added notifications we give up with single updates and just do a result refresh.
We're still in the process of removing the old notifications system, though that work is wrapping up and then we could look at optimizations like this.

Flags: needinfo?(mak)

ok. In my last comment i forgot to mention that just like OP, it wasn't the importing of bookmarks HTML itself (that went smooth and quickly) but moving them from the folder it automatically imported them into, which was either "Bookmarks menu" or "Bookmarks bar", into "Other bookmarks". This is a select all, drag and drop operation within the bookmark manager menu, and it's the thing that goes very slow (without even having a visible progress counter unless you keep going back and forth between folders to see the item count) and eventually eats all that RAM until crash

In essense, you are moving files on disk when Firefox chooses the harshest way of moving these items. You may be done with it if you change what happens (in Firefox code) when you select, drag and drop between folders in Bookmarks manager. It should be doable in the same split moment that it takes to import a bookmarks HTML. Besides, you may avoid 90% of users that feel they have to do this, from doing it, by allowing a bookmarks HTML import to land in a user-specified folder instead of the automatic thing, after which users want to tailor it to their preferences and only have this highly unoptimized way for doing so.

The part where adding multiple tabs takes a long time before showing the dialog, it should have been fixed by the work related to bug 1744551.

The other part about memory usage, it is likely related to two reasons:

  1. the view code is not optimized to manage a batch change of thousands of entries (bug 1821664 may help a bit)
  2. PlacesTransactions module copies over all the bookmark information to memory. This is also discussed in bug 1824872.
Depends on: 1824872, 1744551
Whiteboard: [snt-scrubbed][places-performance]
You need to log in before you can comment on or make changes to this bug.