Closed Bug 428459 Opened 16 years ago Closed 16 years ago

Trying to delete a complex folder with many subfolders hangs Minefield

Categories

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

defect

Tracking

()

RESOLVED WORKSFORME
Firefox 3.5

People

(Reporter: ehume, Assigned: dietrich)

References

Details

(Keywords: perf, Whiteboard: [patch in dependent bugs][re-triage after dependent bugs get fixed][tsnap])

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre

Trying to delete a Places folder with three layer of subfolders causes Minefield to hang.

Reproducible: Always

Steps to Reproduce:
1. Select a Places folder that has more than 100 bookmarks in three levels of subfolders, either in menu or Library.
2. Right-click and select Delete from context menu.

Actual Results:  
First level bookmarks deleted. Subfolders not deleted. Contents of folders not deleted.

Browser hangs, must be killed from Task manager.

Expected Results:  
Folder should be deleted.

With today's build I removed places.sqlite, places.sqlite-journal and the bookmarkbackups folder in my Minefield test profile. I opened Minefield, found - as expected - an empty Library, closed and restarted Minefield. This reconstituted the Most Visited folder on the Bookmarks Toolbar, left the rest of the toolbar empty.

Then I imported my 4368+ bookmark collection from my fx2 bookmarks.html file. 

Then I copied the contents of my fx2 Bookmarks Toolbar Folder and pasted them into the fx3 Bookmarks Toolbar. I now had a fully populated toolbar. Places menu, sidebar and Library all functioning well.

After browsing a bit, tried to delete the old (now unnecessary) fx2 Bookmarks Toolbar Folder from menu. The browser hung, could not be recovered. After killing this with Task Mgr, reopened browser without restoring the old session. This time tried deleting the folder from the Library. Same results. 

I waited several minutes each of the 4 times I tried the deletion. The CPU usage approximated 1-2% after the initial spike to perhaps 15%. There was disk activity. Since this is a WinXP system, the disk is usually quiet. 

Martijn has a copy of my bookmarks, if you need it.
Tony, could we get this tested with our big history file, too?
Keywords: qawanted
I was finally able to delete all of the old folder. But even the last bit took more than a minute, and the browser was frozen during that time. 
Hmm, I tested this by creating a folder called "test", copying my entire bookmarks and pasting it repeatedly in to the fold test till there were about ~3500 items. I pressed deleted and the browser hung for a mere 20 seconds or so, using 25 - 30% CPU on an AMD X2 38000+

I then repeated the test by creating a test folder again, copying about ~1400 items in to it, going in to one of the sub folders, copying about the same in to that, going in to another sub folder of test copying about the same and going in to another sub folder of test. CPU usage was anywhere from 20 - 40%, memory usage ballooned to 320 MiB and only seems to have recovered to about 180 MiB. The whole process took about 1 min 30 seconds. While inconvenient I'm not sure it's unexpected.
Hmm. Only 90 seconds. I just sat through the deletion of my old toolbar folder and it took 510 seconds - more than 8 minutes.

In Bug #415201 comment #26, Martijn noted "The slowdown happens when every link in the bookmarks.html file is unique and when it has a last_charset="ISO-8859-1" attribute," which apparently explained why my bookmark set is so **** SQLite. But my bookmarks are simply a long-maintained set that I doubt is much different from any other old set of bookmarks. Who would totally remake a set of bookmarks? And there are other people who have been on the Web for more than ten years.

Anyway, it seems that Damian has confirmed this bug. Can it be marked confirmed?
here a complex delete took about 120s, with high disk's churnings
it's not so common for a user delete a lot of his bookmarks in bunch, still some perf investigation could be done.

I doubt this is mostly due to TransactionManager, it has to save all infos for deleted nodes (and so it has to catch all those infos) to restore them.
confirming as a perf one
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf
Summary: Trying to delete a complex Places folder hangs Minefield → Trying to delete a complex folder with many subfolders hangs Minefield
I also have this problem with a bookmark collection of 4.1 MB if I export it as html file. The memory consumption jumps to 600 MB and after more than ten minutes waiting I killed the process.
No longer blocks: 432706
OS: Windows XP → All
Hardware: PC → All
Version: unspecified → Trunk
Flags: blocking-firefox3.1?
I had some problems with this, too, especially for folders containing Live Bookmarks. Deleting the top folder [My Feeds] just hung my browser for way too long. I killed it and then looked at deleting the subfolders one by one. This worked, but was still excruciatingly slow.
Approving the blocking request, this really hurts when managing bookmarks, and there've been many complaints about it.
Flags: blocking-firefox3.1? → blocking-firefox3.1+
Priority: -- → P2
Target Milestone: --- → Firefox 3.1b2
Depends on: 432706
Reporter(s), can you please test with a nightly build since bug 418643 was fixed, and see if the performance has improved for you?
Well, it does delete the subfolders.

OTOH, it takes a long time, and while it is doing those deletions Firefox is unusable. Periodically a this-script-is-taking-too-long message will appear. Clicking Yes in the dialog box will stop the process.
apart the better delete (deleting subfolders), do you notice any improvement in time?

I still think that, apart observers problems from bug 432706, a good part of the slowdown (the disk churning at least) is due to the transaction manager that asks information on EVERY deleted item, also if we already have those infos. Plus it asks for each info separately, so one query for title, one for index, one for container... and so on.
an idea i tried in bug 432706 was to pass an hash of known infos when building the transaction, so that we ask only for missing ones, alternatively we could provide a bookmarks service method that returns all infos for an item with a single query.
It is certainly faster: previously the process never finished. So, whatever the time is, it is an improvement over infinity.
Dietrich: is this a hard blocker for b2? We're no worse off than the shipping version atm, and it feels like something we can fix between now and final.
Whiteboard: [patch needed]
No, doesn't need to block b2, retargeting.
Whiteboard: [patch needed] → [patch in dependent bugs]
Target Milestone: Firefox 3.1b2 → Firefox 3.1
Keywords: qawanted
Whiteboard: [patch in dependent bugs] → [patch in dependent bugs][re-triage after dependent bugs get fixed]
Assignee: nobody → dietrich
can others please test with a current nightly and report results?

i retested now that all dependent bugs are fixed: deleting thousands of bookmarks in hundreds of folders and subfolders took about 10 seconds. i did get the osx pinwheel, but CPU was not pegged. previously this would take minutes, with CPU pegged the entire time.

this result is enough to take the issue off the blocking path for 3.1. however, two other issues should be addressed in a subsequent release:

1. after deleting, i hit cmd+z to undo, which took about 30 seconds and did peg the CPU for a short period.

2. in order to not block UI at all, we need to move some of the backend work to a background thread.
filed bug 472264 for improving undo performance.

marking this bug as WFM due to the problem being resolved via dependent bugs. please re-open if you are still able to reproduce the problem.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
shouldn't this be reopened since bug 432706 is. and what about the blocking+?
Target Milestone: Firefox 3.1 → Firefox 3.5
Whiteboard: [patch in dependent bugs][re-triage after dependent bugs get fixed] → [patch in dependent bugs][re-triage after dependent bugs get fixed][tsnap]
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.