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

Sun, 23 Feb 2014 10:44:05 -0800

Title: [164558] trunk/Source/_javascript_Core
Revision
164558
Author
[email protected]
Date
2014-02-23 10:48:43 -0800 (Sun, 23 Feb 2014)

Log Message

Refine DFG+FTL inlining and compilation limits
https://bugs.webkit.org/show_bug.cgi?id=129212

Reviewed by Mark Hahnenberg.
        
Allow larger functions to be DFG-compiled. Institute a limit on FTL compilation,
and set that limit quite high. Institute a limit on inlining-into. The idea here is
that large functions tend to be autogenerated, and code generators like emscripten
appear to leave few inlining opportunities anyway. Also, we don't want the code
size explosion that we would risk if we allowed compilation of a large function and
then inlined a ton of stuff into it.
        
This is a 0.5% speed-up on Octane v2 and almost eliminates the typescript
regression. This is a 9% speed-up on AsmBench.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::noticeIncomingCall):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleInlining):
* dfg/DFGCapabilities.h:
(JSC::DFG::isSmallEnoughToInlineCodeInto):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLState.h:
(JSC::FTL::shouldShowDisassembly):
* runtime/Options.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (164557 => 164558)


--- trunk/Source/_javascript_Core/ChangeLog	2014-02-23 18:06:16 UTC (rev 164557)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-02-23 18:48:43 UTC (rev 164558)
@@ -1,3 +1,32 @@
+2014-02-22  Filip Pizlo  <[email protected]>
+
+        Refine DFG+FTL inlining and compilation limits
+        https://bugs.webkit.org/show_bug.cgi?id=129212
+
+        Reviewed by Mark Hahnenberg.
+        
+        Allow larger functions to be DFG-compiled. Institute a limit on FTL compilation,
+        and set that limit quite high. Institute a limit on inlining-into. The idea here is
+        that large functions tend to be autogenerated, and code generators like emscripten
+        appear to leave few inlining opportunities anyway. Also, we don't want the code
+        size explosion that we would risk if we allowed compilation of a large function and
+        then inlined a ton of stuff into it.
+        
+        This is a 0.5% speed-up on Octane v2 and almost eliminates the typescript
+        regression. This is a 9% speed-up on AsmBench.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::noticeIncomingCall):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleInlining):
+        * dfg/DFGCapabilities.h:
+        (JSC::DFG::isSmallEnoughToInlineCodeInto):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLState.h:
+        (JSC::FTL::shouldShowDisassembly):
+        * runtime/Options.h:
+
 2014-02-22  Dan Bernstein  <[email protected]>
 
         REGRESSION (r164507): Crash beneath JSGlobalObjectInspectorController::reportAPIException at facebook.com, twitter.com, youtube.com

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (164557 => 164558)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2014-02-23 18:06:16 UTC (rev 164557)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2014-02-23 18:48:43 UTC (rev 164558)
@@ -2787,6 +2787,13 @@
 
     if (!canInline(m_capabilityLevelState))
         return;
+    
+    if (!DFG::isSmallEnoughToInlineCodeInto(callerCodeBlock)) {
+        m_shouldAlwaysBeInlined = false;
+        if (Options::verboseCallLink())
+            dataLog("    Clearing SABI because caller is too large.\n");
+        return;
+    }
 
     if (callerCodeBlock->jitType() == JITCode::InterpreterThunk) {
         // If the caller is still in the interpreter, then we can't expect inlining to
@@ -2795,7 +2802,7 @@
         // any of its callers.
         m_shouldAlwaysBeInlined = false;
         if (Options::verboseCallLink())
-            dataLog("    Marking SABI because caller is in LLInt.\n");
+            dataLog("    Clearing SABI because caller is in LLInt.\n");
         return;
     }
     
@@ -2805,7 +2812,7 @@
         // delay eval optimization by a *lot*.
         m_shouldAlwaysBeInlined = false;
         if (Options::verboseCallLink())
-            dataLog("    Marking SABI because caller is not a function.\n");
+            dataLog("    Clearing SABI because caller is not a function.\n");
         return;
     }
     
@@ -2816,7 +2823,7 @@
         if (frame->codeBlock() == this) {
             // Recursive calls won't be inlined.
             if (Options::verboseCallLink())
-                dataLog("    Marking SABI because recursion was detected.\n");
+                dataLog("    Clearing SABI because recursion was detected.\n");
             m_shouldAlwaysBeInlined = false;
             return;
         }
@@ -2828,7 +2835,7 @@
         return;
     
     if (Options::verboseCallLink())
