Changeset 204112 in webkit for trunk/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp

Timestamp:

Aug 3, 2016, 8:43:51 PM (9 years ago)

Author:

[email protected]

Message:

[JSC] Improve the memory locality of DFG Node's AbstractValues
​https://bugs.webkit.org/show_bug.cgi?id=160443

Patch by Benjamin Poulain <​[email protected]> on 2016-08-03
Reviewed by Mark Lam.

The AbstractInterpreter spends a lot of time on memory operations
for AbstractValues. This patch attempts to improve the situation
by putting the values closer together in memory.

First, AbstractValue is moved out of DFG::Node and it kept in
a vector addressed by node indices.

I initially moved them to InPlaceAbstractState but I quickly discovered
initializing the values in the vector was costly.
I moved the vector to Graph as a cache shared by every instantiation of
InPlaceAbstractState. It is mainly there to avoid constructors and destructors
of AbstractValue. The patch of ​https://bugs.webkit.org/show_bug.cgi?id=160370
should also help eventually.

I instrumented CFA to find how packed is SparseCollection.
The answer is it can be very sparse, which is bad for CFA.
I added packIndices() to repack the collection before running
liveness since that's where we start using the memory intensively.
This is a measurable improvement but it implies we can no longer
keep indices on a side channel between phases since they may change.

• b3/B3SparseCollection.h:

(JSC::B3::SparseCollection::packIndices):

• dfg/DFGGraph.cpp:

(JSC::DFG::Graph::packNodeIndices):

• dfg/DFGGraph.h:

(JSC::DFG::Graph::abstractValuesCache):

• dfg/DFGInPlaceAbstractState.cpp:

(JSC::DFG::InPlaceAbstractState::InPlaceAbstractState):

• dfg/DFGInPlaceAbstractState.h:

(JSC::DFG::InPlaceAbstractState::forNode):

• dfg/DFGLivenessAnalysisPhase.cpp:

(JSC::DFG::performLivenessAnalysis):

• dfg/DFGNode.h:

File:

• trunk/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp

@@ -42 +42 @@
 InPlaceAbstractState::InPlaceAbstractState(Graph& graph)
     : m_graph(graph)
+    , m_abstractValues(graph.abstractValuesCache())
     , m_variables(m_graph.m_codeBlock->numParameters(), graph.m_localVars)
     , m_block(0)
@@ -56 +57 @@
     ASSERT(basicBlock->variablesAtTail.numberOfLocals() == basicBlock->valuesAtTail.numberOfLocals());
     ASSERT(basicBlock->variablesAtHead.numberOfLocals() == basicBlock->variablesAtTail.numberOfLocals());
+
+    // Certain phases insert nodes in a block after running through it.
+    // We cannot reserve the space for AbstractValues when initializing AbstractState because the number of values
+    // can increase as we execute. Instead, we increase the size as needed before processing each block.
+    m_abstractValues.resize(m_graph.maxNodeCount());
     
     for (size_t i = 0; i < basicBlock->size(); i++)