Title: [226655] trunk/Source/_javascript_Core
- Revision
- 226655
- Author
- [email protected]
- Date
- 2018-01-09 13:13:35 -0800 (Tue, 09 Jan 2018)
Log Message
Reduce graph size by replacing terminal nodes in blocks that have a ForceOSRExit with Unreachable
https://bugs.webkit.org/show_bug.cgi?id=181409
Reviewed by Keith Miller.
When I was looking at profiler data for Speedometer, I noticed that one of
the hottest functions in Speedometer is around 1100 bytecode operations long.
Only about 100 of those bytecode ops ever execute. However, we ended up
spending a lot of time compiling basic blocks that never executed. We often
plant ForceOSRExit nodes when we parse bytecodes that have a null value profile.
This is the case when such a node never executes.
This patch makes it so that anytime a block has a ForceOSRExit, we replace its
terminal node with an Unreachable node (and remove all nodes after the
ForceOSRExit). This will cut down on graph size when such a block dominates
other blocks in the CFG. This allows us to get rid of huge chunks of the CFG
in certain programs. When doing this transformation, we also insert
Flushes/PhantomLocals to ensure we can recover values that are bytecode
live-in to the ForceOSRExit.
Using ForceOSRExit as the signal for this is a bit of a hack. It definitely
does not get rid of all the CFG that it could. If we decide it's worth
it, we could use additional inputs into this mechanism. For example, we could
profile if a basic block ever executes inside the LLInt/Baseline, and
remove parts of the CFG based on that.
When running Speedometer with the concurrent JIT turned off, this patch
improves DFG/FTL compile times by around 5%.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::addToGraph):
(JSC::DFG::ByteCodeParser::parse):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (226654 => 226655)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-09 20:15:19 UTC (rev 226654)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-09 21:13:35 UTC (rev 226655)
@@ -1,3 +1,38 @@
+2018-01-09 Saam Barati <[email protected]>
+
+ Reduce graph size by replacing terminal nodes in blocks that have a ForceOSRExit with Unreachable
+ https://bugs.webkit.org/show_bug.cgi?id=181409
+
+ Reviewed by Keith Miller.
+
+ When I was looking at profiler data for Speedometer, I noticed that one of
+ the hottest functions in Speedometer is around 1100 bytecode operations long.
+ Only about 100 of those bytecode ops ever execute. However, we ended up
+ spending a lot of time compiling basic blocks that never executed. We often
+ plant ForceOSRExit nodes when we parse bytecodes that have a null value profile.
+ This is the case when such a node never executes.
+
+ This patch makes it so that anytime a block has a ForceOSRExit, we replace its
+ terminal node with an Unreachable node (and remove all nodes after the
+ ForceOSRExit). This will cut down on graph size when such a block dominates
+ other blocks in the CFG. This allows us to get rid of huge chunks of the CFG
+ in certain programs. When doing this transformation, we also insert
+ Flushes/PhantomLocals to ensure we can recover values that are bytecode
+ live-in to the ForceOSRExit.
+
+ Using ForceOSRExit as the signal for this is a bit of a hack. It definitely
+ does not get rid of all the CFG that it could. If we decide it's worth
+ it, we could use additional inputs into this mechanism. For example, we could
+ profile if a basic block ever executes inside the LLInt/Baseline, and
+ remove parts of the CFG based on that.
+
+ When running Speedometer with the concurrent JIT turned off, this patch
+ improves DFG/FTL compile times by around 5%.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::addToGraph):
+ (JSC::DFG::ByteCodeParser::parse):
+
2018-01-09 Mark Lam <[email protected]>
ASSERTION FAILED: pair.second->m_type & PropertyNode::Getter
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (226654 => 226655)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-09 20:15:19 UTC (rev 226654)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-09 21:13:35 UTC (rev 226655)
@@ -694,7 +694,10 @@
Node* addToGraph(Node* node)
{
+ m_hasAnyForceOSRExits |= (node->op() == ForceOSRExit);
+
VERBOSE_LOG(" appended ", node, " ", Graph::opName(node->op()), "\n");
+
m_currentBlock->append(node);
if (clobbersExitState(m_graph, node))
m_exitOK = false;
@@ -1141,6 +1144,7 @@
Instruction* m_currentInstruction;
bool m_hasDebuggerEnabled;
+ bool m_hasAnyForceOSRExits { false };
};
BasicBlock* ByteCodeParser::allocateTargetableBlock(unsigned bytecodeIndex)
@@ -6559,6 +6563,67 @@
parseCodeBlock();
linkBlocks(inlineStackEntry.m_unlinkedBlocks, inlineStackEntry.m_blockLinkingTargets);
+ if (m_hasAnyForceOSRExits) {
+ InsertionSet insertionSet(m_graph);
+ Operands<VariableAccessData*> mapping(OperandsLike, m_graph.block(0)->variablesAtHead);
+
+ for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
+ mapping.fill(nullptr);
+ if (validationEnabled()) {
+ // Verify that it's correct to fill mapping with nullptr.
+ for (unsigned i = 0; i < block->variablesAtHead.size(); ++i) {
+ Node* node = block->variablesAtHead.at(i);
+ RELEASE_ASSERT(!node);
+ }
+ }
+
+ for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
+ Node* node = block->at(nodeIndex);
+
+ if (node->hasVariableAccessData(m_graph))
+ mapping.operand(node->local()) = node->variableAccessData();
+
+ if (node->op() == ForceOSRExit) {
+ NodeOrigin endOrigin = node->origin.withExitOK(true);
+
+ if (validationEnabled()) {
+ // This verifies that we don't need to change any of the successors's predecessor
+ // list after planting the Unreachable below. At this point in the bytecode
+ // parser, we haven't linked up the predecessor lists yet.
+ for (BasicBlock* successor : block->successors())
+ RELEASE_ASSERT(successor->predecessors.isEmpty());
+ }
+
+ block->resize(nodeIndex + 1);
+
+ insertionSet.insertNode(block->size(), SpecNone, ExitOK, endOrigin);
+ m_graph.forAllLiveInBytecode(endOrigin.semantic, [&] (VirtualRegister operand) {
+ VariableAccessData* variable = mapping.operand(operand);
+ if (!variable)
+ variable = newVariableAccessData(operand);
+
+ auto op = PhantomLocal;
+ if ((m_graph.needsScopeRegister() && operand == m_codeBlock->scopeRegister())
+ || (operand.isArgument() && (operand != virtualRegisterForArgument(0) || m_graph.needsFlushedThis()))) {
+ op = Flush;
+ }
+ insertionSet.insertNode(block->size(), SpecNone, op, endOrigin, OpInfo(variable));
+ });
+
+ insertionSet.insertNode(block->size(), SpecNone, Unreachable, endOrigin);
+ insertionSet.execute(block);
+ break;
+ }
+ }
+ }
+ } else if (validationEnabled()) {
+ // Ensure our bookkeeping for ForceOSRExit nodes is working.
+ for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
+ for (Node* node : *block)
+ RELEASE_ASSERT(node->op() != ForceOSRExit);
+ }
+ }
+
m_graph.determineReachability();
m_graph.killUnreachableBlocks();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
