Closed Bug 132013 Opened 23 years ago Closed 22 years ago

Remove from list makes program unresponsive for too long.

Categories

(SeaMonkey :: Download & File Handling, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: ian, Assigned: bugzilla)

References

Details

(Keywords: perf, regression, Whiteboard: [adt2 rtm][verified on trunk],custrtm-)

Attachments

(2 files)

STEPS TO REPRODUCE 1. Select seven downloads in the Download Manager window. 2. Click "Remove From List". ACTUAL RESULTS Takes several seconds. EXPECTED RESULTS Takes no noticable time.
Keywords: perf
hixie, how many items did you have listed in the download manager? fwiw, when i had 3 items it removed 'em quickly... let me try more...
okay, now i see this when i have 10 items in the dl mgr --it takes about 4-5sec to remove them all. tested using 2002.03.20 comm bits on linux rh7.2, win2k and mac 10.1.3.
Keywords: nsbeta1
OS: Linux → All
Hardware: PC → All
I've had this take minutes, especially if the list had to scroll. The first time it happened, I thought the program had gone nuts, and I killed it. The next time I went to lunch and all was well when I got back.
Yup, CPU pegging makes this indistinguishable from hang. nsbeta1+ ->1.0
Keywords: nsbeta1nsbeta1+
Target Milestone: --- → mozilla1.0
outliner should help this, and I have a fix to batch it also.
ADT2
Whiteboard: [adt2]
*** Bug 137413 has been marked as a duplicate of this bug. ***
*** Bug 138089 has been marked as a duplicate of this bug. ***
*** Bug 141797 has been marked as a duplicate of this bug. ***
*** Bug 141964 has been marked as a duplicate of this bug. ***
Attached patch patch (deleted) — Splinter Review
this will help
Comment on attachment 82431 [details] [diff] [review] patch indicating gathered reviews (ben, hewitt on aim/irc)
Attachment #82431 - Flags: superreview+
Attachment #82431 - Flags: review+
There are other speed improvements to be made, but this bug was about batching, and I've checked it in on the trunk. marking fixed.
Status: NEW → RESOLVED
Closed: 23 years ago
Keywords: adt1.0.0+
Resolution: --- → FIXED
This commit added a warning on brad TBox: + 8. xpfe/components/download-manager/src/nsDownloadManager.cpp:89 (See build log excerpt) + Will be re-ordered to match declaration order +87 nsDownloadManager::nsDownloadManager() : mCurrDownloads(nsnull), +88 mBatches(0) +89 { +90 NS_INIT_ISUPPORTS(); +91 } +
changing to adt1.0.0- for beta and adding [adt2 rtm]
Keywords: adt1.0.0+adt1.0.0-
Whiteboard: [adt2] → [adt2 rtm]
vrfy'd fixed on the trunk. tested removing 10-15 entries in the dl mgr --took less than a second. linux rh7.2, 2002.05.07.10-trunk comm mac 10.1.4, 2002.05.07.03-trunk comm win2k, 2002.05.07.08-trunk comm
Whiteboard: [adt2 rtm] → [adt2 rtm][verified on trunk]
*** Bug 143113 has been marked as a duplicate of this bug. ***
*** Bug 144169 has been marked as a duplicate of this bug. ***
Why has this bug been marked as resolved/fixed ? It is very clearly still there in Release Candidate 2 for Windows NT. I currently have about 200 items completed downloads listed in Download Manager, and if I select all of them and click "Remove From List" Mozilla hangs completely. I can leave Mozilla sitting there for an hour and come back to find it still sitting there unchanged and completely unresponsive. When I use NT's "Task Manager" to kill Mozilla and then restart Mozilla, all of the items will still be there in Download Manager. This is reproducible also in W2K.
Sigh. This was checked into the trunk and was not checked into the branch thus it's not in RC2. If you want this, download a trunk build or just wait...
Seeing that this is has adt1.0.0-, I don't think that it will ever be on the branch...
Is this really in the trunk? I have build 2002051408 and when I select 5 items to be removed the removal takes a very long time (about 20s) and I have Athlon 1200MHz.
RE Comment #20 "Sigh. This was checked into the trunk and was not checked into the branch thus it's not in RC2. If you want this, download a trunk build or just wait..." This bug was not fixed in the trunk build I downloaded the day before RC2 was announced - and that trunk build was about 3 days AFTER this bug was marked as fixed. This bug is also not fixed in build 2002051508.
(Using build 2002051508) 1.) Got tired of waiting for a way to clean up download manager entries using Download Manager itself, so I simply tried deleting the file downloads.rdf from my profile. Works great - takes 2 seconds compared to hanging an Athlon 1800+ trying to do it with Download Manager. 2.) Why isn't Download Manager simply deleting this file when the user selects all entries and clicks "Remove From List" ???
Sairuh, I believe the ADT is expecting bugs that have been verified on the trunk to be put into the 'verified' state. This should get the bug onto queries they use for taking good fixes onto the branch.
Test using build 2002051521/linux: on my PIII500 the removal of 1845 entries in the download manager took 6 minutes and 35 seconds, during which all of mozilla was unresponsive and taking 100% CPU.
Reopening. That's not acceptable.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Remove from list should be batched → Remove from list should be batched some more
The real defect here is the speed of the operation, not how it is accomplished. updating summary. This operation should ideally complete in one-tenth of a second or less, and if it takes more than one second it should display some type of indication that a long operation is in progress, perhaps a progress indicator or at least a busy cursor.
Summary: Remove from list should be batched some more → Remove from list makes program unresponsive for too long.
Attached patch proposed patch (deleted) — Splinter Review
I'm proposing this as an additional measure to speed this up. Note that I've been unable to test this as someone's sloppy error checking *cough* means that every time I delete, I'm deluged with assertions about row indices out of bounds :-P I'd appreciate someone testing this and letting me know if it fixes the problem - basically we notify the template builder that a batch operation is about to begin so it doesn't respond until after the operation is complete.
er, the previous patch should be ammended to say 'notify the template builder' and not 'notify the datasource'
Comment on attachment 84414 [details] [diff] [review] proposed patch Good stuff. sr=blake
Attachment #84414 - Flags: superreview+
It's a bit ironic that the test for the optimisation has to be done once for every item in the list... wouldn't something like: for (i = 0; i < selectedItems.length-1; i++) { gDownloadManager.removeDownload(selectedItems[i].id); } gDownloadManager.endBatchUpdate(); observer.endUpdateBatch(ds); gDownloadManager.removeDownload(selectedItems[selectedItems.length-1].id); ...be better? Do we even have to have the last one done after the batching? Doesn't ending the batching trigger all the changes?
Comment on attachment 84414 [details] [diff] [review] proposed patch noting r=hewitt with hixie's proposed change, not that it has any noticeable improvement.
Attachment #84414 - Flags: review+
Some comments: - has anybody tested this patch? - as ben reported, I am seeing tons of assertions and the current behavior is horked: when removing several dls, one is missed. After that, removing dls does not work at all. I wonder if this patch is worth be checked in before that the other pbs are fixed. That's why I could not test is neither. - besides, there should not be methods called startUpdateBatch and startBatchUpdate. That's confusing. - in addition, the question raised by Ian is still unanswered: "Do we even have to have the last one done after the batching? Doesn't ending the batching trigger all the changes?" See above for reference.
- removing several downloads works flawlessly for me. - they're on two different interfaces, it's not *that* confusing. Anyway, I maintain that startUpdateBatch is wrong, which is why I named mine startBatchUpdate (anyway, this patch does not add or change any method names). - yes, we have to do the last one. no, ending the batch doesn't "trigger all the changes," I don't know what that means.
Checked in on trunk.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Keywords: adt1.0.0-adt1.0.0
Resolution: --- → FIXED
Blocks: 91351
Whiteboard: [adt2 rtm][verified on trunk] → [adt2 rtm][verified on trunk],custrtm-
Keywords: adt1.0.1
VERIFIED FIXED. Much faster now.
Status: RESOLVED → VERIFIED
Mass removing adt1.0.0, and adding adt1.0.1 because, we are now on 1.0.1.
Keywords: adt1.0.0
ADT1.0.1+, okay to checkin on branch once you have Driver's approval.
Keywords: adt1.0.1adt1.0.1+
Keywords: mozilla1.0.1
Comment on attachment 84414 [details] [diff] [review] proposed patch a=chofmann for 1.0.1. add the fixed1.0.1 keyword after checking in on the branch
Attachment #84414 - Flags: approval+
fixed on branch.
Keywords: fixed1.0.1
This is NOT fixed in build 2002061707 on WinNT4 or W2K. I had a 136 KB downloads.rdf and made the mistake of assuming that a fix for this bug would have been implemented in the trunk builds by now. I selected all items and clicked on "remove .." and five minutes later Mozilla was still totally unresponsive. CPU usage went up to 94% and stayed there. Eventually I gave up and used task manager to terminate Mozilla. That was on an Athlon 600 system with 256 MB. I next copied the same downloads.rdf file to an Athlon XP 1800+ system running W2K and the same version of Mozilla and tested it there - same thing: I gave up after 5 minutes. I am reasonably sure that the downloads.rdf file was not the problem: selecting and deleting just one item at a time worked - albeit still quite slowly: about 8 seconds on the Athlon 600 and about 5 seconds on the Athlon XP 1800+. In the future I will try to remember to simply use Explorer to delete the downloads.rdf file. What I can't understand is why Mozilla does not simply delete that file when the user is attempting to completely clean out his download history ? IE., deleting ALL items from download manager should be a two second operation even on Pentium 133. Much of this could be avoided if Mozilla would default to automatically removing finished downloads from the list. At the very least, put at option for this in the Preferences.
*** Bug 155040 has been marked as a duplicate of this bug. ***
i'll wait till bug 153480 is fixed on the branch before verifying this one --since i ran into that crasher just now with today's linux branch bits.
vrfy'd on the BRANCH using 2002.07.22 commercial branch builds on linux rh7.2, win2k and mac 10.1.5.
i just dare instaleld a trunk build after a couple of weeks ... since i wasnt using download manager i dont know when this happened but now that it was back on by default, i had a list of about 200 entries in it and it took 4 minutes to delete it ... so to me this problem is back again !
I also did some cleaning a few days ago and I had about 50 - 100 items on list. And like it was said it was slow as hell and I was unable to use any part of Mozilla during the proccess. Currently I use Mozilla 2003011508 on Windows XP.
QA Contact: sairuh → petersen
reopening as per comment and own experience
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
The question is how to deal with this. As far as I can see, there are several ways: - make the process of removing an entry faster - avoid non-responsiveness while it takes as long as it takes - avoid the problem by implementing an automatic delete feature, e.g. only keep the latest N completed downloads in the list (where N around 10) - add a feature for deleting the whole list by quickly zapping the RDF file instead of removing each entry individually.
well, this worked before ... when was did it change back tho ?
Keywords: regression
This bug was about batching multiple deletes. It was fixed. The issue that the most recent comments discuss is bug 161783
Status: REOPENED → RESOLVED
Closed: 23 years ago22 years ago
Resolution: --- → FIXED
putting back verify. If someone can demonstrate that batching does not work, _then_ reopen this bug.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
>A similiar problem was fixed a long time ago, see Bug 132013. For me it >works with all 1.7x and 1.8x on WinNT4. How large is your downloads.rdf file? >Does it work with Mozilla 1.7.x or 1.8a2? This was NOT fixed! In v1.8a6 the problem still appears! Selecting all items and Remove hangs Mozilla! My downloads.rdf is 4-5 Mb! Please REOPEN this bug!
Please see comment 51 and comment 52...
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: