Changeset 181297 in webkit for trunk/Source/JavaScriptCore/runtime/WeakGCMap.h

Timestamp:

Mar 9, 2015, 5:09:39 PM (10 years ago)

Author:

[email protected]

Message:

Stale entries in WeakGCMaps are keeping tons of WeakBlocks alive unnecessarily.
<​https://webkit.org/b/142115>
<rdar://problem/19992268>

Reviewed by Geoffrey Garen.

Prune stale entries from WeakGCMaps as part of every full garbage collection.
This frees up tons of previously-stuck WeakBlocks that were only sitting around
with finalized handles waiting to die.

Note that WeakGCMaps register/unregister themselves with the GC heap in their
ctor/dtor, so creating one now requires passing the VM.

Average time spent in the PruningStaleEntriesFromWeakGCMaps GC phase appears
to be between 0.01ms and 0.3ms, though I've seen a few longer ones at ~1.2ms.
It seems somewhat excessive to do this on every Eden collection, so it's only
doing work in full collections for now.

Because the GC may now mutate WeakGCMap below object allocation, I've made it
so that the classic HashMap::add() optimization can't be used with WeakGCMap.
This caused intermittent test failures when originally landed due to having
an invalid iterator on the stack after add() inserted a new entry and we
proceeded to allocate the new object, triggering GC.

• API/JSWeakObjectMapRefInternal.h:

(OpaqueJSWeakObjectMap::create):
(OpaqueJSWeakObjectMap::OpaqueJSWeakObjectMap):

• API/JSWeakObjectMapRefPrivate.cpp:
• API/JSWrapperMap.mm:

(-[JSWrapperMap initWithContext:]):
(-[JSWrapperMap jsWrapperForObject:]): Pass VM to WeakGCMap constructor.

• JavaScriptCore.xcodeproj/project.pbxproj: Add WeakGCMapInlines.h and make

it project-private so WebCore clients can access it.

• heap/Heap.cpp:

(JSC::Heap::collect):
(JSC::Heap::pruneStaleEntriesFromWeakGCMaps): Added a new GC phase for pruning
stale entries from WeakGCMaps. This is only executed during full collections.

• heap/Heap.h:
• heap/HeapInlines.h:

(JSC::Heap::registerWeakGCMap):
(JSC::Heap::unregisterWeakGCMap): Added a mechanism for WeakGCMaps to register
themselves with the Heap and provide a pruning callback.

• runtime/PrototypeMap.h:

(JSC::PrototypeMap::PrototypeMap):

• runtime/Structure.cpp:

(JSC::StructureTransitionTable::add): Pass VM to WeakGCMap constructor.

• runtime/JSCInlines.h: Add "WeakGCMapInlines.h"

• runtime/JSGlobalObject.cpp: Include "WeakGCMapInlines.h" so this builds.

• runtime/JSString.cpp:

(JSC::jsStringWithCacheSlowCase):

• runtime/PrototypeMap.cpp:

(JSC::PrototypeMap::addPrototype):
(JSC::PrototypeMap::emptyObjectStructureForPrototype): Remove HashMap add()
optimization since it's not safe in the GC-managed WeakGCMap world.

• runtime/VM.cpp:

(JSC::VM::VM): Pass VM to WeakGCMap constructor.

• runtime/WeakGCMap.h:

(JSC::WeakGCMap::set):
(JSC::WeakGCMap::add):
(JSC::WeakGCMap::WeakGCMap): Deleted.
(JSC::WeakGCMap::gcMap): Deleted.
(JSC::WeakGCMap::gcMapIfNeeded): Deleted.

• runtime/WeakGCMapInlines.h: Added.

(JSC::WeakGCMap::WeakGCMap):
(JSC::WeakGCMap::~WeakGCMap):
(JSC::WeakGCMap::pruneStaleEntries): Moved ctor, dtor and pruning callback
to WeakGCMapInlines.h to fix interdependent header issues. Removed code that
prunes WeakGCMap at certain growth milestones and instead rely on the GC
callback for housekeeping.

File:

• trunk/Source/JavaScriptCore/runtime/WeakGCMap.h (modified) (4 diffs)

trunk/Source/JavaScriptCore/runtime/WeakGCMap.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2009, 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -48 +48 @@
     typedef typename HashMapType::const_iterator const_iterator;
 
-    WeakGCMap()
-        : m_gcThreshold(minGCThreshold)
-    {
-    }
+    explicit WeakGCMap(VM&);
+    ~WeakGCMap();
 
     ValueArg* get(const KeyType& key) const
@@ -60 +58 @@
     AddResult set(const KeyType& key, ValueType value)
     {
-        gcMapIfNeeded();
         return m_map.set(key, WTF::move(value));
-    }
-
-    ALWAYS_INLINE AddResult add(const KeyType& key, ValueType value)
-    {
-        gcMapIfNeeded();
-        AddResult addResult = m_map.fastAdd(key, nullptr);
-        if (!addResult.iterator->value) { // New value or found a zombie value.
-            addResult.isNewEntry = true;
-            addResult.iterator->value = WTF::move(value);
-        }
-        return addResult;
     }
 
@@ -104 +90 @@
     }
 
+    void pruneStaleEntries();
+
 private:
-    static const int minGCThreshold = 3;
-
-    NEVER_INLINE void gcMap()
-    {
-        Vector<KeyType, 4> zombies;
-
-        for (iterator it = m_map.begin(), end = m_map.end(); it != end; ++it) {
-            if (!it->value)
-                zombies.append(it->key);
-        }
-
-        for (size_t i = 0; i < zombies.size(); ++i)
-            m_map.remove(zombies[i]);
-    }
-
-    void gcMapIfNeeded()
-    {
-        if (m_map.size() < m_gcThreshold)
-            return;
-
-        gcMap();
-        m_gcThreshold = std::max(minGCThreshold, m_map.size() * 2 - 1);
-    }
-
     HashMapType m_map;
-    int m_gcThreshold;
+    VM& m_vm;
 };
-
-template<typename KeyArg, typename RawMappedArg, typename HashArg, typename KeyTraitsArg>
-const int WeakGCMap<KeyArg, RawMappedArg, HashArg, KeyTraitsArg>::minGCThreshold;
 
 } // namespace JSC