Bug 835596: Speed up getAppByLocalId from O(n) to O(1) r=ferjm
authorFabrice Desré <fabrice@mozilla.com>
Wed, 30 Jan 2013 22:50:28 -0800
changeset 120449 cadd2ad17dc01f2ccdd01b8d4067a12bc0bca042
parent 120448 09b4d9d69c42458bf682640841b9bb68b9e1bb35
child 120450 aea13422f0637ce870a8a0b996f0ad4ad142d22d
push id24251
push userryanvm@gmail.com
push dateThu, 31 Jan 2013 20:56:22 +0000
treeherdermozilla-central@683b08dc1afd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersferjm
bugs835596
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 835596: Speed up getAppByLocalId from O(n) to O(1) r=ferjm
dom/apps/src/AppsServiceChild.jsm
dom/apps/src/AppsUtils.jsm
--- a/dom/apps/src/AppsServiceChild.jsm
+++ b/dom/apps/src/AppsServiceChild.jsm
@@ -28,16 +28,24 @@ this.DOMApplicationRegistry = {
 
     ["Webapps:AddApp", "Webapps:RemoveApp"].forEach((function(aMsgName) {
       this.cpmm.addMessageListener(aMsgName, this);
     }).bind(this));
 
     // We need to prime the cache with the list of apps.
     // XXX shoud we do this async and block callers if it's not yet there?
     this.webapps = this.cpmm.sendSyncMessage("Webapps:GetList", { })[0];
+
+    // We need a fast mapping from localId -> app, so we add an index.
+    this.localIdIndex = { };
+    for (let id in this.webapps) {
+      let app = this.webapps[id];
+      this.localIdIndex[app.localId] = app;
+    }
+
     Services.obs.addObserver(this, "xpcom-shutdown", false);
   },
 
   observe: function(aSubject, aTopic, aData) {
     // cpmm.addMessageListener causes the DOMApplicationRegistry object to live
     // forever if we don't clean up properly.
     this.webapps = null;
     ["Webapps:AddApp", "Webapps:RemoveApp"].forEach((function(aMsgName) {
@@ -46,18 +54,20 @@ this.DOMApplicationRegistry = {
   },
 
   receiveMessage: function receiveMessage(aMessage) {
     debug("Received " + aMessage.name + " message.");
     let msg = aMessage.json;
     switch (aMessage.name) {
       case "Webapps:AddApp":
         this.webapps[msg.id] = msg.app;
+        this.localIdIndex[msg.app.localId] = msg.app;
         break;
       case "Webapps:RemoveApp":
+        delete this.localIdIndex[this.webapps[msg.id].localId];
         delete this.webapps[msg.id];
         break;
     }
   },
 
   getAppByManifestURL: function getAppByManifestURL(aManifestURL) {
     debug("getAppByManifestURL " + aManifestURL);
     return AppsUtils.getAppByManifestURL(this.webapps, aManifestURL);
@@ -70,17 +80,23 @@ this.DOMApplicationRegistry = {
 
   getCSPByLocalId: function(aLocalId) {
     debug("getCSPByLocalId:" + aLocalId);
     return AppsUtils.getCSPByLocalId(this.webapps, aLocalId);
   },
 
   getAppByLocalId: function getAppByLocalId(aLocalId) {
     debug("getAppByLocalId " + aLocalId);
-    return AppsUtils.getAppByLocalId(this.webapps, aLocalId);
+    let app = this.localIdIndex[aLocalId];
+    if (!app) {
+      debug("Ouch, No app!");
+      return null;
+    }
+
+    return AppsUtils.cloneAsMozIApplication(app);
   },
 
   getManifestURLByLocalId: function getManifestURLByLocalId(aLocalId) {
     debug("getManifestURLByLocalId " + aLocalId);
     return AppsUtils.getManifestURLByLocalId(this.webapps, aLocalId);
   },
 
   getAppFromObserverMessage: function getAppFromObserverMessage(aMessage) {
--- a/dom/apps/src/AppsUtils.jsm
+++ b/dom/apps/src/AppsUtils.jsm
@@ -76,18 +76,24 @@ this.AppsUtils = {
       // specific permission. It is not checking if browsers inside |aApp| have such
       // permission.
       let principal = secMan.getAppCodebasePrincipal(uri, aApp.localId,
                                                      /*mozbrowser*/false);
       let perm = Services.perms.testExactPermissionFromPrincipal(principal,
                                                                  aPermission);
       return (perm === Ci.nsIPermissionManager.ALLOW_ACTION);
     };
-    res.QueryInterface = XPCOMUtils.generateQI([Ci.mozIDOMApplication,
-                                                Ci.mozIApplication]);
+    res.QueryInterface = function(aIID) {
+      if (aIID.equals(Ci.mozIDOMApplication) ||
+          aIID.equals(Ci.mozIApplication) ||
+          aIID.equals(Ci.nsISupports))
+        return this;
+      throw Cr.NS_ERROR_NO_INTERFACE;
+    }
+
     return res;
   },
 
   getAppByManifestURL: function getAppByManifestURL(aApps, aManifestURL) {
     debug("getAppByManifestURL " + aManifestURL);
     // This could be O(1) if |webapps| was a dictionary indexed on manifestURL
     // which should be the unique app identifier.
     // It's currently O(n).