[webkit-changes] [204355] trunk/Source/JavaScriptCore

Wed, 10 Aug 2016 14:25:59 -0700

Title: [204355] trunk/Source/_javascript_Core
Revision
204355
Author
benja...@webkit.org
Date
2016-08-10 14:25:28 -0700 (Wed, 10 Aug 2016)

Log Message

[JSC] Speed up SparseCollection & related maps
https://bugs.webkit.org/show_bug.cgi?id=160733

Patch by Benjamin Poulain <bpoul...@apple.com> on 2016-08-10
Reviewed by Saam Barati.

On MBA, Graph::addNode() shows up in profiles due to SparseCollection::add().
This is unfortunate.

The first improvement is to build the new unique_ptr in the empty slot
instead of moving a new value into it.

Previously, the code would load the previous value, test if it is null
then invoke the destructor and finally fastFree(). The initial test
obviously fails so that's a whole bunch of code that is never executed.

With the new code, we just have a store.

I also removed the bounds checking on our maps based on node index.
Those bounds checks are never eliminated by clang because the index
is always loaded from memory instead of being computed.
There are unfortunately too many nodes processed and the bounds checks
get costly.

* b3/B3SparseCollection.h:
(JSC::B3::SparseCollection::add):
* dfg/DFGGraph.h:
(JSC::DFG::Graph::abstractValuesCache):
* dfg/DFGInPlaceAbstractState.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (204354 => 204355)


--- trunk/Source/_javascript_Core/ChangeLog	2016-08-10 21:23:20 UTC (rev 204354)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-08-10 21:25:28 UTC (rev 204355)
@@ -1,5 +1,36 @@
 2016-08-10  Benjamin Poulain  <bpoul...@apple.com>
 
+        [JSC] Speed up SparseCollection & related maps
+        https://bugs.webkit.org/show_bug.cgi?id=160733
+
+        Reviewed by Saam Barati.
+
+        On MBA, Graph::addNode() shows up in profiles due to SparseCollection::add().
+        This is unfortunate.
+
+        The first improvement is to build the new unique_ptr in the empty slot
+        instead of moving a new value into it.
+
+        Previously, the code would load the previous value, test if it is null
+        then invoke the destructor and finally fastFree(). The initial test
+        obviously fails so that's a whole bunch of code that is never executed.
+
+        With the new code, we just have a store.
+
+        I also removed the bounds checking on our maps based on node index.
+        Those bounds checks are never eliminated by clang because the index
+        is always loaded from memory instead of being computed.
+        There are unfortunately too many nodes processed and the bounds checks
+        get costly.
+
+        * b3/B3SparseCollection.h:
+        (JSC::B3::SparseCollection::add):
+        * dfg/DFGGraph.h:
+        (JSC::DFG::Graph::abstractValuesCache):
+        * dfg/DFGInPlaceAbstractState.h:
+
+2016-08-10  Benjamin Poulain  <bpoul...@apple.com>
+
         [JSC] Remove some useless code I left when rewriting CSE's large maps
         https://bugs.webkit.org/show_bug.cgi?id=160720

Modified: trunk/Source/_javascript_Core/b3/B3SparseCollection.h (204354 => 204355)


--- trunk/Source/_javascript_Core/b3/B3SparseCollection.h	2016-08-10 21:23:20 UTC (rev 204354)
+++ trunk/Source/_javascript_Core/b3/B3SparseCollection.h	2016-08-10 21:25:28 UTC (rev 204355)
@@ -55,9 +55,9 @@
             index = m_indexFreeList.takeLast();
 
         value->m_index = index;
+        ASSERT(!m_vector[index]);
+        new (NotNull, &m_vector[index]) std::unique_ptr<T>(WTFMove(value));
 
-        m_vector[index] = WTFMove(value);
-
         return result;
     }
 
@@ -168,8 +168,8 @@
     iterator end() const { return iterator(*this, size()); }
 
 private:
-    Vector<std::unique_ptr<T>> m_vector;
-    Vector<size_t> m_indexFreeList;
+    Vector<std::unique_ptr<T>, 0, UnsafeVectorOverflow> m_vector;
+    Vector<size_t, 0, UnsafeVectorOverflow> m_indexFreeList;
 };
 
 } } // namespace JSC::B3

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (204354 => 204355)


--- trunk/Source/_javascript_Core/dfg/DFGGraph.h	2016-08-10 21:23:20 UTC (rev 204354)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h	2016-08-10 21:25:28 UTC (rev 204355)
@@ -199,7 +199,7 @@
     Node* nodeAt(unsigned index) const { return m_nodes[index]; }
     void packNodeIndices();
 
-    Vector<AbstractValue>& abstractValuesCache() { return m_abstractValuesCache; }
+    Vector<AbstractValue, 0, UnsafeVectorOverflow>& abstractValuesCache() { return m_abstractValuesCache; }
 
     void dethread();
     
@@ -957,7 +957,7 @@
     }
 
     B3::SparseCollection<Node> m_nodes;
-    Vector<AbstractValue> m_abstractValuesCache;
+    Vector<AbstractValue, 0, UnsafeVectorOverflow> m_abstractValuesCache;
 };
 
 } } // namespace JSC::DFG

Modified: trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.h (204354 => 204355)


--- trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.h	2016-08-10 21:23:20 UTC (rev 204354)
+++ trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.h	2016-08-10 21:25:28 UTC (rev 204355)
@@ -133,7 +133,7 @@
     
     Graph& m_graph;
 
-    Vector<AbstractValue>& m_abstractValues;
+    Vector<AbstractValue, 0, UnsafeVectorOverflow>& m_abstractValues;
     Operands<AbstractValue> m_variables;
     BasicBlock* m_block;
     

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to