Closed Bug 397424 Opened 17 years ago Closed 13 years ago

Downloads cause high CPU usage

Categories

(Toolkit :: Downloads API, defect)

x86
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla10
Tracking Status
firefox9 - ---
blocking2.0 --- -

People

(Reporter: Dolske, Assigned: felix)

References

Details

(Keywords: helpwanted, perf, regression, Whiteboard: [qa+])

Attachments

(7 files, 11 obsolete files)

21.83 KB, text/plain
Details
74.27 KB, text/plain
Details
109.82 KB, text/plain
Details
116.05 KB, application/force-download
Details
94.52 KB, text/plain
Details
3.19 KB, text/plain
Details
28.11 KB, patch
Gavin
: review+
Details | Diff | Splinter Review
Seems like there are some existing bugs on this, in different flavors, so this may or may not be a DUPE.

When downloading Solaris Express DVD images from http://developers.sun.com/sxde/download.jsp, Firefox consumes 100% CPU during the download. Closing the DM window doesn't help. The DM shows the file size (1GB), so this isn't bug 123334, I assume.

This might be interesting to profile with DTrace! :-)

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007092404 Minefield/3.0a9pre
203 firefox-bi  47.9%  1:47.09  16   195   852  57.6M  43.0M  85.2M   583M 

Although I see higher than this number (culled from 'top'), which is slightly upwards of 60%, I don't see 100% at any time while downloading a comparable ISO from Knoppix (ftp://ftp.kernel.org/pub/dist/knoppix).

Justin, can you reproduce this with a new profile consistently?  (Or even with the same profile, consistently?)
note that if you are on a mac, chances are it's a dual core, so it won't ever consume 100%....
I downloaded 3 1GB images this afternoon from Sun, and the same thing happened each time. I went back to verify before filing this bug, and the download started to eat CPU yet again.

top on Mac doesn't really adjust for the dual-core... It was actually reporting 100% to 107% CPU because of that. :-)
Was this fixed by bug 410131?
Nope. When downloading from the link in comment #0, I'm still seeing high CPU usage. Not 100% now, but ~50%.
Someone care to dtrace this?  I don't have 10.5...
I can see the same behavior with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008060704 Minefield/3.0pre ID:2008060704

It doesn't depend on a special file type. It always happens when downloading files. The cpu goes up to 60% and after pausing the download it is again on 30%. It's reproducible. I haven't used dtrace ever before. I could have a look because using a MacBook and having the fan on maximum speed isn't nice. Even the battery will be down much quicker. So I don't download any files when I'm not AC powered. Thinking about, I believe we should raise the severity of this bug.
Severity: normal → major
I also have high CPU usage when downloading any files in Firefox. I am currently using Firefox 3 RC2 and this is still a problem. OSX 10.5.3, MacBookPro. Included is a link to a screenshot of this. I have started to use curl instead just to avoid the high cpu.

http://www.digitalextortion.com/files/FF_High_CPU_DL.jpg

I tried to have a look at dtrace or Apples UI called Instruments. It's a bit complicated to get an application related cpu profile. Are there other tools around or how can profile a specific application? Shawn?
Stephen, do you have some more information e.g. an URL? 
Shark might be a little heavy-duty/advanced for what you're trying to do, but here it is: http://developer.apple.com/tools/shark_optimize.html (I've never used it myself, but have heard its accolades).
sayrer would be good to ask on irc, but I'm not really sure how to use dtrace or shark.  I think knowing what's going on in JS as well as C++ is likely to be useful in this case.
Paul, feel like taking a crack at this?
I can confirm that FF will use large amounts of CPU. I couldn't get it up to 100, but it was usually around 50%.

So I took a look at this case using Shark and some DTrace scripts.  The Shark output is too large for bugzilla, so I'll be putting that online somewhere. Some DTrace outputs will be uploaded too.

I'll be learning more about DTrace and Shark over the next few days and may be able to make better sense of the outputs.  
You'll notice here:
 * We're writing in 1MB chunks (you probably knew that)
 * Every few chunks, we lose 32K from 1 of the 1MB chunks and it gets written separately.
 * Other times we write to cache...

I think you can ignore the "urlclassifier3" stuff.
Attached file Dtrace js_functime (obsolete) —
Probably the most notable things are at the bottom.  It looks like some things are taking way long.

