[JSC] Remove easy toRemove & map.remove() use in OAS phase
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Dec 2017 01:02:47 +0000 (01:02 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Dec 2017 01:02:47 +0000 (01:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180208

Reviewed by Mark Lam.

Source/JavaScriptCore:

In this patch, we replace Vector<> toRemove & map.remove loop with removeIf,
to optimize this common pattern. This patch only modifies apparent ones.
But we can apply this refactoring further to OAS phase in the future.

One thing we should care is that predicate of removeIf should not touch the
removing set itself. In this patch, we apply this change to (1) apparently
correct one and (2) things in DFG OAS phase since it is very slow.

* b3/B3MoveConstants.cpp:
* dfg/DFGObjectAllocationSinkingPhase.cpp:

Source/WTF:

* wtf/HashMap.h:
(WTF::X>::removeIf):
* wtf/HashSet.h:
(WTF::V>::removeIf):
* wtf/HashTable.h:
(WTF::KeyTraits>::removeIf):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225375 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3MoveConstants.cpp
Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/HashMap.h
Source/WTF/wtf/HashSet.h
Source/WTF/wtf/HashTable.h

index 2f94f1e..5505670 100644 (file)
@@ -1,3 +1,21 @@
+2017-11-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Remove easy toRemove & map.remove() use in OAS phase
+        https://bugs.webkit.org/show_bug.cgi?id=180208
+
+        Reviewed by Mark Lam.
+
+        In this patch, we replace Vector<> toRemove & map.remove loop with removeIf,
+        to optimize this common pattern. This patch only modifies apparent ones.
+        But we can apply this refactoring further to OAS phase in the future.
+
+        One thing we should care is that predicate of removeIf should not touch the
+        removing set itself. In this patch, we apply this change to (1) apparently
+        correct one and (2) things in DFG OAS phase since it is very slow.
+
+        * b3/B3MoveConstants.cpp:
+        * dfg/DFGObjectAllocationSinkingPhase.cpp:
+
 2017-11-30  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r225362.
index d3ed979..c98172b 100644 (file)
@@ -342,10 +342,8 @@ private:
     }
 
     Procedure& m_proc;
-    Vector<Value*> m_toRemove;
     HashMap<ValueKey, unsigned> m_constTable;
     int64_t* m_dataSection;
-    HashMap<ValueKey, Value*> m_constants;
     InsertionSet m_insertionSet;
 };
 
index 3fda7d4..8171b6e 100644 (file)
@@ -507,14 +507,10 @@ public:
 
     void pruneByLiveness(const NodeSet& live)
     {
-        Vector<Node*> toRemove;
-        for (const auto& entry : m_pointers) {
-            if (!live.contains(entry.key))
-                toRemove.append(entry.key);
-        }
-        for (Node* node : toRemove)
-            m_pointers.remove(node);
-
+        m_pointers.removeIf(
+            [&] (const auto& entry) {
+                return !live.contains(entry.key);
+            });
         prune();
     }
 
@@ -682,15 +678,10 @@ private:
         }
 
         // Remove unreachable allocations
-        {
-            Vector<Node*> toRemove;
-            for (const auto& entry : m_allocations) {
-                if (!reachable.contains(entry.key))
-                    toRemove.append(entry.key);
-            }
-            for (Node* identifier : toRemove)
-                m_allocations.remove(identifier);
-        }
+        m_allocations.removeIf(
+            [&] (const auto& entry) {
+                return !reachable.contains(entry.key);
+            });
     }
 
     bool m_reached = false;
@@ -1249,14 +1240,10 @@ private:
     {
         // We don't create materializations if the escapee is not a
         // sink candidate
-        Vector<Node*> toRemove;
-        for (const auto& entry : escapees) {
-            if (!m_sinkCandidates.contains(entry.key))
-                toRemove.append(entry.key);
-        }
-        for (Node* identifier : toRemove)
-            escapees.remove(identifier);
-
+        escapees.removeIf(
+            [&] (const auto& entry) {
+                return !m_sinkCandidates.contains(entry.key);
+            });
         if (escapees.isEmpty())
             return;
 
index bc4e55e..8864bc1 100644 (file)
@@ -1,3 +1,17 @@
+2017-11-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Remove easy toRemove & map.remove() use in OAS phase
+        https://bugs.webkit.org/show_bug.cgi?id=180208
+
+        Reviewed by Mark Lam.
+
+        * wtf/HashMap.h:
+        (WTF::X>::removeIf):
+        * wtf/HashSet.h:
+        (WTF::V>::removeIf):
+        * wtf/HashTable.h:
+        (WTF::KeyTraits>::removeIf):
+
 2017-11-30  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r225362.
index e4cf6ba..34e72ff 100644 (file)
@@ -135,7 +135,7 @@ public:
     bool remove(const KeyType&);
     bool remove(iterator);
     template<typename Functor>
-    void removeIf(Functor&&);
+    bool removeIf(Functor&&);
     void clear();
 
     MappedTakeType take(const KeyType&); // efficient combination of get with remove
@@ -443,9 +443,9 @@ inline bool HashMap<T, U, V, W, X>::remove(iterator it)
 
 template<typename T, typename U, typename V, typename W, typename X>
 template<typename Functor>
-inline void HashMap<T, U, V, W, X>::removeIf(Functor&& functor)
+inline bool HashMap<T, U, V, W, X>::removeIf(Functor&& functor)
 {
-    m_impl.removeIf(std::forward<Functor>(functor));
+    return m_impl.removeIf(std::forward<Functor>(functor));
 }
 
 template<typename T, typename U, typename V, typename W, typename X>
index e62b3d9..fc45835 100644 (file)
@@ -105,7 +105,7 @@ public:
     bool remove(const ValueType&);
     bool remove(iterator);
     template<typename Functor>
-    void removeIf(const Functor&);
+    bool removeIf(const Functor&);
     void clear();
 
     TakeType take(const ValueType&);
@@ -275,9 +275,9 @@ inline bool HashSet<T, U, V>::remove(const ValueType& value)
 
 template<typename T, typename U, typename V>
 template<typename Functor>
-inline void HashSet<T, U, V>::removeIf(const Functor& functor)
+inline bool HashSet<T, U, V>::removeIf(const Functor& functor)
 {
-    m_impl.removeIf(functor);
+    return m_impl.removeIf(functor);
 }
 
 template<typename T, typename U, typename V>
index 7f4fa9f..d1eebd6 100644 (file)
@@ -405,7 +405,7 @@ namespace WTF {
         void removeWithoutEntryConsistencyCheck(iterator);
         void removeWithoutEntryConsistencyCheck(const_iterator);
         template<typename Functor>
-        void removeIf(const Functor&);
+        bool removeIf(const Functor&);
         void clear();
 
         static bool isEmptyBucket(const ValueType& value) { return isHashTraitsEmptyValue<KeyTraits>(Extractor::extract(value)); }
@@ -1108,7 +1108,7 @@ namespace WTF {
 
     template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>
     template<typename Functor>
-    inline void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::removeIf(const Functor& functor)
+    inline bool HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::removeIf(const Functor& functor)
     {
         // We must use local copies in case "functor" or "deleteBucket"
         // make a function call, which prevents the compiler from keeping
@@ -1134,6 +1134,7 @@ namespace WTF {
             shrink();
         
         internalCheckTableConsistency();
+        return removedBucketCount;
     }
 
     template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>