[runtime] Support Symbols in KeyAccumulator

BUG=

Review URL: https://codereview.chromium.org/1425403002

Cr-Commit-Position: refs/heads/master@{#31788}

BUILD.gn

@@ -1081,6 +1081,8 @@ source_set("v8_base") {
     "src/isolate.h",
     "src/json-parser.h",
     "src/json-stringifier.h",
+    "src/key-accumulator.h",
+    "src/key-accumulator.cc",
     "src/layout-descriptor-inl.h",
     "src/layout-descriptor.cc",
     "src/layout-descriptor.h",

src/elements.h

@@ -8,6 +8,7 @@
 #include "src/elements-kind.h"
 #include "src/heap/heap.h"
 #include "src/isolate.h"
+#include "src/key-accumulator.h"
 #include "src/objects.h"
 
 namespace v8 {

src/key-accumulator.cc

+// Copyright 2013 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/key-accumulator.h"
+
+#include "src/elements.h"
+#include "src/factory.h"
+#include "src/objects-inl.h"
+
+
+namespace v8 {
+namespace internal {
+
+
+KeyAccumulator::~KeyAccumulator() {
+  for (size_t i = 0; i < elements_.size(); i++) {
+    delete elements_[i];
+  }
+}
+
+
+Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
+  if (length_ == 0) {
+    return isolate_->factory()->empty_fixed_array();
+  }
+  // Make sure we have all the lengths collected.
+  NextPrototype();
+
+  // Assemble the result array by first adding the element keys and then the
+  // property keys. We use the total number of String + Symbol keys per level in
+  // |level_lengths_| and the available element keys in the corresponding bucket
+  // in |elements_| to deduce the number of keys to take from the
+  // |string_properties_| and |symbol_properties_| set.
+  Handle<FixedArray> result = isolate_->factory()->NewFixedArray(length_);
+  int insertion_index = 0;
+  int string_properties_index = 0;
+  int symbol_properties_index = 0;
+  // String and Symbol lengths always come in pairs:
+  size_t max_level = level_lengths_.size() / 2;
+  for (size_t level = 0; level < max_level; level++) {
+    int num_string_properties = level_lengths_[level * 2];
+    int num_symbol_properties = level_lengths_[level * 2 + 1];
+    if (num_string_properties < 0) {
+      // If the |num_string_properties| is negative, the current level contains
+      // properties from a proxy, hence we skip the integer keys in |elements_|
+      // since proxies define the complete ordering.
+      num_string_properties = -num_string_properties;
+    } else if (level < elements_.size()) {
+      // Add the element indices for this prototype level.
+      std::vector<uint32_t>* elements = elements_[level];
+      int num_elements = static_cast<int>(elements->size());
+      for (int i = 0; i < num_elements; i++) {
+        Handle<Object> key;
+        if (convert == KEEP_NUMBERS) {
+          key = isolate_->factory()->NewNumberFromUint(elements->at(i));
+        } else {
+          key = isolate_->factory()->Uint32ToString(elements->at(i));
+        }
+        result->set(insertion_index, *key);
+        insertion_index++;
+      }
+    }
+    // Add the string property keys for this prototype level.
+    for (int i = 0; i < num_string_properties; i++) {
+      Object* key = string_properties_->KeyAt(string_properties_index);
+      result->set(insertion_index, key);
+      insertion_index++;
+      string_properties_index++;
+    }
+    // Add the symbol property keys for this prototype level.
+    for (int i = 0; i < num_symbol_properties; i++) {
+      Object* key = symbol_properties_->KeyAt(symbol_properties_index);
+      result->set(insertion_index, key);
+      insertion_index++;
+      symbol_properties_index++;
+    }
+  }
+
+  DCHECK_EQ(insertion_index, length_);
+  return result;
+}
+
+
+namespace {
+
+bool AccumulatorHasKey(std::vector<uint32_t>* sub_elements, uint32_t key) {
+  return std::binary_search(sub_elements->begin(), sub_elements->end(), key);
+}
+
+}  // namespace
+
+bool KeyAccumulator::AddKey(Object* key, AddKeyConversion convert) {
+  return AddKey(handle(key, isolate_), convert);
+}
+
+
+bool KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
+  if (key->IsSymbol()) {
+    if (filter_ == SKIP_SYMBOLS) return false;
+    return AddSymbolKey(key);
+  }
+  // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy).
+  DCHECK_LE(0, level_string_length_);
+  // In some cases (e.g. proxies) we might get in String-converted ints which
+  // should be added to the elements list instead of the properties. For
+  // proxies we have to convert as well but also respect the original order.
+  // Therefore we add a converted key to both sides
+  if (convert == CONVERT_TO_ARRAY_INDEX || convert == PROXY_MAGIC) {
+    uint32_t index = 0;
+    int prev_length = length_;
+    int prev_proto = level_string_length_;
+    if ((key->IsString() && Handle<String>::cast(key)->AsArrayIndex(&index)) ||
+        key->ToArrayIndex(&index)) {
+      bool key_was_added = AddIntegerKey(index);
+      if (convert == CONVERT_TO_ARRAY_INDEX) return key_was_added;
+      if (convert == PROXY_MAGIC) {
+        // If we had an array index (number) and it wasn't added, the key
+        // already existed before, hence we cannot add it to the properties
+        // keys as it would lead to duplicate entries.
+        if (!key_was_added) {
+          return false;
+        }
+        length_ = prev_length;
+        level_string_length_ = prev_proto;
+      }
+    }
+  }
+  return AddStringKey(key, convert);
+}
+
+
+bool KeyAccumulator::AddKey(uint32_t key) { return AddIntegerKey(key); }
+
+
+bool KeyAccumulator::AddIntegerKey(uint32_t key) {
+  // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy).
+  // We mark proxy-levels with a negative length
+  DCHECK_LE(0, level_string_length_);
+  // Binary search over all but the last level. The last one might not be
+  // sorted yet.
+  for (size_t i = 1; i < elements_.size(); i++) {
+    if (AccumulatorHasKey(elements_[i - 1], key)) return false;
+  }
+  elements_.back()->push_back(key);
+  length_++;
+  return true;
+}
+
+
+bool KeyAccumulator::AddStringKey(Handle<Object> key,
+                                  AddKeyConversion convert) {
+  if (string_properties_.is_null()) {
+    string_properties_ = OrderedHashSet::Allocate(isolate_, 16);
+  }
+  // TODO(cbruni): remove this conversion once we throw the correct TypeError
+  // for non-string/symbol elements returned by proxies
+  if (convert == PROXY_MAGIC && key->IsNumber()) {
+    key = isolate_->factory()->NumberToString(key);
+  }
+  int prev_size = string_properties_->NumberOfElements();
+  string_properties_ = OrderedHashSet::Add(string_properties_, key);
+  if (prev_size < string_properties_->NumberOfElements()) {
+    length_++;
+    level_string_length_++;
+    return true;
+  } else {
+    return false;
+  }
+}
+
+
+bool KeyAccumulator::AddSymbolKey(Handle<Object> key) {
+  if (symbol_properties_.is_null()) {
+    symbol_properties_ = OrderedHashSet::Allocate(isolate_, 16);
+  }
+  int prev_size = symbol_properties_->NumberOfElements();
+  symbol_properties_ = OrderedHashSet::Add(symbol_properties_, key);
+  if (prev_size < symbol_properties_->NumberOfElements()) {
+    length_++;
+    level_symbol_length_++;
+    return true;
+  } else {
+    return false;
+  }
+}
+
+
+void KeyAccumulator::AddKeys(Handle<FixedArray> array,
+                             AddKeyConversion convert) {
+  int add_length = array->length();
+  if (add_length == 0) return;
+  for (int i = 0; i < add_length; i++) {
+    Handle<Object> current(array->get(i), isolate_);
+    AddKey(current, convert);
+  }
+}
+
+
+void KeyAccumulator::AddKeys(Handle<JSObject> array_like,
+                             AddKeyConversion convert) {
+  DCHECK(array_like->IsJSArray() || array_like->HasSloppyArgumentsElements());
+  ElementsAccessor* accessor = array_like->GetElementsAccessor();
+  accessor->AddElementsToKeyAccumulator(array_like, this, convert);
+}
+
+
+void KeyAccumulator::AddKeysFromProxy(Handle<JSObject> array_like) {
+  // Proxies define a complete list of keys with no distinction of
+  // elements and properties, which breaks the normal assumption for the
+  // KeyAccumulator.
+  AddKeys(array_like, PROXY_MAGIC);
+  // Invert the current length to indicate a present proxy, so we can ignore
+  // element keys for this level. Otherwise we would not fully respect the order
+  // given by the proxy.
+  level_string_length_ = -level_string_length_;
+}
+
+
+void KeyAccumulator::AddElementKeysFromInterceptor(
+    Handle<JSObject> array_like) {
+  AddKeys(array_like, CONVERT_TO_ARRAY_INDEX);
+  // The interceptor might introduce duplicates for the current level, since
+  // these keys get added after the objects's normal element keys.
+  SortCurrentElementsListRemoveDuplicates();
+}
+
+
+void KeyAccumulator::SortCurrentElementsListRemoveDuplicates() {
+  // Sort and remove duplicates from the current elements level and adjust.
+  // the lengths accordingly.
+  auto last_level = elements_.back();
+  size_t nof_removed_keys = last_level->size();
+  std::sort(last_level->begin(), last_level->end());
+  last_level->erase(std::unique(last_level->begin(), last_level->end()),
+                    last_level->end());
+  // Adjust total length by the number of removed duplicates.
+  nof_removed_keys -= last_level->size();
+  length_ -= static_cast<int>(nof_removed_keys);
+}
+
+
+void KeyAccumulator::SortCurrentElementsList() {
+  if (elements_.empty()) return;
+  auto element_keys = elements_.back();
+  std::sort(element_keys->begin(), element_keys->end());
+}
+
+
+void KeyAccumulator::NextPrototype() {
+  // Store the protoLength on the first call of this method.
+  if (!elements_.empty()) {
+    level_lengths_.push_back(level_string_length_);
+    level_lengths_.push_back(level_symbol_length_);
+  }
+  elements_.push_back(new std::vector<uint32_t>());
+  level_string_length_ = 0;
+  level_symbol_length_ = 0;
+}
+
+
+}  // namespace internal
+}  // namespace v8

src/key-accumulator.h

+// Copyright 2012 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_KEY_ACCUMULATOR_H_
+#define V8_KEY_ACCUMULATOR_H_
+
+#include "src/isolate.h"
+#include "src/objects.h"
+
+namespace v8 {
+namespace internal {
+
+enum AddKeyConversion { DO_NOT_CONVERT, CONVERT_TO_ARRAY_INDEX, PROXY_MAGIC };
+
+// This is a helper class for JSReceiver::GetKeys which collects and sorts keys.
+// GetKeys needs to sort keys per prototype level, first showing the integer
+// indices from elements then the strings from the properties. However, this
+// does not apply to proxies which are in full control of how the keys are
+// sorted.
+//
+// For performance reasons the KeyAccumulator internally separates integer keys
+// in |elements_| into sorted lists per prototype level. String keys are
+// collected in |string_properties_|, a single OrderedHashSet (similar for
+// Symbols in |symbol_properties_|. To separate the keys per level later when
+// assembling the final list, |levelLengths_| keeps track of the number of
+// String and Symbol keys per level.
+//
+// Only unique keys are kept by the KeyAccumulator, strings are stored in a
+// HashSet for inexpensive lookups. Integer keys are kept in sorted lists which
+// are more compact and allow for reasonably fast includes check.
+class KeyAccumulator final BASE_EMBEDDED {
+ public:
+  explicit KeyAccumulator(Isolate* isolate,
+                          KeyFilter filter = KeyFilter::SKIP_SYMBOLS)
+      : isolate_(isolate), filter_(filter) {}
+  ~KeyAccumulator();
+
+  bool AddKey(uint32_t key);
+  bool AddKey(Object* key, AddKeyConversion convert = DO_NOT_CONVERT);
+  bool AddKey(Handle<Object> key, AddKeyConversion convert = DO_NOT_CONVERT);
+  void AddKeys(Handle<FixedArray> array,
+               AddKeyConversion convert = DO_NOT_CONVERT);
+  void AddKeys(Handle<JSObject> array,
+               AddKeyConversion convert = DO_NOT_CONVERT);
+  void AddKeysFromProxy(Handle<JSObject> array);
+  void AddElementKeysFromInterceptor(Handle<JSObject> array);
+  // Jump to the next level, pushing the current |levelLength_| to
+  // |levelLengths_| and adding a new list to |elements_|.
+  void NextPrototype();
+  // Sort the integer indices in the last list in |elements_|
+  void SortCurrentElementsList();
+  Handle<FixedArray> GetKeys(GetKeysConversion convert = KEEP_NUMBERS);
+  int length() { return length_; }
+
+ private:
+  bool AddIntegerKey(uint32_t key);
+  bool AddStringKey(Handle<Object> key, AddKeyConversion convert);
+  bool AddSymbolKey(Handle<Object> array);
+  void SortCurrentElementsListRemoveDuplicates();
+
+  Isolate* isolate_;
+  KeyFilter filter_;
+  // |elements_| contains the sorted element keys (indices) per level.
+  std::vector<std::vector<uint32_t>*> elements_;
+  // |protoLengths_| contains the total number of keys (elements + properties)
+  // per level. Negative values mark counts for a level with keys from a proxy.
+  std::vector<int> level_lengths_;
+  // |string_properties_| contains the unique String property keys for all
+  // levels in insertion order per level.
+  Handle<OrderedHashSet> string_properties_;
+  // |symbol_properties_| contains the unique Symbol property keys for all
+  // levels in insertion order per level.
+  Handle<OrderedHashSet> symbol_properties_;
+  // |length_| keeps track of the total number of all element and property keys.
+  int length_ = 0;
+  // |levelLength_| keeps track of the number of String keys in the current
+  // level.
+  int level_string_length_ = 0;
+  // |levelSymbolLength_| keeps track of the number of Symbol keys in the
+  // current level.
+  int level_symbol_length_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(KeyAccumulator);
+};
+
+
+}  // namespace internal
+}  // namespace v8
+
+
+#endif  // V8_KEY_ACCUMULATOR_H_

src/objects.cc

@@ -30,6 +30,7 @@
 #include "src/ic/ic.h"
 #include "src/interpreter/bytecodes.h"
 #include "src/isolate-inl.h"
+#include "src/key-accumulator.h"
 #include "src/list.h"
 #include "src/log.h"
 #include "src/lookup.h"
@@ -7579,218 +7580,6 @@ Handle<FixedArray> JSObject::GetEnumPropertyKeys(Handle<JSObject> object,
 }
 
 
-KeyAccumulator::~KeyAccumulator() {
-  for (size_t i = 0; i < elements_.size(); i++) {
-    delete elements_[i];
-  }
-}
-
-
-Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
-  if (length_ == 0) {
-    return isolate_->factory()->empty_fixed_array();
-  }
-  // Make sure we have all the lengths collected.
-  NextPrototype();
-
-  // Assemble the result array by first adding the element keys and then
-  // the property keys. We use the total number of keys per level in
-  // |protoLengths_| and the available element keys in the corresponding bucket
-  // in |elements_| to deduce the number of keys to take from the |properties_|
-  // set.
-  Handle<FixedArray> result = isolate_->factory()->NewFixedArray(length_);
-  int index = 0;
-  int properties_index = 0;
-  for (size_t level = 0; level < levelLengths_.size(); level++) {
-    int num_total = levelLengths_[level];
-    int num_elements = 0;
-    if (num_total < 0) {
-      // If the total is negative, the current level contains properties from a
-      // proxy, hence we skip the integer keys in |elements_| since proxies
-      // define the complete ordering.
-      num_total = -num_total;
-    } else if (level < elements_.size()) {
-      std::vector<uint32_t>* elements = elements_[level];
-      num_elements = static_cast<int>(elements->size());
-      for (int i = 0; i < num_elements; i++) {
-        Handle<Object> key;
-        if (convert == KEEP_NUMBERS) {
-          key = isolate_->factory()->NewNumberFromUint(elements->at(i));
-        } else {
-          key = isolate_->factory()->Uint32ToString(elements->at(i));
-        }
-        result->set(index, *key);
-        index++;
-      }
-    }
-    // Add the property keys for this prototype level.
-    int num_properties = num_total - num_elements;
-    for (int i = 0; i < num_properties; i++) {
-      Object* key = properties_->KeyAt(properties_index);
-      result->set(index, key);
-      index++;
-      properties_index++;
-    }
-  }
-
-  DCHECK_EQ(index, length_);
-  return result;
-}
-
-
-namespace {
-
-bool AccumulatorHasKey(std::vector<uint32_t>* sub_elements, uint32_t key) {
-  return std::binary_search(sub_elements->begin(), sub_elements->end(), key);
-}
-
-}  // namespace
-
-
-bool KeyAccumulator::AddKey(uint32_t key) {
-  // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy).
-  // We mark proxy-levels with a negative length
-  DCHECK_LE(0, levelLength_);
-  // Binary search over all but the last level. The last one might not be
-  // sorted yet.
-  for (size_t i = 1; i < elements_.size(); i++) {
-    if (AccumulatorHasKey(elements_[i - 1], key)) return false;
-  }
-  elements_.back()->push_back(key);
-  length_++;
-  levelLength_++;
-  return true;
-}
-
-
-bool KeyAccumulator::AddKey(Object* key, AddKeyConversion convert) {
-  return AddKey(handle(key, isolate_), convert);
-}
-
-
-bool KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
-  if (filter_ == SKIP_SYMBOLS && key->IsSymbol()) {
-    return false;
-  }
-  // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy).
-  DCHECK_LE(0, levelLength_);
-  // In some cases (e.g. proxies) we might get in String-converted ints which
-  // should be added to the elements list instead of the properties. For
-  // proxies we have to convert as well but also respect the original order.
-  // Therefore we add a converted key to both sides
-  if (convert == CONVERT_TO_ARRAY_INDEX || convert == PROXY_MAGIC) {
-    uint32_t index = 0;
-    int prev_length = length_;
-    int prev_proto = levelLength_;
-    bool was_array_index = false;
-    bool key_was_added = false;
-    if ((key->IsString() && Handle<String>::cast(key)->AsArrayIndex(&index)) ||
-        key->ToArrayIndex(&index)) {
-      key_was_added = AddKey(index);
-      was_array_index = true;
-      if (convert == CONVERT_TO_ARRAY_INDEX) return key_was_added;
-    }
-    if (was_array_index && convert == PROXY_MAGIC) {
-      // If we had an array index (number) and it wasn't added, the key
-      // already existed before, hence we cannot add it to the properties
-      // keys as it would lead to duplicate entries.
-      if (!key_was_added) {
-        return false;
-      }
-      length_ = prev_length;
-      levelLength_ = prev_proto;
-    }
-  }
-  if (properties_.is_null()) {
-    properties_ = OrderedHashSet::Allocate(isolate_, 16);
-  }
-  // TODO(cbruni): remove this conversion once we throw the correct TypeError
-  // for non-string/symbol elements returned by proxies
-  if (convert == PROXY_MAGIC && key->IsNumber()) {
-    key = isolate_->factory()->NumberToString(key);
-  }
-  int prev_size = properties_->NumberOfElements();
-  properties_ = OrderedHashSet::Add(properties_, key);
-  if (prev_size < properties_->NumberOfElements()) {
-    length_++;
-    levelLength_++;
-  }
-  return true;
-}
-
-
-void KeyAccumulator::AddKeys(Handle<FixedArray> array,
-                             AddKeyConversion convert) {
-  int add_length = array->length();
-  if (add_length == 0) return;
-  for (int i = 0; i < add_length; i++) {
-    Handle<Object> current(array->get(i), isolate_);
-    AddKey(current);
-  }
-}
-
-
-void KeyAccumulator::AddKeys(Handle<JSObject> array_like,
-                             AddKeyConversion convert) {
-  DCHECK(array_like->IsJSArray() || array_like->HasSloppyArgumentsElements());
-  ElementsAccessor* accessor = array_like->GetElementsAccessor();
-  accessor->AddElementsToKeyAccumulator(array_like, this, convert);
-}
-
-
-void KeyAccumulator::AddKeysFromProxy(Handle<JSObject> array_like) {
-  // Proxies define a complete list of keys with no distinction of
-  // elements and properties, which breaks the normal assumption for the
-  // KeyAccumulator.
-  AddKeys(array_like, PROXY_MAGIC);
-  // Invert the current length to indicate a present proxy, so we can ignore
-  // element keys for this level. Otherwise we would not fully respect the order
-  // given by the proxy.
-  levelLength_ = -levelLength_;
-}
-
-
-void KeyAccumulator::AddElementKeysFromInterceptor(
-    Handle<JSObject> array_like) {
-  AddKeys(array_like, CONVERT_TO_ARRAY_INDEX);
-  // The interceptor might introduce duplicates for the current level, since
-  // these keys get added after the objects's normal element keys.
-  SortCurrentElementsListRemoveDuplicates();
-}
-
-
-void KeyAccumulator::SortCurrentElementsListRemoveDuplicates() {
-  // Sort and remove duplciated from the current elements level and adjust
-  // the lengths accordingly.
-  auto last_level = elements_.back();
-  size_t nof_removed_keys = last_level->size();
-  std::sort(last_level->begin(), last_level->end());
-  last_level->erase(std::unique(last_level->begin(), last_level->end()),
-                    last_level->end());
-  // Adjust total length / level length by the number of removed duplicates
-  nof_removed_keys -= last_level->size();
-  levelLength_ -= static_cast<int>(nof_removed_keys);
-  length_ -= static_cast<int>(nof_removed_keys);
-}
-
-
-void KeyAccumulator::SortCurrentElementsList() {
-  if (elements_.empty()) return;
-  auto element_keys = elements_.back();
-  std::sort(element_keys->begin(), element_keys->end());
-}
-
-
-void KeyAccumulator::NextPrototype() {
-  // Store the protoLength on the first call of this method.
-  if (!elements_.empty()) {
-    levelLengths_.push_back(levelLength_);
-  }
-  elements_.push_back(new std::vector<uint32_t>());
-  levelLength_ = 0;
-}
-
-
 MaybeHandle<FixedArray> JSReceiver::GetKeys(Handle<JSReceiver> object,
                                             KeyCollectionType type,
                                             KeyFilter filter,
@@ -7873,10 +7662,7 @@ MaybeHandle<FixedArray> JSReceiver::GetKeys(Handle<JSReceiver> object,
       DCHECK(filter == INCLUDE_SYMBOLS);
       PropertyAttributes attr_filter =
           static_cast<PropertyAttributes>(DONT_ENUM | PRIVATE_SYMBOL);
-      Handle<FixedArray> property_keys = isolate->factory()->NewFixedArray(
-          current->NumberOfOwnProperties(attr_filter));
-      current->GetOwnPropertyNames(*property_keys, 0, attr_filter);
-      accumulator.AddKeys(property_keys);
+      current->CollectOwnPropertyNames(&accumulator, attr_filter);
     }
 
     // Add the property keys from the interceptor.
@@ -15072,6 +14858,27 @@ int JSObject::GetOwnPropertyNames(FixedArray* storage, int index,
 }
 
 
+int JSObject::CollectOwnPropertyNames(KeyAccumulator* keys,
+                                      PropertyAttributes filter) {
+  if (HasFastProperties()) {
+    int nof_keys = keys->length();
+    int real_size = map()->NumberOfOwnDescriptors();
+    Handle<DescriptorArray> descs(map()->instance_descriptors());
+    for (int i = 0; i < real_size; i++) {
+      if ((descs->GetDetails(i).attributes() & filter) != 0) continue;
+      Name* key = descs->GetKey(i);
+      if (key->FilterKey(filter)) continue;
+      keys->AddKey(key);
+    }
+    return nof_keys - keys->length();
+  } else if (IsJSGlobalObject()) {
+    return global_dictionary()->CollectKeysTo(keys, filter);
+  } else {
+    return property_dictionary()->CollectKeysTo(keys, filter);
+  }
+}
+
+
 int JSObject::NumberOfOwnElements(PropertyAttributes filter) {
   // Fast case for objects with no elements.
   if (!IsJSValue() && HasFastElements()) {
@@ -16843,12 +16650,12 @@ int Dictionary<Derived, Shape, Key>::CopyKeysTo(
   int capacity = this->Capacity();
   for (int i = 0; i < capacity; i++) {
     Object* k = this->KeyAt(i);
-    if (this->IsKey(k) && !k->FilterKey(filter)) {
-      if (this->IsDeleted(i)) continue;
-      PropertyDetails details = this->DetailsAt(i);
-      PropertyAttributes attr = details.attributes();
-      if ((attr & filter) == 0) storage->set(index++, k);
-    }
+    if (!this->IsKey(k) || k->FilterKey(filter)) continue;
+    if (this->IsDeleted(i)) continue;
+    PropertyDetails details = this->DetailsAt(i);
+    PropertyAttributes attr = details.attributes();
+    if ((attr & filter) != 0) continue;
+    storage->set(index++, k);
   }
   if (sort_mode == Dictionary::SORTED) {
     storage->SortPairs(storage, index);
@@ -16858,6 +16665,24 @@ int Dictionary<Derived, Shape, Key>::CopyKeysTo(
 }
 
 
+template <typename Derived, typename Shape, typename Key>
+int Dictionary<Derived, Shape, Key>::CollectKeysTo(KeyAccumulator* keys,
+                                                   PropertyAttributes filter) {
+  int capacity = this->Capacity();
+  int keyLength = keys->length();
+  for (int i = 0; i < capacity; i++) {
+    Object* k = this->KeyAt(i);
+    if (!this->IsKey(k) || k->FilterKey(filter)) continue;
+    if (this->IsDeleted(i)) continue;
+    PropertyDetails details = this->DetailsAt(i);
+    PropertyAttributes attr = details.attributes();
+    if ((attr & filter) != 0) continue;
+    keys->AddKey(k);
+  }
+  return keyLength - keys->length();
+}
+
+
 // Backwards lookup (slow).
 template<typename Derived, typename Shape, typename Key>
 Object* Dictionary<Derived, Shape, Key>::SlowReverseLookup(Object* value) {

src/objects.h

@@ -2267,6 +2267,8 @@ class JSObject: public JSReceiver {
   // index. Returns the number of properties added.
   int GetOwnPropertyNames(FixedArray* storage, int index,
                           PropertyAttributes filter = NONE);
+  int CollectOwnPropertyNames(KeyAccumulator* keys,
+                              PropertyAttributes filter = NONE);
 
   // Returns the number of properties on this object filtering out properties
   // with the specified attributes (ignoring interceptors).
@@ -3416,6 +3418,8 @@ class Dictionary: public HashTable<Derived, Shape, Key> {
   // Returns the number of properties added.
   int CopyKeysTo(FixedArray* storage, int index, PropertyAttributes filter,
                  SortMode sort_mode);
+  // Collect the unsorted keys into the given KeyAccumulator.
+  int CollectKeysTo(KeyAccumulator* keys, PropertyAttributes filter);
 
   // Copies enumerable keys to preallocated fixed array.
   void CopyEnumKeysTo(FixedArray* storage);
@@ -10743,67 +10747,6 @@ class BooleanBit : public AllStatic {
 };
 
 
-enum AddKeyConversion { DO_NOT_CONVERT, CONVERT_TO_ARRAY_INDEX, PROXY_MAGIC };
-
-// This is a helper class for JSReceiver::GetKeys which collects and sorts keys.
-// GetKeys needs to sort keys per prototype level, first showing the integer
-// indices from elements then the strings from the properties. However, this
-// does not apply to proxies which are in full control of how the keys are
-// sorted.
-//
-// For performance reasons the KeyAccumulator internally separates integer
-// keys in |elements_| into sorted lists per prototype level. String keys are
-// collected in |properties_|, a single OrderedHashSet. To separate the keys per
-// level later when assembling the final list, |levelLengths_| keeps track of
-// the total number of keys (integers + strings) per level.
-//
-// Only unique keys are kept by the KeyAccumulator, strings are stored in a
-// HashSet for inexpensive lookups. Integer keys are kept in sorted lists which
-// are more compact and allow for reasonably fast includes check.
-class KeyAccumulator final BASE_EMBEDDED {
- public:
-  explicit KeyAccumulator(Isolate* isolate,
-                          KeyFilter filter = KeyFilter::SKIP_SYMBOLS)
-      : isolate_(isolate), filter_(filter) {}
-  ~KeyAccumulator();
-
-  bool AddKey(uint32_t key);
-  bool AddKey(Object* key, AddKeyConversion convert = DO_NOT_CONVERT);
-  bool AddKey(Handle<Object> key, AddKeyConversion convert = DO_NOT_CONVERT);
-  void AddKeys(Handle<FixedArray> array,
-               AddKeyConversion convert = DO_NOT_CONVERT);
-  void AddKeys(Handle<JSObject> array,
-               AddKeyConversion convert = DO_NOT_CONVERT);
-  void AddKeysFromProxy(Handle<JSObject> array);
-  void AddElementKeysFromInterceptor(Handle<JSObject> array);
-  // Jump to the next level, pushing the current |levelLength_| to
-  // |levelLengths_| and adding a new list to |elements_|.
-  void NextPrototype();
-  // Sort the integer indices in the last list in |elements_|
-  void SortCurrentElementsList();
-  void SortCurrentElementsListRemoveDuplicates();
-  Handle<FixedArray> GetKeys(GetKeysConversion convert = KEEP_NUMBERS);
-
-
- private:
-  Isolate* isolate_;
-  KeyFilter filter_;
-  // |elements_| contains the sorted element keys (indices) per level.
-  std::vector<std::vector<uint32_t>*> elements_;
-  // |protoLengths_| contains the total number of keys (elements + properties)
-  // per level. Negative values mark counts for a level with keys from a proxy.
-  std::vector<int> levelLengths_;
-  // |properties_| contains the property keys per level in insertion order.
-  Handle<OrderedHashSet> properties_;
-  // |length_| keeps track of the total number of all element and property keys.
-  int length_ = 0;
-  // |levelLength_| keeps track of the total number of keys
-  // (elements + properties) in the current level.
-  int levelLength_ = 0;
-
-  DISALLOW_COPY_AND_ASSIGN(KeyAccumulator);
-};
-
 }  // NOLINT, false-positive due to second-order macros.
 }  // NOLINT, false-positive due to second-order macros.
 

src/runtime/runtime-array.cc

@@ -9,6 +9,7 @@
 #include "src/elements.h"
 #include "src/factory.h"
 #include "src/isolate-inl.h"
+#include "src/key-accumulator.h"
 #include "src/messages.h"
 #include "src/prototype.h"
 

tools/gyp/v8.gyp

@@ -843,6 +843,8 @@
         '../../src/isolate.h',
         '../../src/json-parser.h',
         '../../src/json-stringifier.h',
+        '../../src/key-accumulator.h',
+        '../../src/key-accumulator.cc',
         '../../src/layout-descriptor-inl.h',
         '../../src/layout-descriptor.cc',
         '../../src/layout-descriptor.h',