-        dataLog("    Marking SABI because the caller is not a DFG candidate.\n");
+        dataLog("    Clearing SABI because the caller is not a DFG candidate.\n");
     
     m_shouldAlwaysBeInlined = false;
 #endif

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (164557 => 164558)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2014-02-23 18:06:16 UTC (rev 164557)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2014-02-23 18:48:43 UTC (rev 164558)
@@ -1316,6 +1316,16 @@
         return false;
     }
     
+    // Check if the caller is already too large. We do this check here because that's just
+    // where we happen to also have the callee's code block, and we want that for the
+    // purpose of unsetting SABI.
+    if (!isSmallEnoughToInlineCodeInto(m_codeBlock)) {
+        codeBlock->m_shouldAlwaysBeInlined = false;
+        if (verbose)
+            dataLog("    Failing because the caller is too large.\n");
+        return false;
+    }
+    
     // FIXME: this should be better at predicting how much bloat we will introduce by inlining
     // this function.
     // https://bugs.webkit.org/show_bug.cgi?id=127627

Modified: trunk/Source/_javascript_Core/dfg/DFGCapabilities.h (164557 => 164558)


--- trunk/Source/_javascript_Core/dfg/DFGCapabilities.h	2014-02-23 18:06:16 UTC (rev 164557)
+++ trunk/Source/_javascript_Core/dfg/DFGCapabilities.h	2014-02-23 18:48:43 UTC (rev 164558)
@@ -144,6 +144,11 @@
     return inlineFunctionForConstructCapabilityLevel(codeBlock);
 }
 
+inline bool isSmallEnoughToInlineCodeInto(CodeBlock* codeBlock)
+{
+    return codeBlock->instructionCount() <= Options::maximumInliningCallerSize();
+}
+
 } } // namespace JSC::DFG
 
 #endif // DFGCapabilities_h

Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (164557 => 164558)


--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2014-02-23 18:06:16 UTC (rev 164557)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2014-02-23 18:48:43 UTC (rev 164558)
@@ -278,6 +278,12 @@
 
 CapabilityLevel canCompile(Graph& graph)
 {
+    if (graph.m_codeBlock->instructionCount() > Options::maximumFTLCandidateInstructionCount()) {
+        if (verboseCapabilities())
+            dataLog("FTL rejecting ", *graph.m_codeBlock, " because it's too big.\n");
+        return CannotCompile;
+    }
+    
     if (graph.m_codeBlock->codeType() != FunctionCode) {
         if (verboseCapabilities())
             dataLog("FTL rejecting ", *graph.m_codeBlock, " because it doesn't belong to a function.\n");

Modified: trunk/Source/_javascript_Core/ftl/FTLState.h (164557 => 164558)


--- trunk/Source/_javascript_Core/ftl/FTLState.h	2014-02-23 18:06:16 UTC (rev 164557)
+++ trunk/Source/_javascript_Core/ftl/FTLState.h	2014-02-23 18:48:43 UTC (rev 164558)
@@ -46,7 +46,7 @@
     return DFG::verboseCompilationEnabled(DFG::FTLMode);
 }
 
-inline bool showDisassembly()
+inline bool shouldShowDisassembly()
 {
     return DFG::shouldShowDisassembly(DFG::FTLMode);
 }

Modified: trunk/Source/_javascript_Core/runtime/Options.h (164557 => 164558)


--- trunk/Source/_javascript_Core/runtime/Options.h	2014-02-23 18:06:16 UTC (rev 164557)
+++ trunk/Source/_javascript_Core/runtime/Options.h	2014-02-23 18:48:43 UTC (rev 164558)
@@ -170,18 +170,24 @@
     \
     v(bool, breakOnThrow, false) \
     \
-    v(unsigned, maximumOptimizationCandidateInstructionCount, 10000) \
+    v(unsigned, maximumOptimizationCandidateInstructionCount, 100000) \
     \
     v(unsigned, maximumFunctionForCallInlineCandidateInstructionCount, 180) \
     v(unsigned, maximumFunctionForClosureCallInlineCandidateInstructionCount, 100) \
     v(unsigned, maximumFunctionForConstructInlineCandidateInstructionCount, 100) \
     \
+    v(unsigned, maximumFTLCandidateInstructionCount, 20000) \
+    \
     /* Depth of inline stack, so 1 = no inlining, 2 = one level, etc. */ \
     v(unsigned, maximumInliningDepth, 5) \
     v(unsigned, maximumInliningRecursion, 2) \
     v(unsigned, maximumInliningDepthForMustInline, 7) \
     v(unsigned, maximumInliningRecursionForMustInline, 3) \
     \
+    /* Maximum size of a caller for enabling inlining. This is purely to protect us */\
+    /* from super long compiles that take a lot of memory. */\
+    v(unsigned, maximumInliningCallerSize, 10000) \
+    \
     v(bool, enablePolyvariantCallInlining, true) \
     v(bool, enablePolyvariantByIdInlining, true) \
     \

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to