Bug 1389381: Part 3 - Loop over the entire set iterator when truncating a LimitedSet. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 11 Aug 2017 14:46:44 -0700
changeset 645324 11eb975e303a8abbcb3b5689407693fdf7cbd77e
parent 645323 767c494324db1160a24a76ae82947ed5ff1fd9dd
child 725897 8feed21f8440c8bf1142e0cd041f4341203bd679
push id73747
push usermaglione.k@gmail.com
push dateSat, 12 Aug 2017 18:12:29 +0000
reviewersaswan
bugs1389381
milestone57.0a1
Bug 1389381: Part 3 - Loop over the entire set iterator when truncating a LimitedSet. r?aswan MozReview-Commit-ID: 3imHF9IRI2N
toolkit/components/extensions/ExtensionUtils.jsm
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -274,20 +274,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);