From 18cf6c9ac96a62ec320fc10d738d26fba784e2f4 Mon Sep 17 00:00:00 2001 From: "ishell@chromium.org" Date: Mon, 10 Nov 2014 18:03:50 +0000 Subject: [PATCH] MapCache simplification. It is now a FixedArray that maps number of properties to a WeakCell with a Map. R=verwaest@chromium.org Review URL: https://codereview.chromium.org/712943002 Cr-Commit-Position: refs/heads/master@{#25253} git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25253 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/contexts.h | 2 +- src/factory.cc | 55 +++++++++++++++++-------------- src/factory.h | 15 +++------ src/heap-snapshot-generator.cc | 2 +- src/heap/mark-compact.cc | 57 --------------------------------- src/heap/mark-compact.h | 4 --- src/objects-inl.h | 6 ---- src/objects.cc | 24 -------------- src/objects.h | 38 ---------------------- src/runtime/runtime-literals.cc | 42 +++--------------------- test/cctest/test-api.cc | 41 +++++++++++++++--------- 11 files changed, 68 insertions(+), 218 deletions(-) diff --git a/src/contexts.h b/src/contexts.h index 716682d273..8182532039 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -413,13 +413,13 @@ class Context: public FixedArray { UNSCOPABLES_SYMBOL_INDEX, ARRAY_VALUES_ITERATOR_INDEX, GLOBAL_CONTEXT_TABLE_INDEX, + MAP_CACHE_INDEX, // Properties from here are treated as weak references by the full GC. // Scavenge treats them as strong references. OPTIMIZED_FUNCTIONS_LIST, // Weak. OPTIMIZED_CODE_LIST, // Weak. DEOPTIMIZED_CODE_LIST, // Weak. - MAP_CACHE_INDEX, // Weak. NEXT_CONTEXT_LINK, // Weak. // Total number of slots. diff --git a/src/factory.cc b/src/factory.cc index 796fd13c08..e68ac9b6fa 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -2420,35 +2420,42 @@ Handle Factory::CreateApiFunction( } -Handle Factory::AddToMapCache(Handle context, - Handle keys, - Handle map) { - Handle map_cache = handle(MapCache::cast(context->map_cache())); - Handle result = MapCache::Put(map_cache, keys, map); - context->set_map_cache(*result); - return result; -} - - Handle Factory::ObjectLiteralMapFromCache(Handle context, - Handle keys) { + int number_of_properties, + bool* is_result_from_cache) { + const int kMapCacheSize = 128; + + if (number_of_properties > kMapCacheSize) { + *is_result_from_cache = false; + return Map::Create(isolate(), number_of_properties); + } + *is_result_from_cache = true; + if (number_of_properties == 0) { + // Reuse the initial map of the Object function if the literal has no + // predeclared properties. + return handle(context->object_function()->initial_map(), isolate()); + } + int cache_index = number_of_properties - 1; if (context->map_cache()->IsUndefined()) { // Allocate the new map cache for the native context. - Handle new_cache = MapCache::New(isolate(), 24); + Handle new_cache = NewFixedArray(kMapCacheSize, TENURED); context->set_map_cache(*new_cache); } // Check to see whether there is a matching element in the cache. - Handle cache = - Handle(MapCache::cast(context->map_cache())); - Handle result = Handle(cache->Lookup(*keys), isolate()); - if (result->IsMap()) return Handle::cast(result); - int length = keys->length(); - // Create a new map and add it to the cache. Reuse the initial map of the - // Object function if the literal has no predeclared properties. - Handle map = length == 0 - ? handle(context->object_function()->initial_map()) - : Map::Create(isolate(), length); - AddToMapCache(context, keys, map); + Handle cache(FixedArray::cast(context->map_cache())); + { + Object* result = cache->get(cache_index); + if (result->IsWeakCell()) { + WeakCell* cell = WeakCell::cast(result); + if (!cell->cleared()) { + return handle(Map::cast(cell->value()), isolate()); + } + } + } + // Create a new map and add it to the cache. + Handle map = Map::Create(isolate(), number_of_properties); + Handle cell = NewWeakCell(map); + cache->set(cache_index, *cell); return map; } @@ -2467,6 +2474,7 @@ void Factory::SetRegExpAtomData(Handle regexp, regexp->set_data(*store); } + void Factory::SetRegExpIrregexpData(Handle regexp, JSRegExp::Type type, Handle source, @@ -2488,7 +2496,6 @@ void Factory::SetRegExpIrregexpData(Handle regexp, } - MaybeHandle Factory::ConfigureInstance( Handle desc, Handle instance) { // Configure the instance by adding the properties specified by the diff --git a/src/factory.h b/src/factory.h index 6a9ee5506a..44bb1be531 100644 --- a/src/factory.h +++ b/src/factory.h @@ -643,10 +643,11 @@ class Factory FINAL { Handle NewDebugInfo(Handle shared); - // Return a map using the map cache in the native context. - // The key the an ordered set of property names. + // Return a map for given number of properties using the map cache in the + // native context. Handle ObjectLiteralMapFromCache(Handle context, - Handle keys); + int number_of_properties, + bool* is_result_from_cache); // Creates a new FixedArray that holds the data associated with the // atom regexp and stores it in the regexp. @@ -689,14 +690,6 @@ class Factory FINAL { // Creates a code object that is not yet fully initialized yet. inline Handle NewCodeRaw(int object_size, bool immovable); - // Create a new map cache. - Handle NewMapCache(int at_least_space_for); - - // Update the map cache in the native context with (keys, map) - Handle AddToMapCache(Handle context, - Handle keys, - Handle map); - // Attempt to find the number in a small cache. If we finds it, return // the string representation of the number. Otherwise return undefined. Handle GetNumberStringCache(Handle number); diff --git a/src/heap-snapshot-generator.cc b/src/heap-snapshot-generator.cc index 7f217bb8d6..df99de4d98 100644 --- a/src/heap-snapshot-generator.cc +++ b/src/heap-snapshot-generator.cc @@ -1282,7 +1282,7 @@ void V8HeapExplorer::ExtractContextReferences(int entry, Context* context) { Context::FIRST_WEAK_SLOT); STATIC_ASSERT(Context::NEXT_CONTEXT_LINK + 1 == Context::NATIVE_CONTEXT_SLOTS); - STATIC_ASSERT(Context::FIRST_WEAK_SLOT + 5 == + STATIC_ASSERT(Context::FIRST_WEAK_SLOT + 4 == Context::NATIVE_CONTEXT_SLOTS); } } diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index 2cefebf980..b929d8567f 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -2232,12 +2232,6 @@ void MarkCompactCollector::MarkLiveObjects() { void MarkCompactCollector::AfterMarking() { - // Object literal map caches reference strings (cache keys) and maps - // (cache values). At this point still useful maps have already been - // marked. Mark the keys for the alive values before we process the - // string table. - ProcessMapCaches(); - // Prune the string table removing all strings only pointed to by the // string table. Cannot use string_table() here because the string // table is marked. @@ -2274,57 +2268,6 @@ void MarkCompactCollector::AfterMarking() { } -void MarkCompactCollector::ProcessMapCaches() { - Object* raw_context = heap()->native_contexts_list(); - while (raw_context != heap()->undefined_value()) { - Context* context = reinterpret_cast(raw_context); - if (IsMarked(context)) { - HeapObject* raw_map_cache = - HeapObject::cast(context->get(Context::MAP_CACHE_INDEX)); - // A map cache may be reachable from the stack. In this case - // it's already transitively marked and it's too late to clean - // up its parts. - if (!IsMarked(raw_map_cache) && - raw_map_cache != heap()->undefined_value()) { - MapCache* map_cache = reinterpret_cast(raw_map_cache); - int existing_elements = map_cache->NumberOfElements(); - int used_elements = 0; - for (int i = MapCache::kElementsStartIndex; i < map_cache->length(); - i += MapCache::kEntrySize) { - Object* raw_key = map_cache->get(i); - if (raw_key == heap()->undefined_value() || - raw_key == heap()->the_hole_value()) - continue; - STATIC_ASSERT(MapCache::kEntrySize == 2); - Object* raw_map = map_cache->get(i + 1); - if (raw_map->IsHeapObject() && IsMarked(raw_map)) { - ++used_elements; - } else { - // Delete useless entries with unmarked maps. - DCHECK(raw_map->IsMap()); - map_cache->set_the_hole(i); - map_cache->set_the_hole(i + 1); - } - } - if (used_elements == 0) { - context->set(Context::MAP_CACHE_INDEX, heap()->undefined_value()); - } else { - // Note: we don't actually shrink the cache here to avoid - // extra complexity during GC. We rely on subsequent cache - // usages (EnsureCapacity) to do this. - map_cache->ElementsRemoved(existing_elements - used_elements); - MarkBit map_cache_markbit = Marking::MarkBitFrom(map_cache); - MarkObject(map_cache, map_cache_markbit); - } - } - } - // Move to next element in the list. - raw_context = context->get(Context::NEXT_CONTEXT_LINK); - } - ProcessMarkingDeque(); -} - - void MarkCompactCollector::ClearNonLiveReferences() { // Iterate over the map space, setting map transitions that go from // a marked map to an unmarked map to null transitions. This action diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h index e48d5a3d22..cd2c9b68c8 100644 --- a/src/heap/mark-compact.h +++ b/src/heap/mark-compact.h @@ -786,10 +786,6 @@ class MarkCompactCollector { // flag on the marking stack. void RefillMarkingDeque(); - // After reachable maps have been marked process per context object - // literal map caches removing unmarked entries. - void ProcessMapCaches(); - // Callback function for telling whether the object *p is an unmarked // heap object. static bool IsUnmarkedHeapObject(Object** p); diff --git a/src/objects-inl.h b/src/objects-inl.h index 40ce81a9b7..9564ff0be0 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -479,11 +479,6 @@ Handle StringTableShape::AsHandle(Isolate* isolate, HashTableKey* key) { } -Handle MapCacheShape::AsHandle(Isolate* isolate, HashTableKey* key) { - return key->AsHandle(isolate); -} - - Handle CompilationCacheShape::AsHandle(Isolate* isolate, HashTableKey* key) { return key->AsHandle(isolate); @@ -3288,7 +3283,6 @@ CAST_ACCESSOR(JSValue) CAST_ACCESSOR(JSWeakMap) CAST_ACCESSOR(JSWeakSet) CAST_ACCESSOR(Map) -CAST_ACCESSOR(MapCache) CAST_ACCESSOR(Name) CAST_ACCESSOR(NameDictionary) CAST_ACCESSOR(NormalizedMapCache) diff --git a/src/objects.cc b/src/objects.cc index 2b5b567112..ab13412f52 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -14207,8 +14207,6 @@ template class HashTable; -template class HashTable; - template class HashTable >; @@ -15158,28 +15156,6 @@ class StringsKey : public HashTableKey { }; -Object* MapCache::Lookup(FixedArray* array) { - DisallowHeapAllocation no_alloc; - StringsKey key(handle(array)); - int entry = FindEntry(&key); - if (entry == kNotFound) return GetHeap()->undefined_value(); - return get(EntryToIndex(entry) + 1); -} - - -Handle MapCache::Put( - Handle map_cache, Handle array, Handle value) { - StringsKey key(array); - - Handle new_cache = EnsureCapacity(map_cache, 1, &key); - int entry = new_cache->FindInsertionEntry(key.Hash()); - new_cache->set(EntryToIndex(entry), *array); - new_cache->set(EntryToIndex(entry) + 1, *value); - new_cache->ElementAdded(); - return new_cache; -} - - template Handle Dictionary::New( Isolate* isolate, diff --git a/src/objects.h b/src/objects.h index 31d0a16dfb..d025b24823 100644 --- a/src/objects.h +++ b/src/objects.h @@ -3462,44 +3462,6 @@ class StringTable: public HashTable { - public: - static inline bool IsMatch(HashTableKey* key, Object* value) { - return key->IsMatch(value); - } - - static inline uint32_t Hash(HashTableKey* key) { - return key->Hash(); - } - - static inline uint32_t HashForObject(HashTableKey* key, Object* object) { - return key->HashForObject(object); - } - - static inline Handle AsHandle(Isolate* isolate, HashTableKey* key); - - static const int kPrefixSize = 0; - static const int kEntrySize = 2; -}; - - -// MapCache. -// -// Maps keys that are a fixed array of unique names to a map. -// Used for canonicalize maps for object literals. -class MapCache: public HashTable { - public: - // Find cached value for a name key, otherwise return null. - Object* Lookup(FixedArray* key); - static Handle Put( - Handle map_cache, Handle key, Handle value); - DECLARE_CAST(MapCache) - - private: - DISALLOW_IMPLICIT_CONSTRUCTORS(MapCache); -}; - - template class Dictionary: public HashTable { protected: diff --git a/src/runtime/runtime-literals.cc b/src/runtime/runtime-literals.cc index c6efd02dba..8bbe0eeddb 100644 --- a/src/runtime/runtime-literals.cc +++ b/src/runtime/runtime-literals.cc @@ -17,53 +17,22 @@ namespace internal { static Handle ComputeObjectLiteralMap( Handle context, Handle constant_properties, bool* is_result_from_cache) { - Isolate* isolate = context->GetIsolate(); int properties_length = constant_properties->length(); int number_of_properties = properties_length / 2; - // Check that there are only internal strings and array indices among keys. - int number_of_string_keys = 0; + for (int p = 0; p != properties_length; p += 2) { Object* key = constant_properties->get(p); uint32_t element_index = 0; - if (key->IsInternalizedString()) { - number_of_string_keys++; - } else if (key->ToArrayIndex(&element_index)) { + if (key->ToArrayIndex(&element_index)) { // An index key does not require space in the property backing store. number_of_properties--; - } else { - // Bail out as a non-internalized-string non-index key makes caching - // impossible. - // DCHECK to make sure that the if condition after the loop is false. - DCHECK(number_of_string_keys != number_of_properties); - break; } } - // If we only have internalized strings and array indices among keys then we - // can use the map cache in the native context. - const int kMaxKeys = 10; - if ((number_of_string_keys == number_of_properties) && - (number_of_string_keys < kMaxKeys)) { - // Create the fixed array with the key. - Handle keys = - isolate->factory()->NewFixedArray(number_of_string_keys); - if (number_of_string_keys > 0) { - int index = 0; - for (int p = 0; p < properties_length; p += 2) { - Object* key = constant_properties->get(p); - if (key->IsInternalizedString()) { - keys->set(index++, key); - } - } - DCHECK(index == number_of_string_keys); - } - *is_result_from_cache = true; - return isolate->factory()->ObjectLiteralMapFromCache(context, keys); - } - *is_result_from_cache = false; - return Map::Create(isolate, number_of_properties); + Isolate* isolate = context->GetIsolate(); + return isolate->factory()->ObjectLiteralMapFromCache( + context, number_of_properties, is_result_from_cache); } - MUST_USE_RESULT static MaybeHandle CreateLiteralBoilerplate( Isolate* isolate, Handle literals, Handle constant_properties); @@ -169,7 +138,6 @@ MUST_USE_RESULT static MaybeHandle CreateObjectLiteralBoilerplate( boilerplate->map()->unused_property_fields(), "FastLiteral"); } - return boilerplate; } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 5fa2a2fb3a..d4fbb046bf 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -21211,29 +21211,40 @@ THREADED_TEST(ReadOnlyIndexedProperties) { } +static int CountLiveMapsInMapCache(i::Context* context) { + i::FixedArray* map_cache = i::FixedArray::cast(context->map_cache()); + int length = map_cache->length(); + int count = 0; + for (int i = 0; i < length; i++) { + i::Object* value = map_cache->get(i); + if (value->IsWeakCell() && !i::WeakCell::cast(value)->cleared()) count++; + } + return count; +} + + THREADED_TEST(Regress1516) { LocalContext context; v8::HandleScope scope(context->GetIsolate()); + // Object with 20 properties is not a common case, so it should be removed + // from the cache after GC. { v8::HandleScope temp_scope(context->GetIsolate()); - CompileRun("({'a': 0})"); + CompileRun( + "({" + "'a00': 0, 'a01': 0, 'a02': 0, 'a03': 0, 'a04': 0, " + "'a05': 0, 'a06': 0, 'a07': 0, 'a08': 0, 'a09': 0, " + "'a10': 0, 'a11': 0, 'a12': 0, 'a13': 0, 'a14': 0, " + "'a15': 0, 'a16': 0, 'a17': 0, 'a18': 0, 'a19': 0, " + "})"); } - int elements; - { i::MapCache* map_cache = - i::MapCache::cast(CcTest::i_isolate()->context()->map_cache()); - elements = map_cache->NumberOfElements(); - CHECK_LE(1, elements); - } + int elements = CountLiveMapsInMapCache(CcTest::i_isolate()->context()); + CHECK_LE(1, elements); - CcTest::heap()->CollectAllGarbage( - i::Heap::kAbortIncrementalMarkingMask); - { i::Object* raw_map_cache = CcTest::i_isolate()->context()->map_cache(); - if (raw_map_cache != CcTest::heap()->undefined_value()) { - i::MapCache* map_cache = i::MapCache::cast(raw_map_cache); - CHECK_GT(elements, map_cache->NumberOfElements()); - } - } + CcTest::heap()->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask); + + CHECK_GT(elements, CountLiveMapsInMapCache(CcTest::i_isolate()->context())); }