Also, this sample was taken over 1 minute, which means updateStatus was being run for ~3.6 seconds of 60.
A couple of questions:
1) is this just one donwnload?
2) do we have any idea what <null> in DownloadUtils.jsm is?  I see we have lots of anonymous functions in there, so we'll have to get that fixed (bug 438993)

My guess is that we are calling onProgressChange in DownloadProgressListener.js too much, and that's what calls all the other stuff which might be it.  Of course, I'm not totally sure since it's an anonymous function (bug 438998).  I'll get those fixed tomorrow (and hunt for someone to get the review) so we can get new results that are a bit more useful.
Yes, this was a single download. To be clear it was the file Justin mentioned in comment #0. Also, I think all the data shown here is from after unpausing or mid download (not beginning).

Here's the shark file if you can make heads or tails of it: http://dl.getdropbox.com/u/16967/Mozilla/bug397424/Session%203%20-%20Processor%20Bandwidth%20%28Intel%20Core%202%29%20of%20firefox-bin.mshark
Attached file DTrace js_functime 2
Here's a second look, from me clicking download to it finishing.  This was a ~700MB file, taking ~8:30 to download.
hrm - it doesn't seem to show JS code that is called from c++.

Nevertheless, I have an idea as to what's up.  I'll post a patch you can try out and see if it improves things.
Assignee: nobody → sdwilsh
We already limit progress listener updates to being at least 400 ms apart.  I'm not inclined to increase that, so I'm going to look into reducing the number of calls to some of the more expensive functions.
Attached patch v0.1 (obsolete) — Splinter Review
So far there's only one thing I've found that we can optimize out, and that's an additional call to isNil in DownloadUtils.jsm.  The good news is that we call that a lot, but the bad news is that it only saves us roughly .112545 seconds in your run.
Attachment #324902 - Attachment is obsolete: true
I'm also going to try and change how we manage strings here - I'm hoping it gives us a win.
Attached patch 0.2 (obsolete) — Splinter Review
This triggers an assertion in nsTextFormatter.cpp, and I haven't tracked down why yet.  However, once I get this working we'll be in *much* better shape since this removes *all* of the replaceInsert calls in DownloadUtils.jsm, which make up a large chunk of what we were calling according to dtrace.
Attachment #325019 - Attachment is obsolete: true
Shawn, shouldn't this affect all Hardware/OS?
Status: NEW → ASSIGNED
Comment on attachment 325058 [details] [diff] [review]
0.2

I expect the assertion to be about

>+transferSameUnits=%1$S of %3$S %4$S

