Bug 1398630: Part 2 - Avoid unnecessary Map/Set lookups. r?zombie draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 10 Sep 2017 15:39:49 -0700
changeset 662072 71f5c6267f68a842c2fd3f3b98c307a4857b1770
parent 662071 8a7e15de7bf9a73f963ada5da88f26de44459c44
child 662073 14d4cf5bf4a360347a8ab542871157044cff40df
push id78941
push usermaglione.k@gmail.com
push dateSun, 10 Sep 2017 22:46:43 +0000
reviewerszombie
bugs1398630
milestone57.0a1
Bug 1398630: Part 2 - Avoid unnecessary Map/Set lookups. r?zombie We currently call has() every time we do a DefaultMap/DefaultWeakMap lookup, which unfortunately shows up a lot in profiles. We only actually need to check, though, if get() returns an undefined value. Similar things in other places, where we only need to do a has() call if another operation fails. MozReview-Commit-ID: 9qFWsb4vlZj
toolkit/components/extensions/ExtensionUtils.jsm
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -71,34 +71,38 @@ function instanceOf(value, type) {
  */
 class DefaultWeakMap extends WeakMap {
   constructor(defaultConstructor, init) {
     super(init);
     this.defaultConstructor = defaultConstructor;
   }
 
   get(key) {
-    if (!this.has(key)) {
-      this.set(key, this.defaultConstructor(key));
+    let value = super.get(key);
+    if (value === undefined && !this.has(key)) {
+      value = this.defaultConstructor(key);
+      this.set(key, value);
     }
-    return super.get(key);
+    return value;
   }
 }
 
 class DefaultMap extends Map {
   constructor(defaultConstructor, init) {
     super(init);
     this.defaultConstructor = defaultConstructor;
   }
 
   get(key) {
-    if (!this.has(key)) {
-      this.set(key, this.defaultConstructor(key));
+    let value = super.get(key);
+    if (value === undefined && !this.has(key)) {
+      value = this.defaultConstructor(key);
+      this.set(key, value);
     }
-    return super.get(key);
+    return value;
   }
 }
 
 const _winUtils = new DefaultWeakMap(win => {
   return win.QueryInterface(Ci.nsIInterfaceRequestor)
             .getInterface(Ci.nsIDOMWindowUtils);
 });
 const getWinUtils = win => _winUtils.get(win);
@@ -133,35 +137,36 @@ class EventEmitter {
    * dispatchers may need to block on.
    *
    * @param {string} event
    *       The name of the event to listen for.
    * @param {function(string, ...any)} listener
    *        The listener to call when events are emitted.
    */
   on(event, listener) {
-    if (!this[LISTENERS].has(event)) {
-      this[LISTENERS].set(event, new Set());
+    let listeners = this[LISTENERS].get(event);
+    if (!listeners) {
+      listeners = new Set();
+      this[LISTENERS].set(event, listeners);
     }
 
-    this[LISTENERS].get(event).add(listener);
+    listeners.add(listener);
   }
 
   /**
    * Removes the given function as a listener for the given event.
    *
    * @param {string} event
    *       The name of the event to stop listening for.
    * @param {function(string, ...any)} listener
    *        The listener function to remove.
    */
   off(event, listener) {
-    if (this[LISTENERS].has(event)) {
-      let set = this[LISTENERS].get(event);
-
+    let set = this.listeners.get(event);
+    if (set) {
       set.delete(listener);
       set.delete(this[ONCE_MAP].get(listener));
       if (!set.size) {
         this[LISTENERS].delete(event);
       }
     }
   }
 
@@ -250,17 +255,17 @@ class LimitedSet extends Set {
       // the entire loop even after we're done truncating.
       if (this.size > limit) {
         this.delete(item);
       }
     }
   }
 
   add(item) {
-    if (!this.has(item) && this.size >= this.limit + this.slop) {
+    if (this.size >= this.limit + this.slop && !this.has(item)) {
       this.truncate(this.limit - 1);
     }
     super.add(item);
   }
 }
 
 /**
  * Returns a Promise which resolves when the given document's DOM has