Bug 397424 - Downloads cause high CPU usage. r=gavin

This addresses an issue with the download manager that can cause high CPU usage
when there is an active download.  The underlying issue is the frequency of
updates that the download progress listener receives.
Things changed:
- reduced the number of null checks in DownloadUtils.jsm's getDownloadStatus
  function by one (down to two from three).
- obtain and format strings from the nsIStringBundle.  This removes all the
  calls to String.replace in DownloadUtils.jsm.
- modifies the download manager back-end to update the percentComplete and size
  property on downloads before dispatching a state changed notification for
  downloads entering the DOWNLOAD_DOWNLOADING state.  This saves us two calls to
  setAttribute on downloads that we know how big they are, and saves us the same
  two calls to setAttribute for indeterminate downloads as well as not
  dispatching a ValueChange event on the progressmeter every time
  onProgressChange is called on the DownloadProgressListener.
- has nsDownload implement nsIClassInfo so we do not need to QueryInterface when
  going through the list of active downloads in both the download manager's UI
  and the browser's taskbar UI.
This commit is contained in:
Felix Fung 2011-11-03 15:25:55 -07:00
parent 6bd2aae9fe
commit 42b50d5d59
6 changed files with 130 additions and 165 deletions

View File

@ -8190,7 +8190,7 @@ let DownloadMonitorPanel = {
let maxTime = -Infinity;
let dls = gDownloadMgr.activeDownloads;
while (dls.hasMoreElements()) {
let dl = dls.getNext().QueryInterface(Ci.nsIDownload);
let dl = dls.getNext();
if (dl.state == gDownloadMgr.DOWNLOAD_DOWNLOADING) {
// Figure out if this download takes longer
if (dl.speed > 0 && dl.size > 0)

View File

@ -46,6 +46,7 @@
#include "mozIStorageService.h"
#include "nsIAlertsService.h"
#include "nsIClassInfoImpl.h"
#include "nsIDOMWindow.h"
#include "nsIDownloadHistory.h"
#include "nsIDownloadManagerUI.h"
@ -2130,8 +2131,14 @@ nsDownloadManager::ConfirmCancelDownloads(PRInt32 aCount,
////////////////////////////////////////////////////////////////////////////////
//// nsDownload
NS_IMPL_ISUPPORTS4(nsDownload, nsIDownload, nsITransfer, nsIWebProgressListener,
nsIWebProgressListener2)
NS_IMPL_CLASSINFO(nsDownload, NULL, 0, NS_DOWNLOAD_CID);
NS_IMPL_ISUPPORTS4_CI(
nsDownload
, nsIDownload
, nsITransfer
, nsIWebProgressListener
, nsIWebProgressListener2
)
nsDownload::nsDownload() : mDownloadState(nsIDownloadManager::DOWNLOAD_NOTSTARTED),
mID(0),
@ -2399,6 +2406,11 @@ nsDownload::OnProgressChange64(nsIWebProgress *aWebProgress,
if (resumableChannel)
(void)resumableChannel->GetEntityID(mEntityID);
// Before we update the state and dispatch state notifications, we want to
// ensure that we have the correct state for this download with regards to
// its percent completion and size.
SetProgressBytes(0, aMaxTotalProgress);
// Update the state and the database
rv = SetState(nsIDownloadManager::DOWNLOAD_DOWNLOADING);
NS_ENSURE_SUCCESS(rv, rv);

View File

@ -42,31 +42,37 @@ dontLeavePrivateBrowsingButton=Stay in Private Browsing Mode
downloadsCompleteTitle=Downloads Complete
downloadsCompleteMsg=All files have finished downloading.
# LOCALIZATION NOTE (statusFormat2): — is the "em dash" (long dash)
# #1 transfer progress; #2 rate number; #3 rate unit; #4 time left
# LOCALIZATION NOTE (statusFormat3): — is the "em dash" (long dash)
# %1$S transfer progress; %2$S rate number; %3$S rate unit; %4$S time left
# example: 4 minutes left — 1.1 of 11.1 GB (2.2 MB/sec)
statusFormat2=#4 — #1 (#2 #3/sec)
statusFormat3=%4$S — %1$S (%2$S %3$S/sec)
bytes=bytes
kilobyte=KB
megabyte=MB
gigabyte=GB
# LOCALIZATION NOTE (transferSameUnits, transferDiffUnits, transferNoTotal):
# #1 progress number; #2 progress unit; #3 total number; #4 total unit
# examples: 1.1 of 333 MB; 11.1 MB of 3.3 GB; 111 KB
transferSameUnits=#1 of #3 #4
transferDiffUnits=#1 #2 of #3 #4
transferNoTotal=#1 #2
# LOCALIZATION NOTE (transferSameUnits2):
# %1$S progress number; %2$S total number; %3$S total unit
# example: 1.1 of 333 MB
transferSameUnits2=%1$S of %2$S %3$S
# LOCALIZATION NOTE (transferDiffUnits2):
# %1$S progress number; %2$S progress unit; %3$S total number; %4$S total unit
# example: 11.1 MB of 3.3 GB
transferDiffUnits2=%1$S %2$S of %3$S %4$S
# LOCALIZATION NOTE (transferNoTotal2):
# %1$S progress number; %2$S unit
# example: 111 KB
transferNoTotal2=%1$S %2$S
# LOCALIZATION NOTE (timePair): #1 time number; #2 time unit
# LOCALIZATION NOTE (timePair2): %1$S time number; %2$S time unit
# example: 1 minute; 11 hours
timePair=#1 #2
# LOCALIZATION NOTE (timeLeftSingle): #1 time left
timePair2=%1$S %2$S
# LOCALIZATION NOTE (timeLeftSingle2): %1$S time left
# example: 1 minute remaining; 11 hours remaining
timeLeftSingle=#1 remaining
# LOCALIZATION NOTE (timeLeftDouble): #1 time left; #2 time left sub units
timeLeftSingle2=%1$S remaining
# LOCALIZATION NOTE (timeLeftDouble2): %1$S time left; %2$S time left sub units
# example: 11 hours, 2 minutes remaining; 1 day, 22 hours remaining
timeLeftDouble=#1, #2 remaining
timeLeftDouble2=%1$S, %2$S remaining
timeFewSeconds=A few seconds remaining
timeUnknown=Unknown time remaining
@ -79,7 +85,7 @@ doneStatus=#1 — #2
doneSize=#1 #2
doneSizeUnknown=Unknown size
# LOCALIZATION NOTE (doneScheme): #1 URI scheme like data: jar: about:
doneScheme=#1 resource
doneScheme2=%1$S resource
# LOCALIZATION NOTE (doneFileScheme): Special case of doneScheme for file:
# This is used as an eTLD replacement for local files, so make it lower case
doneFileScheme=local file
@ -95,7 +101,7 @@ stateDirty=Blocked: Download may contain a virus or spyware
# LOCALIZATION NOTE (yesterday): Displayed time for files finished yesterday
yesterday=Yesterday
# LOCALIZATION NOTE (monthDate): #1 month name; #2 date number; e.g., January 22
monthDate=#1 #2
monthDate2=%1$S %2$S
fileDoesNotExistOpenTitle=Cannot Open %S
fileDoesNotExistShowTitle=Cannot Show %S

View File

@ -1,4 +1,5 @@
/* ***** BEGIN LICENSE BLOCK *****
/* vim: sw=2 ts=2 sts=2 expandtab filetype=javascript
* ***** BEGIN LICENSE BLOCK *****
* Version: MPL 1.1/GPL 2.0/LGPL 2.1
*
* The contents of this file are subject to the Mozilla Public License Version
@ -84,76 +85,32 @@ __defineGetter__("gDecimalSymbol", function() {
const kDownloadProperties =
"chrome://mozapps/locale/downloads/downloads.properties";
// These strings will be converted to the corresponding ones from the string
// bundle on use
let kStrings = {
statusFormat: "statusFormat2",
transferSameUnits: "transferSameUnits",
transferDiffUnits: "transferDiffUnits",
transferNoTotal: "transferNoTotal",
timePair: "timePair",
timeLeftSingle: "timeLeftSingle",
timeLeftDouble: "timeLeftDouble",
let gStr = {
statusFormat: "statusFormat3",
transferSameUnits: "transferSameUnits2",
transferDiffUnits: "transferDiffUnits2",
transferNoTotal: "transferNoTotal2",
timePair: "timePair2",
timeLeftSingle: "timeLeftSingle2",
timeLeftDouble: "timeLeftDouble2",
timeFewSeconds: "timeFewSeconds",
timeUnknown: "timeUnknown",
monthDate: "monthDate",
monthDate: "monthDate2",
yesterday: "yesterday",
doneScheme: "doneScheme",
doneScheme: "doneScheme2",
doneFileScheme: "doneFileScheme",
units: ["bytes", "kilobyte", "megabyte", "gigabyte"],
// Update timeSize in convertTimeUnits if changing the length of this array
timeUnits: ["seconds", "minutes", "hours", "days"],
};
// This object will lazily load the strings defined in kStrings
let gStr = {
/**
* Initialize lazy string getters
*/
_init: function()
{
// Make each "name" a lazy-loading string that knows how to load itself. We
// need to locally scope name and value to keep them around for the getter.
for (let [name, value] in Iterator(kStrings))
let ([n, v] = [name, value])
gStr.__defineGetter__(n, function() gStr._getStr(n, v));
},
/**
* Convert strings to those in the string bundle. This lazily loads the
* string bundle *once* only when used the first time.
*/
get _getStr()
{
// Delete the getter to be overwritten
delete gStr._getStr;
// Lazily load the bundle into the closure on first call to _getStr
let getStr = Cc["@mozilla.org/intl/stringbundle;1"].
getService(Ci.nsIStringBundleService).
createBundle(kDownloadProperties).
GetStringFromName;
// _getStr is a function that sets string "name" to stringbundle's "value"
return gStr._getStr = function(name, value) {
// Delete the getter to be overwritten
delete gStr[name];
try {
// "name" is a string or array of the stringbundle-loaded "value"
return gStr[name] = typeof value == "string" ?
getStr(value) :
value.map(getStr);
} catch (e) {
log(["Couldn't get string '", name, "' from property '", value, "'"]);
// Don't return anything (undefined), and because we deleted ourselves,
// future accesses will also be undefined
}
};
},
};
// Initialize the lazy string getters!
gStr._init();
// This lazily initializes the string bundle upon first use.
__defineGetter__("gBundle", function() {
delete gBundle;
return this.gBundle = Cc["@mozilla.org/intl/stringbundle;1"].
getService(Ci.nsIStringBundleService).
createBundle(kDownloadProperties);
});
// Keep track of at most this many second/lastSec pairs so that multiple calls
// to getTimeLeft produce the same time left
@ -189,28 +146,14 @@ let DownloadUtils = {
let seconds = (aSpeed > 0) && (aMaxBytes > 0) ?
(aMaxBytes - aCurrBytes) / aSpeed : -1;
// Update the bytes transferred and bytes total
let status;
let (transfer = DownloadUtils.getTransferTotal(aCurrBytes, aMaxBytes)) {
// Insert 1 is the download progress
status = replaceInsert(gStr.statusFormat, 1, transfer);
}
let transfer = DownloadUtils.getTransferTotal(aCurrBytes, aMaxBytes);
let [rate, unit] = DownloadUtils.convertByteUnits(aSpeed);
let [timeLeft, newLast] = DownloadUtils.getTimeLeft(seconds, aLastSec);
// Update the download rate
let ([rate, unit] = DownloadUtils.convertByteUnits(aSpeed)) {
// Insert 2 is the download rate
status = replaceInsert(status, 2, rate);
// Insert 3 is the |unit|/sec
status = replaceInsert(status, 3, unit);
}
// Update time remaining
let ([timeLeft, newLast] = DownloadUtils.getTimeLeft(seconds, aLastSec)) {
// Insert 4 is the time remaining
status = replaceInsert(status, 4, timeLeft);
return [status, newLast];
}
let params = [transfer, rate, unit, timeLeft];
let status = gBundle.formatStringFromName(gStr.statusFormat, params,
params.length);
return [status, newLast];
},
/**
@ -233,20 +176,31 @@ let DownloadUtils = {
let [total, totalUnits] = DownloadUtils.convertByteUnits(aMaxBytes);
// Figure out which byte progress string to display
let transfer;
if (aMaxBytes < 0)
transfer = gStr.transferNoTotal;
else if (progressUnits == totalUnits)
transfer = gStr.transferSameUnits;
else
transfer = gStr.transferDiffUnits;
let name, values;
if (aMaxBytes < 0) {
name = gStr.transferNoTotal;
values = [
progress,
progressUnits,
];
} else if (progressUnits == totalUnits) {
name = gStr.transferSameUnits;
values = [
progress,
total,
totalUnits,
];
} else {
name = gStr.transferDiffUnits;
values = [
progress,
progressUnits,
total,
totalUnits,
];
}
transfer = replaceInsert(transfer, 1, progress);
transfer = replaceInsert(transfer, 2, progressUnits);
transfer = replaceInsert(transfer, 3, total);
transfer = replaceInsert(transfer, 4, totalUnits);
return transfer;
return gBundle.formatStringFromName(name, values, values.length);
},
/**
@ -267,7 +221,7 @@ let DownloadUtils = {
aLastSec = Infinity;
if (aSeconds < 0)
return [gStr.timeUnknown, aLastSec];
return [gBundle.GetStringFromName(gStr.timeUnknown), aLastSec];
// Try to find a cached lastSec for the given second
aLastSec = gCachedLast.reduce(function(aResult, aItem)
@ -300,25 +254,26 @@ let DownloadUtils = {
let timeLeft;
if (aSeconds < 4) {
// Be friendly in the last few seconds
timeLeft = gStr.timeFewSeconds;
timeLeft = gBundle.GetStringFromName(gStr.timeFewSeconds);
} else {
// Convert the seconds into its two largest units to display
let [time1, unit1, time2, unit2] =
DownloadUtils.convertTimeUnits(aSeconds);
let pair1 = replaceInsert(gStr.timePair, 1, time1);
pair1 = replaceInsert(pair1, 2, unit1);
let pair2 = replaceInsert(gStr.timePair, 1, time2);
pair2 = replaceInsert(pair2, 2, unit2);
let pair1 =
gBundle.formatStringFromName(gStr.timePair, [time1, unit1], 2);
let pair2 =
gBundle.formatStringFromName(gStr.timePair, [time2, unit2], 2);
// 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.timeLeftSingle,
[pair1], 1);
} else {
// We've got 2 pairs of times to display
timeLeft = replaceInsert(gStr.timeLeftDouble, 1, pair1);
timeLeft = replaceInsert(timeLeft, 2, pair2);
timeLeft = gBundle.formatStringFromName(gStr.timeLeftDouble,
[pair1, pair2], 2);
}
}
@ -364,7 +319,7 @@ let DownloadUtils = {
0);
} else if (today - aDate < (24 * 60 * 60 * 1000)) {
// After yesterday started, show yesterday
dateTimeCompact = gStr.yesterday;
dateTimeCompact = gBundle.GetStringFromName(gStr.yesterday);
} else if (today - aDate < (6 * 24 * 60 * 60 * 1000)) {
// After last week started, show day of week
dateTimeCompact = aDate.toLocaleFormat("%A");
@ -373,8 +328,7 @@ let DownloadUtils = {
let month = aDate.toLocaleFormat("%B");
// Remove leading 0 by converting the date string to a number
let date = Number(aDate.toLocaleFormat("%d"));
dateTimeCompact = replaceInsert(gStr.monthDate, 1, month);
dateTimeCompact = replaceInsert(dateTimeCompact, 2, date);
dateTimeCompact = gBundle.formatStringFromName(gStr.monthDate, [month, date], 2);
}
let dateTimeFull = dts.FormatDateTime("",
@ -437,11 +391,12 @@ let DownloadUtils = {
// Check if we need to show something else for the host
if (uri.scheme == "file") {
// Display special text for file protocol
displayHost = gStr.doneFileScheme;
displayHost = gBundle.GetStringFromName(gStr.doneFileScheme);
fullHost = displayHost;
} else if (displayHost.length == 0) {
// Got nothing; show the scheme (data: about: moz-icon:)
displayHost = replaceInsert(gStr.doneScheme, 1, uri.scheme);
displayHost =
gBundle.formatStringFromName(gStr.doneScheme, [uri.scheme], 1);
fullHost = displayHost;
} else if (uri.port != -1) {
// Tack on the port if it's not the default port
@ -479,7 +434,7 @@ let DownloadUtils = {
if (gDecimalSymbol != ".")
aBytes = aBytes.replace(".", gDecimalSymbol);
return [aBytes, gStr.units[unitIndex]];
return [aBytes, gBundle.GetStringFromName(gStr.units[unitIndex])];
},
/**
@ -552,23 +507,7 @@ function convertTimeUnitsUnits(aTime, aIndex)
if (aIndex < 0)
return "";
return PluralForm.get(aTime, gStr.timeUnits[aIndex]);
}
/**
* Private helper function to replace a placeholder string with a real string
*
* @param aText
* Source text containing placeholder (e.g., #1)
* @param aIndex
* Index number of placeholder to replace
* @param aValue
* New string to put in place of placeholder
* @return The string with placeholder replaced with the new string
*/
function replaceInsert(aText, aIndex, aValue)
{
return aText.replace("#" + aIndex, aValue);
return PluralForm.get(aTime, gBundle.GetStringFromName(gStr.timeUnits[aIndex]));
}
/**

View File

@ -63,6 +63,7 @@ DownloadProgressListener.prototype = {
// Update window title in-case we don't get all progress notifications
onUpdateProgress();
let dl;
let state = aDownload.state;
switch (state) {
case nsIDM.DOWNLOAD_QUEUED:
@ -82,16 +83,26 @@ DownloadProgressListener.prototype = {
if (state == nsIDM.DOWNLOAD_FINISHED)
autoRemoveAndClose(aDownload);
break;
case nsIDM.DOWNLOAD_DOWNLOADING: {
dl = getDownload(aDownload.id);
// At this point, we know if we are an indeterminate download or not
dl.setAttribute("progressmode", aDownload.percentComplete == -1 ?
"undetermined" : "normal");
// As well as knowing the referrer
let referrer = aDownload.referrer;
if (referrer)
dl.setAttribute("referrer", referrer.spec);
break;
}
}
// autoRemoveAndClose could have already closed our window...
try {
let dl = getDownload(aDownload.id);
// We should eventually know the referrer at some point
let referrer = aDownload.referrer;
if (referrer)
dl.setAttribute("referrer", referrer.spec);
if (!dl)
dl = getDownload(aDownload.id);
// Update to the new state
dl.setAttribute("state", state);
@ -112,18 +123,15 @@ DownloadProgressListener.prototype = {
var download = getDownload(aDownload.id);
// Update this download's progressmeter
if (aDownload.percentComplete == -1) {
download.setAttribute("progressmode", "undetermined");
} else {
download.setAttribute("progressmode", "normal");
if (aDownload.percentComplete != -1) {
download.setAttribute("progress", aDownload.percentComplete);
}
// Dispatch ValueChange for a11y
var event = document.createEvent("Events");
event.initEvent("ValueChange", true, true);
document.getAnonymousElementByAttribute(download, "anonid", "progressmeter")
.dispatchEvent(event);
// Dispatch ValueChange for a11y
let event = document.createEvent("Events");
event.initEvent("ValueChange", true, true);
document.getAnonymousElementByAttribute(download, "anonid", "progressmeter")
.dispatchEvent(event);
}
// Update the progress so the status can be correctly updated
download.setAttribute("currBytes", aDownload.amountTransferred);

View File

@ -411,7 +411,7 @@ function onUpdateProgress()
var base = 0;
var dls = gDownloadManager.activeDownloads;
while (dls.hasMoreElements()) {
let dl = dls.getNext().QueryInterface(Ci.nsIDownload);
let dl = dls.getNext();
if (dl.percentComplete < 100 && dl.size > 0) {
mean += dl.amountTransferred;
base += dl.size;