Make sure that NormalizeElements and ShouldConvertToFastElements are based on the same values

verwaest · Commit bot · commit 066747ea0530 · 2015-11-25T11:46:50.000Z
BUG=v8:4518 LOG=n Review URL: https://codereview.chromium.org/1472293002 Cr-Commit-Position: refs/heads/master@{#32265}
diff --git a/src/elements.cc b/src/elements.cc
@@ -1227,13 +1227,18 @@ class FastElementsAccessor
       }
       int num_used = 0;
       for (int i = 0; i < backing_store->length(); ++i) {
-        if (!backing_store->is_the_hole(i)) ++num_used;
-        // Bail out early if more than 1/4 is used.
-        if (4 * num_used > backing_store->length()) break;
-      }
-      if (4 * num_used <= backing_store->length()) {
-        JSObject::NormalizeElements(obj);
+        if (!backing_store->is_the_hole(i)) {
+          ++num_used;
+          // Bail out if a number dictionary wouldn't be able to save at least
+          // 75% space.
+          if (4 * SeededNumberDictionary::ComputeCapacity(num_used) *
+                  SeededNumberDictionary::kEntrySize >
+              backing_store->length()) {
+            return;
+          }
+        }
       }
+      JSObject::NormalizeElements(obj);
     }
   }
 
diff --git a/src/objects.cc b/src/objects.cc
@@ -14646,6 +14646,8 @@ static bool ShouldConvertToFastElements(JSObject* object,
 
   uint32_t dictionary_size = static_cast<uint32_t>(dictionary->Capacity()) *
                              SeededNumberDictionary::kEntrySize;
+
+  // Turn fast if the dictionary only saves 50% space.
   return 2 * dictionary_size >= *new_capacity;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -1227,13 +1227,18 @@ class FastElementsAccessor`
`1227`	`1227`	`}`
`1228`	`1228`	`int num_used = 0;`
`1229`	`1229`	`for (int i = 0; i < backing_store->length(); ++i) {`
`1230`		`- if (!backing_store->is_the_hole(i)) ++num_used;`
`1231`		`- // Bail out early if more than 1/4 is used.`
`1232`		`- if (4 * num_used > backing_store->length()) break;`
`1233`		`- }`
`1234`		`- if (4 * num_used <= backing_store->length()) {`
`1235`		`- JSObject::NormalizeElements(obj);`
	`1230`	`+ if (!backing_store->is_the_hole(i)) {`
	`1231`	`+ ++num_used;`
	`1232`	`+ // Bail out if a number dictionary wouldn't be able to save at least`
	`1233`	`+ // 75% space.`
	`1234`	`+ if (4 * SeededNumberDictionary::ComputeCapacity(num_used) *`
	`1235`	`+ SeededNumberDictionary::kEntrySize >`
	`1236`	`+ backing_store->length()) {`
	`1237`	`+ return;`
	`1238`	`+ }`
	`1239`	`+ }`
`1236`	`1240`	`}`
	`1241`	`+ JSObject::NormalizeElements(obj);`
`1237`	`1242`	`}`
`1238`	`1243`	`}`
`1239`	`1244`
Original file line number	Diff line number	Diff line change
`@@ -14646,6 +14646,8 @@ static bool ShouldConvertToFastElements(JSObject* object,`
`14646`	`14646`
`14647`	`14647`	`uint32_t dictionary_size = static_cast<uint32_t>(dictionary->Capacity()) *`
`14648`	`14648`	`SeededNumberDictionary::kEntrySize;`
	`14649`	`+`
	`14650`	`+ // Turn fast if the dictionary only saves 50% space.`
`14649`	`14651`	`return 2 * dictionary_size >= *new_capacity;`
`14650`	`14652`	`}`
`14651`	`14653`