Bug 1389381 - Part 3: Loop over the entire set iterator when truncating a LimitedSet. r=aswan, a=sledru
authorKris Maglione <maglione.k@gmail.com>
Fri, 11 Aug 2017 14:46:44 -0700
changeset 421169 b301a9babee6ac845939eb93352988b321b09efd
parent 421168 ffabdaaebceac4ac77c595703494b6f919d998e8
child 421170 5f81b09432e0f866815ef111c2bc761128adcd53
push id7612
push userryanvm@gmail.com
push dateMon, 14 Aug 2017 14:21:34 +0000
treeherdermozilla-beta@b301a9babee6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, sledru
bugs1389381
milestone56.0
Bug 1389381 - Part 3: Loop over the entire set iterator when truncating a LimitedSet. r=aswan, a=sledru MozReview-Commit-ID: 3imHF9IRI2N
toolkit/components/extensions/ExtensionUtils.jsm
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -276,20 +276,24 @@ class LimitedSet extends Set {
   constructor(limit, slop = Math.round(limit * .25), iterable = undefined) {
     super(iterable);
     this.limit = limit;
     this.slop = slop;
   }
 
   truncate(limit) {
     for (let item of this) {
-      if (this.size <= limit) {
-        break;
+      // Live set iterators can ge relatively expensive, since they need
+      // to be updated after every modification to the set. Since
+      // breaking out of the loop early will keep the iterator alive
+      // until the next full GC, we're currently better off finishing
+      // the entire loop even after we're done truncating.
+      if (this.size > limit) {
+        this.delete(item);
       }
-      this.delete(item);
     }
   }
 
   add(item) {
     if (!this.has(item) && this.size >= this.limit + this.slop) {
       this.truncate(this.limit - 1);
     }
     super.add(item);