The textformatter doesn't like non-consecutive numberings, IIRC.
(In reply to comment #27)
> >+transferSameUnits=%1$S of %3$S %4$S
> 
> The textformatter doesn't like non-consecutive numberings, IIRC.
Ugh!  That's really unfortunate, and means we can't use it for a number of our strings :(
Attached patch v0.3 (obsolete) — Splinter Review
Now assertion free.
Attachment #325058 - Attachment is obsolete: true
Comment on attachment 325260 [details] [diff] [review]
v0.3

In the localization notes, could you move from the #3 notation over to %3$S?

Didn't check the actual logic in DownloadUtils.jsm, fwiw.
I also need to change the identifier of the strings I've changed.  Just trying to get something up so people can see if it helps (I have yet to be able to reproduce this problem, which makes it hard to debug).
Shawn, you could start a TryServer build. When it's available I could run a test. I can clearly reproduce the issue on my side.
I don't know if this will apply cleanly to CVS, but I'll try.  The try server doesn't support patches to mozilla-central yet. :(
I did a test with the TryServer build. Sadly, the mentioned symptoms still exist. Having the build open without doing any user action, Minefield uses around 5% cpu load. Starting or resuming the download raise the cpu load up to 33% on my system. I did a couple of these tests while running the Ubuntu 8.04 download. It's clearly reproducible.
Do you see high CPU when the download manager is closed? How about if it's opened but your cursor is not over the download manager? And if the cursor is over an inactive download? And if the cursor is over the updating download?

I remember seeing a ton of assertions in debug builds relating to active downloads and redraw stuff.
Yes, even with the closed Download Manager the high cpu load is visible. So it's probably a networking or streaming issue?
That's strange. When I open an item from the main menu e.g. "Tools", while the download is running, I can make Firefox use 100% cpu load. After closing the menu it goes down to 30%. Saw this while trying to get answers for Edwards questions.
Might very well be.  We should at least fix what I've already got up in this bug though - we call String.replace *a lot*.
Well, we also have the status bar entry that is going to be doing all this work to generate new strings (although, not as much as the downloads window).  I don't think my patch does anything with that.
Attached file DTrace js_functime 3
I could also get the CPU up to ~100% with and without the DM window.

JS calls here seem to be running a little better, and less often.

Also, I'm downloading at 2-3 MB/s so I'm wondering how much that has to do with it (since high speeds usually hit CPU extra).
Shawn, shall I file the open menu issue as a new bug?
(In reply to comment #42)
> Shawn, shall I file the open menu issue as a new bug?
Are you talking about just opening any menu and having it show?  If so, file a new bug, although I'm not sure what the right component is (it isn't here).
Attached file test profile
I tried to minimize the profile I'm using. I hope it will also be happen on your side. Just do following steps to reproduce the issue:

1. Start Firefox with the test profile
2. Wait some seconds until the cpu load goes down to ~5%
3. Press Cmd+J to open the Download Manager
4. Press the Retry button and start the download of Ubuntu

Now the cpu load should go up to nearly 25% and down again when pausing the download.
(In reply to comment #43)
> Are you talking about just opening any menu and having it show?  If so, file a
> new bug, although I'm not sure what the right component is (it isn't here).

Yeah. I filed bug 439488 for that issue.
Attached patch v0.4 (obsolete) — Splinter Review
More work.  This updates our size and percentComplete properties before sending the DOWNLOAD_DOWNLOADING state changed notification, which allows us to save some setAttribute calls.

I need to add an xpcshell test for this behavior still, but I'm not sure how to make the server not give a file size.  I suspect Edward can provide a pointer there.

I don't think there is much more to optimize here, but I'll spend a bit more time on this this week.
Attachment #325260 - Attachment is obsolete: true
Attached patch v0.5 (obsolete) — Splinter Review
forgot to qrefresh before submitting...
Attachment #325303 - Attachment is obsolete: true
Attached file DTrace js_functime 4
With the most recent patch applied.

Still need to set up bandwidth limiting proxy so that I'm not DLing at high speed and possibly throwing off my CPU measurements
Attached patch v1.0 (obsolete) — Splinter Review
Alright - this is about all I can do to reduce the amount of work we do without modifying nsIStringBundle to handle the plural form stuff.  We should probably do that anyway, but not in this bug.
Attachment #325304 - Attachment is obsolete: true
Attachment #325438 - Flags: review?(rflint)
Attachment #325438 - Flags: review?(gavin.sharp)
It's a regression when comparing the cpu load against Firefox 2. A latest nightly build of Bon Echo only uses approximately 15% at maximum.
Keywords: regression
Product: Firefox → Toolkit
Comment on attachment 325438 [details] [diff] [review]
v1.0

>diff --git a/toolkit/mozapps/downloads/src/DownloadUtils.jsm
>+// This lazily initializes the string bundle upon first use.
>+__defineGetter__("gBundle", function() {
>+  delete gBundle;
>+  
>+  let bundle = Cc["@mozilla.org/intl/stringbundle;1"].
>+               getService(Ci.nsIStringBundleService).
>+               createBundle(kDownloadProperties);
>+  __defineGetter__("gBundle", function() bundle);
>+  return bundle;
>+});
You can just reference the global scope via |this| here and eliminate the need to define another getter.
Attachment #325438 - Flags: review?(rflint) → review+
Whiteboard: [needs review gavin]
Shawn, any progress on this bug? Is a new patch needed to get it reviewed by Gavin?
Flags: wanted1.9.1?
Whiteboard: [needs review gavin] → [needs review gavin][nees status update]
Whiteboard: [needs review gavin][nees status update] → [needs review gavin][needs status update]
(In reply to comment #52)
> Shawn, any progress on this bug? Is a new patch needed to get it reviewed by
> Gavin?
Like the whiteboard said - needs review from gavin.
Whiteboard: [needs review gavin][needs status update] → [has patch][needs review gavin]
Comment on attachment 325438 [details] [diff] [review]
v1.0

>- reduced the number of isNil checks in DownloadUtils.jsm's getDownloadStatus
>  function by one (down to two from three).
We got rid of isNil in bug 448583, but removing checks should still help.

Would it be bad in the spirit of speeding things up to add in a helper function? ;)

function getStr(aName, aVals) {
  return aVals ? gBundle.formatStringFromName(gStr[aName], aVals, aVals.length) :
    gBundle.GetStringFromName(gStr[aName]);
}

>+      let status = gBundle.formatStringFromName(gStr.statusFormat, values,
>+                                                values.length);
let status = getStr("statusFormat", values);
>+    return gBundle.formatStringFromName(name, values, values.length);
return getStr("transfer" + name, values);
>+      return [gBundle.GetStringFromName(gStr.timeUnknown), aLastSec];
return [getStr("timeUnknown"), aLastSec];
>+      timeLeft = gBundle.GetStringFromName(gStr.timeFewSeconds);
timeLeft = getStr("timeFewSeconds");

>+    return [aBytes, gBundle.GetStringFromName(gStr.units[unitIndex])];
Odd one out is this one because of the array index...
Comment on attachment 325438 [details] [diff] [review]
v1.0

This no longer applies cleanly, unfortunately. Looks OK, though.
Attachment #325438 - Flags: review?(gavin.sharp)
Whiteboard: [has patch][needs review gavin] → [needs new patch]
(In reply to comment #54)
> Would it be bad in the spirit of speeding things up to add in a helper
> function? ;)
I think that abstracts away a bit too much and makes the coder harder to follow what is going on.
Attached patch v1.1 (obsolete) — Splinter Review
unbitrot
Attachment #325438 - Attachment is obsolete: true
Attachment #383800 - Flags: review?(gavin.sharp)
Whiteboard: [needs new patch] → [needs review gavin]
Comment on attachment 383800 [details] [diff] [review]
v1.1

>diff --git a/toolkit/mozapps/downloads/content/DownloadProgressListener.js b/toolkit/mozapps/downloads/content/DownloadProgressListener.js

>+        dl.setAttribute("maxBytes", aDownload.size);

>-    download.setAttribute("maxBytes", aDownload.size);

Not sure you can safely make this change, since aMaxTotalProgressBytes may change during the download, as discussed on IRC.

>diff --git a/toolkit/mozapps/downloads/src/DownloadUtils.jsm b/toolkit/mozapps/downloads/src/DownloadUtils.jsm

>-    if (aMaxBytes == null)
>-      aMaxBytes = -1;
>     if (aSpeed == null)
>       aSpeed = -1;
>-    if (aLastSec == null)
>+    if (aMaxBytes == null) {
>+      aMaxBytes = -1;
>+
>+      // If we do not know our max bytes, we will not know what our last seconds
>+      // were either, so we can save a check.
>       aLastSec = Infinity;
>+    }
>+    else if (aLastSec == null) {
>+      aLastSec = Infinity;
>+    }

Not sure this is worth the trouble, really - makes the code slightly harder to read, and a single null-check in JS isn't going to have any kind of significant perf impact.

>       // Only show minutes for under 1 hour unless there's a few minutes left;
>       // or the second pair is 0.
>       if ((aSeconds < 3600 && time1 >= 4) || time2 == 0) {
>-        timeLeft = replaceInsert(gStr.timeLeftSingle, 1, pair1);

>+        timeLeft = gBundle.formatStringFromName(gStr.timeLeftDouble,

This looks like a mistake (unintentional change of string name).

Would be nice to verify the strings are still correct in all of the cases you're changing - maybe unit tests can help there?

r=me with those addressed.
Attachment #383800 - Flags: review?(gavin.sharp) → review+
Whiteboard: [needs review gavin] → [no longer needs review gavin because gavin has awesomely reviewed it already]
Assignee: sdwilsh → nobody
Brahmana would you be interested in helping out?
Status: ASSIGNED → NEW
Keywords: helpwanted
Can we get further progress on this bug? It is really weird to see a spike of about 20-30% of cpu load while a file gets downloaded. What about the attached patch? I assume it will be bit-rotted again.
blocking2.0: --- → ?
Flags: wanted1.9.1?
While very unlikeable, I don't think we should hold the release on a 2007 bug with a patch from over a year ago. If someone unrots the patch and it's tested, then request approval.
blocking2.0: ? → -
reproduced it today on nightly. 9.0a1 (2011-08-19)
Can we please get this bug tracked for one of our next releases? This is a kinda annoying in terms of cpu load issue and also drains my batteries a lot while traveling. I'm always hesitated these days to download stuff while I'm not connected to AC. A CPU load of about 30-40% only because we are downloading a file is not acceptable. A patch has been reviewed and only seems to get unbitrotted. Can we please get someone assigned to it? Thanks.
We shouldn't forget about this bug again. Asking for qa tracking.
Whiteboard: [no longer needs review gavin because gavin has awesomely reviewed it already] → [qa?][no longer needs review gavin because gavin has awesomely reviewed it already]
Can I request to have this as a blocker for release 9?

I have been fed up for many years having to use an alternative browser or wget to download large files.
I've un-rotted the old patch and made some of the requested changes. I wasn't around so the following comment:

>>diff --git a/toolkit/mozapps/downloads/content/DownloadProgressListener.js b/toolkit/mozapps/downloads/content/DownloadProgressListener.js

>>+        dl.setAttribute("maxBytes", aDownload.size);

>>-    download.setAttribute("maxBytes", aDownload.size);

>Not sure you can safely make this change, since aMaxTotalProgressBytes may change during the download, as discussed on IRC.

doesn't really mean anything to me. Review would probably be cool.
Assignee: nobody → ffung
Attachment #383800 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #562694 - Flags: review?(sdwilsh)
Attachment #562694 - Flags: review?(gavin.sharp)
Whiteboard: [qa?][no longer needs review gavin because gavin has awesomely reviewed it already] → [qa+][no longer needs review gavin because gavin has awesomely reviewed it already]
Whiteboard: [qa+][no longer needs review gavin because gavin has awesomely reviewed it already] → [qa+]
(In reply to Henrik Skupin (:whimboo) from comment #66)
> We shouldn't forget about this bug again. Asking for qa tracking.

(In reply to Simon Howes from comment #67)
> Can I request to have this as a blocker for release 9?
> 
> I have been fed up for many years having to use an alternative browser or
> wget to download large files.

I would like to see this fixed, but that's not what tracking flags are for. This isn't novel in Firefox 9, it's just something we'd like fixed ASAP. So tracking-, but I am very happy to see our intern putting up patches here.
(In reply to Felix Fung (:felix) from comment #68)
> >>-    download.setAttribute("maxBytes", aDownload.size);
> 
> >Not sure you can safely make this change, since aMaxTotalProgressBytes may change during the download, as discussed on IRC.
> 
> doesn't really mean anything to me. Review would probably be cool.

This comment meant that is wasn't OK to change to only set maxBytes in onDownloadStateChange for DOWNLOAD_DOWNLOADING - you need to continue setting it in onProgressChange.
Ran this DTrace with the patch applied while the download was in progress.
Moved max progress back to onProgressChange
Attachment #562694 - Attachment is obsolete: true
Attachment #563322 - Flags: review?(sdwilsh)
Attachment #563322 - Flags: review?(gavin.sharp)
Attachment #562694 - Flags: review?(sdwilsh)
Attachment #562694 - Flags: review?(gavin.sharp)
Comment on attachment 563322 [details] [diff] [review]
Reduce CPU Usage During Downloads

>diff --git a/toolkit/components/downloads/nsDownloadManager.cpp b/toolkit/components/downloads/nsDownloadManager.cpp

>+NS_IMPL_CLASSINFO(nsDownload, NULL, nsIClassInfo::MAIN_THREAD_ONLY, NS_DOWNLOAD_CID);

I don't think you actually want/need MAIN_THREAD_ONLY?

>@@ -228,30 +188,43 @@ let DownloadUtils = {

>-    if (aMaxBytes < 0)

>+    if (total < 0) {

This looks like an improper merge of http://hg.mozilla.org/mozilla-central/rev/b7500203ad6d .

Looks good otherwise, though so far I've only interdiffed based on my last review. Given the age of this it might be a good idea to go over it again in its entirety.
Attachment #563322 - Flags: review?(sdwilsh)
Attachment #563322 - Flags: review?(gavin.sharp)
Attachment #563322 - Flags: review-
Additional Changes:
- Removed MAIN_THREAD_ONLY flag
- Fixed out-of-date merge issue
Attachment #563322 - Attachment is obsolete: true
Attachment #563600 - Flags: review?(sdwilsh)
Attachment #563600 - Flags: review?(gavin.sharp)
Gavin or Shawn, do you have any chance to get to the review request on this bug? Thanks.
Keywords: perf
Comment on attachment 563600 [details] [diff] [review]
Reduce CPU Usage During Downloads

Shawn said he wrote the original patches, so probably shouldn't review it.
Attachment #563600 - Flags: review?(sdwilsh)
Comment on attachment 563600 [details] [diff] [review]
Reduce CPU Usage During Downloads

>diff --git a/toolkit/mozapps/downloads/DownloadUtils.jsm b/toolkit/mozapps/downloads/DownloadUtils.jsm

There are two calls to replaceInsert here that haven't been updated. Is there no test coverage for this code? That makes me nervous, these changes are tricky and hard to thoroughly review.

Some other nits:

>+    let values = [];

This code could be simplified:

let transfer = DownloadUtils.getTransferTotal(aCurrBytes, aMaxBytes);
let [rate, unit] = DownloadUtils.convertByteUnits(aSpeed);
let [timeLeft, newLast] = DownloadUtils.getTimeLeft(seconds, aLastSec);

let params = [transfer, rate, unit, timeLeft];
let status = gBundle.formatStringFromName(gStr.statusFormat, values,
                                           values.length);
return [status, , newLast];

>+    }
>+    else if (progressUnits == totalUnits) {

>-    } else {

>+    }
>+    else {

omit this change (and the other like it below). Just use same-line braces everywhere.

>+      let (values = [time1, unit1]) {
>+        var pair1 = gBundle.formatStringFromName(gStr.timePair, values,
>+                                                 values.length);
>+      }
>+
>+      let (values = [time2, unit2]) {
>+        var pair2 = gBundle.formatStringFromName(gStr.timePair, values,
>+                                                 values.length);
>+      }

These let blocks aren't useful, you can just pass [time1, unit1] directly.
Attachment #563600 - Flags: review?(gavin.sharp) → review-
Additional Changes:
- Addressed above changed

There is a good amount of test coverage for DownloadUtils.jsm in
../toolkits/mozapps/downloads/tests/unit
Attachment #563600 - Attachment is obsolete: true
Attachment #569909 - Flags: review?(gavin.sharp)
(In reply to Felix Fung (:felix) from comment #78)
> There is a good amount of test coverage for DownloadUtils.jsm in
> ../toolkits/mozapps/downloads/tests/unit

Did those tests fail with the prior buggy versions of this patch?
Yes
Comment on attachment 569909 [details] [diff] [review]
Reduce CPU Usage During Downloads

>diff --git a/toolkit/mozapps/downloads/DownloadUtils.jsm b/toolkit/mozapps/downloads/DownloadUtils.jsm

>-      let pair1 = replaceInsert(gStr.timePair, 1, time1);
>-      let pair2 = replaceInsert(gStr.timePair, 1, time2);
>+      var pair1 =
>+        gBundle.formatStringFromName(gStr.timePair, [time1, unit1], 2);
>+      var pair2 =
>+        gBundle.formatStringFromName(gStr.timePair, [time2, unit2], 2);

nit: don't change these to var (use let)

r=me with that!
Attachment #569909 - Flags: review?(gavin.sharp) → review+
This sent maemo red, presumably because of the stray ';' on this line:

  NS_IMPL_CLASSINFO(nsDownload, NULL, 0, NS_DOWNLOAD_CID);
https://hg.mozilla.org/mozilla-central/rev/57d2ada7f864
https://hg.mozilla.org/mozilla-central/rev/4e13cd176312
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Depends on: 700220
There're several bugreports on downloading with high speed causing high CPU usage.
Eg.
https://bugzilla.mozilla.org/show_bug.cgi?id=306570
https://bugzilla.mozilla.org/show_bug.cgi?id=270312

I was considering to put my comment in a bugreport that is not closed (marked as resolved) yet, but since I only experience the problem on Mac OS X, I decided to go with this thread.

I still experience the high CPU issue with or without the Downloads window being open.
My test download URL is the 1080p version of Big Buck Bunny from the Italian source:
http://www.bigbuckbunny.org/index.php/download/
http://mirrorblender.top-ix.org/peach/bigbuckbunny_movies/big_buck_bunny_1080p_h264.mov

The server does provide a Content-Length header field so Bug #270312 does not apply in this case.

I've tested on Mac OS X 10.6.8 and Ubuntu 10.10. In both cases I used Firefox 14.0.1.
On Linux I saw 10-14% CPU usage from Firefox while downloading the large file and having the Downloads window opened or closed made no difference.
On Mac OS X I saw 50-60% CPU usage with the Downloads window opened and 40-50% usage with the Downloads window closed.

I must add that in both cases (Linux and Mac) I tested with a whole new profile (ie. no extensions) and all plugins disabled.

The problem with high CPU usage is definitely not resolved on the Mac.
Furthermore I've tested the same download with Safari (5.1.7) too and CPU usage was below 10%!
And there was no other high user process (or system load) during the download so the reason for Safari to perform that much better is not because it offloads the task to some system/kernel component.
I've tested with Chrome 21.0.1180.75 too and it showed a constant 25% CPU usage.
Even if we consider Safari as a privileged app (which I'm sure it is) on the Mac, the test with Chrome proves that CPU usage for large file downloads can be a lot lower than what Firefox provides now.
Btw. I cannot change the status of this bugreport to REOPENED. I guess it's by design. I ask the maintainer of this bug to reopen it. I'll provide more feedback if you require any logs/traces to track the problem down.
I duped bug 306570 to this one. There's clearly some users out there who still encounter this. Please keep this bug "resolved" until we have a reproducible testcase.

@bit2, what version of Firefox were you testing? If you weren't using the latest Nightly can you please try it? You can get it from nightly.mozilla.org.

Note that we've moved the Download Manager from it's own window to a notification panel in recent Firefox builds. It might be interesting to see if the performance has improved as a result.
Keywords: testcase-wanted
Also note that this was allegedly fixed in Firefox 10, whereas bit2 tested Firefox 14.0.1. It's quite possible that we fixed this by reintroduced a performance regression within the 24 weeks between Firefox 10 and 14.
Bit2, I would highly encourage to create a new bug. Please don't reopen this one or attach any other testcase to it. Looks like there is still a remaining issue which needs investigation and has to be fixed. But as said, on a new bug.
Keywords: testcase-wanted
Tested with Nightly (17.0a1 (2012-08-20)) too and it's all the same. This time I tested only on Mac since other platforms (eg. Linux) do not seem to be affected by the problem/bug.

With Nightly the CPU usage was the same as with 14.0.1. And the new Downloads notification panel caused the same difference: without it CPU usage was around 40-50%, with it CPU usage was around 50-60%.

Anthony: should I open a new ticket? I don't know how things go around here, who is the lead on this (who decides whether my findings should go into a separate ticket or not)?

Should I create a DTrace of the problem? Which Firefox version should I use (14.* or nightly)? It seems that the problem is present in both so if it's up to me, I'd choose 14.*.

Thanks to all who help figuring this out and fix the bug.
@bit2, please file a new ticket, thank you. If you can provide a DTrace please do so with the latest Nightly in your new ticket. Thanks
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #92)
> @bit2, please file a new ticket, thank you. If you can provide a DTrace
> please do so with the latest Nightly in your new ticket. Thanks

When doing this please also tell us the bug id. Thanks.
I'm currently looking at the output of top and Firefox is responsible for 296.0% CPU usage while I'm downloading a 2Gb movie file (unfortunately the file is password protected).  Did this bug ever really get fixed, or am I looking at a possible new issue?
(In reply to Rob Crowther from comment #94)
> I'm currently looking at the output of top and Firefox is responsible for
> 296.0% CPU usage while I'm downloading a 2Gb movie file (unfortunately the
> file is password protected).  Did this bug ever really get fixed, or am I
> looking at a possible new issue?

Please file a new bug report with steps to reproduce.
Rob, when you have filed the bug please let us know about the bug number. Thanks!
You need to log in before you can comment on or make changes to this bug.