MapCache simplification. It is now a FixedArray that maps number of properties to...
authorishell@chromium.org <ishell@chromium.org>
Mon, 10 Nov 2014 18:03:50 +0000 (18:03 +0000)
committerishell@chromium.org <ishell@chromium.org>
Mon, 10 Nov 2014 18:04:17 +0000 (18:04 +0000)
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
src/factory.cc
src/factory.h
src/heap-snapshot-generator.cc
src/heap/mark-compact.cc
src/heap/mark-compact.h
src/objects-inl.h
src/objects.cc
src/objects.h
src/runtime/runtime-literals.cc
test/cctest/test-api.cc

index 716682d..8182532 100644 (file)
@@ -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.
index 796fd13..e68ac9b 100644 (file)
@@ -2420,35 +2420,42 @@ Handle<JSFunction> Factory::CreateApiFunction(
 }
 
 
-Handle<MapCache> Factory::AddToMapCache(Handle<Context> context,
-                                        Handle<FixedArray> keys,
-                                        Handle<Map> map) {
-  Handle<MapCache> map_cache = handle(MapCache::cast(context->map_cache()));
-  Handle<MapCache> result = MapCache::Put(map_cache, keys, map);
-  context->set_map_cache(*result);
-  return result;
-}
-
-
 Handle<Map> Factory::ObjectLiteralMapFromCache(Handle<Context> context,
-                                               Handle<FixedArray> 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<MapCache> new_cache = MapCache::New(isolate(), 24);
+    Handle<FixedArray> new_cache = NewFixedArray(kMapCacheSize, TENURED);
     context->set_map_cache(*new_cache);
   }
   // Check to see whether there is a matching element in the cache.
-  Handle<MapCache> cache =
-      Handle<MapCache>(MapCache::cast(context->map_cache()));
-  Handle<Object> result = Handle<Object>(cache->Lookup(*keys), isolate());
-  if (result->IsMap()) return Handle<Map>::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> map = length == 0
-                        ? handle(context->object_function()->initial_map())
-                        : Map::Create(isolate(), length);
-  AddToMapCache(context, keys, map);
+  Handle<FixedArray> 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 = Map::Create(isolate(), number_of_properties);
+  Handle<WeakCell> cell = NewWeakCell(map);
+  cache->set(cache_index, *cell);
   return map;
 }
 
@@ -2467,6 +2474,7 @@ void Factory::SetRegExpAtomData(Handle<JSRegExp> regexp,
   regexp->set_data(*store);
 }
 
+
 void Factory::SetRegExpIrregexpData(Handle<JSRegExp> regexp,
                                     JSRegExp::Type type,
                                     Handle<String> source,
@@ -2488,7 +2496,6 @@ void Factory::SetRegExpIrregexpData(Handle<JSRegExp> regexp,
 }
 
 
-
 MaybeHandle<FunctionTemplateInfo> Factory::ConfigureInstance(
     Handle<FunctionTemplateInfo> desc, Handle<JSObject> instance) {
   // Configure the instance by adding the properties specified by the
index 6a9ee55..44bb1be 100644 (file)
@@ -643,10 +643,11 @@ class Factory FINAL {
 
   Handle<DebugInfo> NewDebugInfo(Handle<SharedFunctionInfo> 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<Map> ObjectLiteralMapFromCache(Handle<Context> context,
-                                        Handle<FixedArray> 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<Code> NewCodeRaw(int object_size, bool immovable);
 
-  // Create a new map cache.
-  Handle<MapCache> NewMapCache(int at_least_space_for);
-
-  // Update the map cache in the native context with (keys, map)
-  Handle<MapCache> AddToMapCache(Handle<Context> context,
-                                 Handle<FixedArray> keys,
-                                 Handle<Map> 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<Object> GetNumberStringCache(Handle<Object> number);
index 7f217bb..df99de4 100644 (file)
@@ -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);
   }
 }
index 2cefebf..b929d85 100644 (file)
@@ -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<Context*>(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<MapCache*>(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
index e48d5a3..cd2c9b6 100644 (file)
@@ -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);
index 40ce81a..9564ff0 100644 (file)
@@ -479,11 +479,6 @@ Handle<Object> StringTableShape::AsHandle(Isolate* isolate, HashTableKey* key) {
 }
 
 
-Handle<Object> MapCacheShape::AsHandle(Isolate* isolate, HashTableKey* key) {
-  return key->AsHandle(isolate);
-}
-
-
 Handle<Object> 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)
index 2b5b567..ab13412 100644 (file)
@@ -14207,8 +14207,6 @@ template class HashTable<CompilationCacheTable,
                          CompilationCacheShape,
                          HashTableKey*>;
 
-template class HashTable<MapCache, MapCacheShape, HashTableKey*>;
-
 template class HashTable<ObjectHashTable,
                          ObjectHashTableShape,
                          Handle<Object> >;
@@ -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> MapCache::Put(
-    Handle<MapCache> map_cache, Handle<FixedArray> array, Handle<Map> value) {
-  StringsKey key(array);
-
-  Handle<MapCache> 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<typename Derived, typename Shape, typename Key>
 Handle<Derived> Dictionary<Derived, Shape, Key>::New(
     Isolate* isolate,
index 31d0a16..d025b24 100644 (file)
@@ -3462,44 +3462,6 @@ class StringTable: public HashTable<StringTable,
 };
 
 
-class MapCacheShape : public BaseShape<HashTableKey*> {
- 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<Object> 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<MapCache, MapCacheShape, HashTableKey*> {
- public:
-  // Find cached value for a name key, otherwise return null.
-  Object* Lookup(FixedArray* key);
-  static Handle<MapCache> Put(
-      Handle<MapCache> map_cache, Handle<FixedArray> key, Handle<Map> value);
-  DECLARE_CAST(MapCache)
-
- private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(MapCache);
-};
-
-
 template <typename Derived, typename Shape, typename Key>
 class Dictionary: public HashTable<Derived, Shape, Key> {
  protected:
index c6efd02..8bbe0ee 100644 (file)
@@ -17,53 +17,22 @@ namespace internal {
 static Handle<Map> ComputeObjectLiteralMap(
     Handle<Context> context, Handle<FixedArray> 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<FixedArray> 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<Object> CreateLiteralBoilerplate(
     Isolate* isolate, Handle<FixedArray> literals,
     Handle<FixedArray> constant_properties);
@@ -169,7 +138,6 @@ MUST_USE_RESULT static MaybeHandle<Object> CreateObjectLiteralBoilerplate(
                                 boilerplate->map()->unused_property_fields(),
                                 "FastLiteral");
   }
-
   return boilerplate;
 }
 
index 5fa2a2f..d4fbb04 100644 (file)
@@ -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()));
 }