Title: [225375] trunk/Source
Revision
225375
Author
utatane....@gmail.com
Date
2017-11-30 17:02:47 -0800 (Thu, 30 Nov 2017)

Log Message

[JSC] Remove easy toRemove & map.remove() use in OAS phase
https://bugs.webkit.org/show_bug.cgi?id=180208

Reviewed by Mark Lam.

Source/_javascript_Core:

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):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (225374 => 225375)


--- trunk/Source/_javascript_Core/ChangeLog	2017-12-01 00:57:13 UTC (rev 225374)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-12-01 01:02:47 UTC (rev 225375)
@@ -1,3 +1,21 @@
+2017-11-30  Yusuke Suzuki  <utatane....@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-qu...@webkit.org>
 
         Unreviewed, rolling out r225362.

Modified: trunk/Source/_javascript_Core/b3/B3MoveConstants.cpp (225374 => 225375)


--- trunk/Source/_javascript_Core/b3/B3MoveConstants.cpp	2017-12-01 00:57:13 UTC (rev 225374)
+++ trunk/Source/_javascript_Core/b3/B3MoveConstants.cpp	2017-12-01 01:02:47 UTC (rev 225375)
@@ -342,10 +342,8 @@
     }
 
     Procedure& m_proc;
-    Vector<Value*> m_toRemove;
     HashMap<ValueKey, unsigned> m_constTable;
     int64_t* m_dataSection;
-    HashMap<ValueKey, Value*> m_constants;
     InsertionSet m_insertionSet;
 };
 

Modified: trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (225374 => 225375)


--- trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp	2017-12-01 00:57:13 UTC (rev 225374)
+++ trunk/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp	2017-12-01 01:02:47 UTC (rev 225375)
@@ -507,14 +507,10 @@
 
     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 @@
         }
 
         // 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 @@
     {
         // 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;
 

Modified: trunk/Source/WTF/ChangeLog (225374 => 225375)


--- trunk/Source/WTF/ChangeLog	2017-12-01 00:57:13 UTC (rev 225374)
+++ trunk/Source/WTF/ChangeLog	2017-12-01 01:02:47 UTC (rev 225375)
@@ -1,3 +1,17 @@
+2017-11-30  Yusuke Suzuki  <utatane....@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-qu...@webkit.org>
 
         Unreviewed, rolling out r225362.

Modified: trunk/Source/WTF/wtf/HashMap.h (225374 => 225375)


--- trunk/Source/WTF/wtf/HashMap.h	2017-12-01 00:57:13 UTC (rev 225374)
+++ trunk/Source/WTF/wtf/HashMap.h	2017-12-01 01:02:47 UTC (rev 225375)
@@ -135,7 +135,7 @@
     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 @@
 
 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>

Modified: trunk/Source/WTF/wtf/HashSet.h (225374 => 225375)


--- trunk/Source/WTF/wtf/HashSet.h	2017-12-01 00:57:13 UTC (rev 225374)
+++ trunk/Source/WTF/wtf/HashSet.h	2017-12-01 01:02:47 UTC (rev 225375)
@@ -105,7 +105,7 @@
     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 @@
 
 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>

Modified: trunk/Source/WTF/wtf/HashTable.h (225374 => 225375)


--- trunk/Source/WTF/wtf/HashTable.h	2017-12-01 00:57:13 UTC (rev 225374)
+++ trunk/Source/WTF/wtf/HashTable.h	2017-12-01 01:02:47 UTC (rev 225375)
@@ -405,7 +405,7 @@
         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 @@
 
     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 @@
             shrink();
         
         internalCheckTableConsistency();
+        return removedBucketCount;
     }
 
     template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to