Commit 9c61327e authored by cbruni's avatar cbruni Committed by Commit bot

[key-accumulator] Starting to reimplement the key-accumulator

Introducing the KeyAccumulator accidentally removed some crucial fast-paths.
This CL starts rewriting the KeyAccumulator, step-by-step introducing the
special cases again.

BUG=chromium:545503, v8:4758
LOG=y

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

Cr-Commit-Position: refs/heads/master@{#34532}
parent ee98147e
......@@ -1139,8 +1139,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/keys.h",
"src/keys.cc",
"src/layout-descriptor-inl.h",
"src/layout-descriptor.cc",
"src/layout-descriptor.h",
......
This diff is collapsed.
......@@ -8,7 +8,7 @@
#include "src/elements-kind.h"
#include "src/heap/heap.h"
#include "src/isolate.h"
#include "src/key-accumulator.h"
#include "src/keys.h"
#include "src/objects.h"
namespace v8 {
......@@ -99,6 +99,19 @@ class ElementsAccessor {
filter, offset);
}
//
virtual Handle<FixedArray> PrependElementIndices(
Handle<JSObject> object, Handle<FixedArrayBase> backing_store,
Handle<FixedArray> keys, GetKeysConversion convert,
PropertyFilter filter = ALL_PROPERTIES) = 0;
inline Handle<FixedArray> PrependElementIndices(
Handle<JSObject> object, Handle<FixedArray> keys,
GetKeysConversion convert, PropertyFilter filter = ALL_PROPERTIES) {
return PrependElementIndices(object, handle(object->elements()), keys,
convert, filter);
}
virtual void AddElementsToKeyAccumulator(Handle<JSObject> receiver,
KeyAccumulator* accumulator,
AddKeyConversion convert) = 0;
......
......@@ -2,26 +2,24 @@
// 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/keys.h"
#include "src/elements.h"
#include "src/factory.h"
#include "src/isolate-inl.h"
#include "src/objects-inl.h"
#include "src/property-descriptor.h"
#include "src/prototype.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();
......@@ -86,7 +84,6 @@ Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
return result;
}
namespace {
bool AccumulatorHasKey(std::vector<uint32_t>* sub_elements, uint32_t key) {
......@@ -99,7 +96,6 @@ 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;
......@@ -136,10 +132,8 @@ bool KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
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
......@@ -154,7 +148,6 @@ bool KeyAccumulator::AddIntegerKey(uint32_t key) {
return true;
}
bool KeyAccumulator::AddStringKey(Handle<Object> key,
AddKeyConversion convert) {
if (string_properties_.is_null()) {
......@@ -176,7 +169,6 @@ bool KeyAccumulator::AddStringKey(Handle<Object> key,
}
}
bool KeyAccumulator::AddSymbolKey(Handle<Object> key) {
if (symbol_properties_.is_null()) {
symbol_properties_ = OrderedHashSet::Allocate(isolate_, 16);
......@@ -192,7 +184,6 @@ bool KeyAccumulator::AddSymbolKey(Handle<Object> key) {
}
}
void KeyAccumulator::AddKeys(Handle<FixedArray> array,
AddKeyConversion convert) {
int add_length = array->length();
......@@ -203,7 +194,6 @@ void KeyAccumulator::AddKeys(Handle<FixedArray> array,
}
}
void KeyAccumulator::AddKeys(Handle<JSObject> array_like,
AddKeyConversion convert) {
DCHECK(array_like->IsJSArray() || array_like->HasSloppyArgumentsElements());
......@@ -211,7 +201,6 @@ void KeyAccumulator::AddKeys(Handle<JSObject> array_like,
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
......@@ -223,7 +212,6 @@ void KeyAccumulator::AddKeysFromProxy(Handle<JSObject> array_like) {
level_string_length_ = -level_string_length_;
}
MaybeHandle<FixedArray> FilterProxyKeys(Isolate* isolate, Handle<JSProxy> owner,
Handle<FixedArray> keys,
PropertyFilter filter) {
......@@ -253,7 +241,6 @@ MaybeHandle<FixedArray> FilterProxyKeys(Isolate* isolate, Handle<JSProxy> owner,
return keys;
}
// Returns "nothing" in case of exception, "true" on success.
Maybe<bool> KeyAccumulator::AddKeysFromProxy(Handle<JSProxy> proxy,
Handle<FixedArray> keys) {
......@@ -277,7 +264,6 @@ Maybe<bool> KeyAccumulator::AddKeysFromProxy(Handle<JSProxy> proxy,
return Just(true);
}
void KeyAccumulator::AddElementKeysFromInterceptor(
Handle<JSObject> array_like) {
AddKeys(array_like, CONVERT_TO_ARRAY_INDEX);
......@@ -286,7 +272,6 @@ void KeyAccumulator::AddElementKeysFromInterceptor(
SortCurrentElementsListRemoveDuplicates();
}
void KeyAccumulator::SortCurrentElementsListRemoveDuplicates() {
// Sort and remove duplicates from the current elements level and adjust.
// the lengths accordingly.
......@@ -300,14 +285,12 @@ void KeyAccumulator::SortCurrentElementsListRemoveDuplicates() {
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()) {
......@@ -319,6 +302,125 @@ void KeyAccumulator::NextPrototype() {
level_symbol_length_ = 0;
}
namespace {
void TrySettingEmptyEnumCache(JSReceiver* object) {
Map* map = object->map();
DCHECK_EQ(kInvalidEnumCacheSentinel, map->EnumLength());
if (!map->OnlyHasSimpleProperties()) return;
if (map->IsJSProxyMap()) return;
if (map->NumberOfOwnDescriptors() > 0) {
int number_of_enumerable_own_properties =
map->NumberOfDescribedProperties(OWN_DESCRIPTORS, ENUMERABLE_STRINGS);
if (number_of_enumerable_own_properties > 0) return;
}
DCHECK(object->IsJSObject());
map->SetEnumLength(0);
}
bool CheckAndInitalizeSimpleEnumCache(JSReceiver* object) {
if (object->map()->EnumLength() == kInvalidEnumCacheSentinel) {
TrySettingEmptyEnumCache(object);
}
if (object->map()->EnumLength() != 0) return false;
DCHECK(object->IsJSObject());
return !JSObject::cast(object)->HasEnumerableElements();
}
} // namespace
void FastKeyAccumulator::Prepare() {
DisallowHeapAllocation no_gc;
// Directly go for the fast path for OWN_ONLY keys.
if (type_ == OWN_ONLY) return;
// Fully walk the prototype chain and find the last prototype with keys.
is_receiver_simple_enum_ = false;
has_empty_prototype_ = true;
JSReceiver* first_non_empty_prototype;
for (PrototypeIterator iter(isolate_, *receiver_); !iter.IsAtEnd();
iter.Advance()) {
JSReceiver* current = iter.GetCurrent<JSReceiver>();
if (CheckAndInitalizeSimpleEnumCache(current)) continue;
has_empty_prototype_ = false;
first_non_empty_prototype = current;
// TODO(cbruni): use the first non-empty prototype.
USE(first_non_empty_prototype);
return;
}
DCHECK(has_empty_prototype_);
is_receiver_simple_enum_ =
receiver_->map()->EnumLength() != kInvalidEnumCacheSentinel &&
!JSObject::cast(*receiver_)->HasEnumerableElements();
}
namespace {
Handle<FixedArray> GetOwnKeysWithElements(Isolate* isolate,
Handle<JSObject> object,
GetKeysConversion convert) {
Handle<FixedArray> keys = JSObject::GetFastEnumPropertyKeys(isolate, object);
ElementsAccessor* accessor = object->GetElementsAccessor();
return accessor->PrependElementIndices(object, keys, convert,
ONLY_ENUMERABLE);
}
MaybeHandle<FixedArray> GetOwnKeysWithUninitializedEnumCache(
Isolate* isolate, Handle<JSObject> object) {
// Uninitalized enum cache
Map* map = object->map();
if (object->elements() != isolate->heap()->empty_fixed_array() ||
object->elements() != isolate->heap()->empty_slow_element_dictionary()) {
// Assume that there are elements.
return MaybeHandle<FixedArray>();
}
int number_of_own_descriptors = map->NumberOfOwnDescriptors();
if (number_of_own_descriptors == 0) {
map->SetEnumLength(0);
return isolate->factory()->empty_fixed_array();
}
// We have no elements but possibly enumerable property keys, hence we can
// directly initialize the enum cache.
return JSObject::GetFastEnumPropertyKeys(isolate, object);
}
} // namespace
MaybeHandle<FixedArray> FastKeyAccumulator::GetKeys(GetKeysConversion convert) {
Handle<FixedArray> keys;
if (GetKeysFast(convert).ToHandle(&keys)) {
return keys;
}
return GetKeysSlow(convert);
}
MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysFast(
GetKeysConversion convert) {
bool own_only = has_empty_prototype_ || type_ == OWN_ONLY;
if (!own_only || !receiver_->map()->OnlyHasSimpleProperties()) {
return MaybeHandle<FixedArray>();
}
Handle<FixedArray> keys;
DCHECK(receiver_->IsJSObject());
Handle<JSObject> object = Handle<JSObject>::cast(receiver_);
int enum_length = receiver_->map()->EnumLength();
if (enum_length == kInvalidEnumCacheSentinel) {
// Try initializing the enum cache and return own properties.
if (GetOwnKeysWithUninitializedEnumCache(isolate_, object)
.ToHandle(&keys)) {
is_receiver_simple_enum_ =
object->map()->EnumLength() != kInvalidEnumCacheSentinel;
return keys;
}
}
// The properties-only case failed because there were probably elements on the
// receiver.
return GetOwnKeysWithElements(isolate_, object, convert);
}
MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysSlow(
GetKeysConversion convert) {
return JSReceiver::GetKeys(receiver_, type_, ENUMERABLE_STRINGS);
}
} // namespace internal
} // namespace v8
......@@ -86,9 +86,40 @@ class KeyAccumulator final BASE_EMBEDDED {
DISALLOW_COPY_AND_ASSIGN(KeyAccumulator);
};
// The FastKeyAccumulator handles the cases where there are no elements on the
// prototype chain and forwords the complex/slow cases to the normal
// KeyAccumulator.
class FastKeyAccumulator {
public:
FastKeyAccumulator(Isolate* isolate, Handle<JSReceiver> receiver,
KeyCollectionType type, PropertyFilter filter)
: isolate_(isolate), receiver_(receiver), type_(type), filter_(filter) {
Prepare();
// TODO(cbruni): pass filter_ directly to the KeyAccumulator.
USE(filter_);
}
bool is_receiver_simple_enum() { return is_receiver_simple_enum_; }
bool has_empty_prototype() { return has_empty_prototype_; }
MaybeHandle<FixedArray> GetKeys(GetKeysConversion convert = KEEP_NUMBERS);
private:
void Prepare();
MaybeHandle<FixedArray> GetKeysFast(GetKeysConversion convert);
MaybeHandle<FixedArray> GetKeysSlow(GetKeysConversion convert);
Isolate* isolate_;
Handle<JSReceiver> receiver_;
KeyCollectionType type_;
PropertyFilter filter_;
bool is_receiver_simple_enum_ = false;
bool has_empty_prototype_ = false;
DISALLOW_COPY_AND_ASSIGN(FastKeyAccumulator);
};
} // namespace internal
} // namespace v8
#endif // V8_KEY_ACCUMULATOR_H_
......@@ -455,6 +455,12 @@ void Map::MapPrint(std::ostream& os) { // NOLINT
}
os << "\n - elements kind: " << ElementsKindToString(elements_kind());
os << "\n - unused property fields: " << unused_property_fields();
os << "\n - enum length: ";
if (EnumLength() == kInvalidEnumCacheSentinel) {
os << "invalid";
} else {
os << EnumLength();
}
if (is_deprecated()) os << "\n - deprecated_map";
if (is_stable()) os << "\n - stable_map";
if (is_dictionary_map()) os << "\n - dictionary_map";
......
......@@ -34,7 +34,7 @@
#include "src/interpreter/bytecodes.h"
#include "src/interpreter/source-position-table.h"
#include "src/isolate-inl.h"
#include "src/key-accumulator.h"
#include "src/keys.h"
#include "src/list.h"
#include "src/log.h"
#include "src/lookup.h"
......@@ -8226,7 +8226,9 @@ MaybeHandle<Object> JSReceiver::OrdinaryToPrimitive(
// TODO(cbruni/jkummerow): Consider moving this into elements.cc.
bool HasEnumerableElements(JSObject* object) {
bool JSObject::HasEnumerableElements() {
// TODO(cbruni): cleanup
JSObject* object = this;
switch (object->GetElementsKind()) {
case FAST_SMI_ELEMENTS:
case FAST_ELEMENTS:
......@@ -8291,7 +8293,6 @@ bool HasEnumerableElements(JSObject* object) {
return true;
}
// Tests for the fast common case for property enumeration:
// - This object and all prototypes has an enum cache (which means that
// it is no proxy, has no interceptors and needs no access checks).
......@@ -8308,7 +8309,7 @@ bool JSReceiver::IsSimpleEnum() {
if (current->IsAccessCheckNeeded()) return false;
DCHECK(!current->HasNamedInterceptor());
DCHECK(!current->HasIndexedInterceptor());
if (HasEnumerableElements(current)) return false;
if (current->HasEnumerableElements()) return false;
if (current != this && enum_length != 0) return false;
}
return true;
......@@ -8372,10 +8373,9 @@ bool Map::OnlyHasSimpleProperties() {
!has_hidden_prototype() && !is_dictionary_map();
}
namespace {
Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
Handle<JSObject> object) {
// static
Handle<FixedArray> JSObject::GetFastEnumPropertyKeys(Isolate* isolate,
Handle<JSObject> object) {
Handle<Map> map(object->map());
bool cache_enum_length = map->OnlyHasSimpleProperties();
......@@ -8426,8 +8426,9 @@ Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
for (int i = 0; i < size; i++) {
PropertyDetails details = descs->GetDetails(i);
if (details.IsDontEnum()) continue;
Object* key = descs->GetKey(i);
if (details.IsDontEnum() || key->IsSymbol()) continue;
if (key->IsSymbol()) continue;
storage->set(index, key);
if (!indices.is_null()) {
if (details.type() != DATA) {
......@@ -8449,7 +8450,6 @@ Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
return storage;
}
} // namespace
Handle<FixedArray> JSObject::GetEnumPropertyKeys(Handle<JSObject> object) {
Isolate* isolate = object->GetIsolate();
......@@ -10665,7 +10665,6 @@ Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object> obj,
return array;
}
Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object> obj1,
Handle<Object> obj2, AddMode mode) {
int length = array->Length();
......@@ -16427,7 +16426,6 @@ void FixedArray::SortPairs(FixedArray* numbers, uint32_t len) {
}
}
void JSObject::CollectOwnPropertyNames(KeyAccumulator* keys,
PropertyFilter filter) {
if (HasFastProperties()) {
......@@ -16468,7 +16466,6 @@ int JSObject::NumberOfOwnElements(PropertyFilter filter) {
return GetOwnElementKeys(NULL, filter);
}
void JSObject::CollectOwnElementKeys(Handle<JSObject> object,
KeyAccumulator* keys,
PropertyFilter filter) {
......@@ -18476,7 +18473,6 @@ int Dictionary<Derived, Shape, Key>::CopyKeysTo(
return index - start_index;
}
template <typename Derived, typename Shape, typename Key>
void Dictionary<Derived, Shape, Key>::CollectKeysTo(
Handle<Dictionary<Derived, Shape, Key> > dictionary, KeyAccumulator* keys,
......
......@@ -2043,6 +2043,8 @@ class JSObject: public JSReceiver {
inline bool HasSlowArgumentsElements();
inline bool HasFastStringWrapperElements();
inline bool HasSlowStringWrapperElements();
bool HasEnumerableElements();
inline SeededNumberDictionary* element_dictionary(); // Gets slow elements.
// Requires: HasFastElements().
......@@ -2292,6 +2294,9 @@ class JSObject: public JSReceiver {
static Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object);
static Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
Handle<JSObject> object);
// Returns a new map with all transitions dropped from the object's current
// map and the ElementsKind set.
static Handle<Map> GetElementsTransitionMap(Handle<JSObject> object,
......
......@@ -9,7 +9,7 @@
#include "src/elements.h"
#include "src/factory.h"
#include "src/isolate-inl.h"
#include "src/key-accumulator.h"
#include "src/keys.h"
#include "src/messages.h"
#include "src/prototype.h"
......
......@@ -5,8 +5,10 @@
#include "src/runtime/runtime-utils.h"
#include "src/arguments.h"
#include "src/elements.h"
#include "src/factory.h"
#include "src/isolate-inl.h"
#include "src/keys.h"
#include "src/objects-inl.h"
namespace v8 {
......@@ -20,15 +22,15 @@ namespace {
// deletions during a for-in.
MaybeHandle<HeapObject> Enumerate(Handle<JSReceiver> receiver) {
Isolate* const isolate = receiver->GetIsolate();
FastKeyAccumulator accumulator(isolate, receiver, INCLUDE_PROTOS,
ENUMERABLE_STRINGS);
// Test if we have an enum cache for {receiver}.
if (!receiver->IsSimpleEnum()) {
if (!accumulator.is_receiver_simple_enum()) {
Handle<FixedArray> keys;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, keys,
JSReceiver::GetKeys(receiver, INCLUDE_PROTOS, ENUMERABLE_STRINGS),
HeapObject);
ASSIGN_RETURN_ON_EXCEPTION(isolate, keys, accumulator.GetKeys(KEEP_NUMBERS),
HeapObject);
// Test again, since cache may have been built by GetKeys() calls above.
if (!receiver->IsSimpleEnum()) return keys;
if (!accumulator.is_receiver_simple_enum()) return keys;
}
return handle(receiver->map(), isolate);
}
......@@ -36,10 +38,19 @@ MaybeHandle<HeapObject> Enumerate(Handle<JSReceiver> receiver) {
MaybeHandle<Object> Filter(Handle<JSReceiver> receiver, Handle<Object> key) {
Isolate* const isolate = receiver->GetIsolate();
// TODO(turbofan): Fast case for array indices.
Handle<Name> name;
ASSIGN_RETURN_ON_EXCEPTION(isolate, name, Object::ToName(isolate, key),
Object);
// Directly check for elements if the key is a smi and avoid a conversion
// roundtrip (Number -> Name -> Number).
if (key->IsNumber() && receiver->map()->OnlyHasSimpleProperties()) {
Handle<JSObject> object = Handle<JSObject>::cast(receiver);
ElementsAccessor* accessor = object->GetElementsAccessor();
DCHECK_LT(key->Number(), kMaxUInt32);
if (accessor->HasElement(object, key->Number(), ONLY_ENUMERABLE)) {
return name;
}
}
Maybe<bool> result = JSReceiver::HasProperty(receiver, name);
MAYBE_RETURN_NULL(result);
if (result.FromJust()) return name;
......
......@@ -120,7 +120,23 @@ for (i=0 ; i < 3; ++i) {
assertEquals("undefined", typeof y[0], "y[0]");
}
(function() {
(function testLargeElementKeys() {
// Key out of SMI range but well within safe double representaion.
var large_key = 2147483650;
var o = [];
// Trigger dictionary elements with HeapNumber keys.
o[large_key] = 0;
o[large_key+1] = 1;
o[large_key+2] = 2;
o[large_key+3] = 3;
var keys = [];
for (var k in o) {
keys.push(k);
}
assertEquals(["2147483650", "2147483651", "2147483652", "2147483653"], keys);
})();
(function testLargeElementKeysWithProto() {
var large_key = 2147483650;
var o = {__proto__: {}};
o[large_key] = 1;
......@@ -131,3 +147,17 @@ for (i=0 ; i < 3; ++i) {
}
assertEquals(["2147483650"], keys);
})();
(function testNonEnumerableArgumentsIndex() {
Object.defineProperty(arguments, 0, {enumerable:false});
for (var k in arguments) {
assertUnreachable();
}
})();
(function testNonEnumerableSloppyArgumentsIndex(a) {
Object.defineProperty(arguments, 0, {enumerable:false});
for (var k in arguments) {
assertUnreachable();
}
})(true);
......@@ -957,8 +957,8 @@
'../../src/isolate.h',
'../../src/json-parser.h',
'../../src/json-stringifier.h',
'../../src/key-accumulator.h',
'../../src/key-accumulator.cc',
'../../src/keys.h',
'../../src/keys.cc',
'../../src/layout-descriptor-inl.h',
'../../src/layout-descriptor.cc',
'../../src/layout-descriptor.h',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment