Attachment 223211 Details for Bug 128234 – work in progress

[patch] work in progress

blah.patch (text/plain), 11.06 KB, created by Filip Pizlo on 2014-02-04 21:55:25 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2014-02-04 21:55:25 PST

Size: 11.06 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 163429)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,24 @@
>+2014-02-04  Filip Pizlo  <fpizlo@apple.com>
>+
>+        Make FTL OSR entry something we only try after we've already compiled the function with the FTL and it still got stuck in a loop after that without ever returning like a sensible function oughta have
>+        https://bugs.webkit.org/show_bug.cgi?id=128234
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Use DFG::JITCode::osrEntryRetry as a counter to decide when to invoke OSR entry. That
>+        comes into play only after we've done a replacement compile.
>+        
>+        This appears to still give us a speed-up on the kinds of things that OSR entry is good
>+        for, while also eliminating pointless OSR entry compilations on other things.
>+
>+        * dfg/DFGJITCode.cpp:
>+        (JSC::DFG::JITCode::JITCode):
>+        * dfg/DFGJITCode.h:
>+        * dfg/DFGOperations.cpp:
>+        * dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:
>+        (JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::compilationDidComplete):
>+        * runtime/Options.h:
>+
> 2014-02-04  Mark Lam  <mark.lam@apple.com>
> 
>         The stack limit computation does not work for Windows.
>Index: Source/JavaScriptCore/dfg/DFGJITCode.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGJITCode.cpp	(revision 163422)
>+++ Source/JavaScriptCore/dfg/DFGJITCode.cpp	(working copy)
>@@ -34,6 +34,10 @@ namespace JSC { namespace DFG {
> 
> JITCode::JITCode()
>     : DirectJITCode(DFGJIT)
>+#if ENABLE(FTL_JIT)
>+    , osrEntryRetry(0)
>+    , abandonOSREntry(false)
>+#endif // ENABLE(FTL_JIT)
> {
> }
> 
>Index: Source/JavaScriptCore/dfg/DFGJITCode.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGJITCode.h	(revision 163422)
>+++ Source/JavaScriptCore/dfg/DFGJITCode.h	(working copy)
>@@ -125,6 +125,8 @@ public:
> #if ENABLE(FTL_JIT)
>     ExecutionCounter tierUpCounter;
>     RefPtr<CodeBlock> osrEntryBlock;
>+    unsigned osrEntryRetry;
>+    bool abandonOSREntry;
> #endif // ENABLE(FTL_JIT)
> };
> 
>Index: Source/JavaScriptCore/dfg/DFGOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.cpp	(revision 163422)
>+++ Source/JavaScriptCore/dfg/DFGOperations.cpp	(working copy)
>@@ -1111,21 +1111,8 @@ extern "C" void JIT_OPERATION triggerReo
> }
> 
> #if ENABLE(FTL_JIT)
>-void JIT_OPERATION triggerTierUpNow(ExecState* exec)
>+static void triggerFTLReplacementCompile(VM* vm, CodeBlock* codeBlock, JITCode* jitCode)
> {
>-    VM* vm = &exec->vm();
>-    NativeCallFrameTracer tracer(vm, exec);
>-    DeferGC deferGC(vm->heap);
>-    CodeBlock* codeBlock = exec->codeBlock();
>-    
>-    JITCode* jitCode = codeBlock->jitCode()->dfg();
>-    
>-    if (Options::verboseOSR()) {
>-        dataLog(
>-            *codeBlock, ": Entered triggerTierUpNow with executeCounter = ",
>-            jitCode->tierUpCounter, "\n");
>-    }
>-    
>     if (codeBlock->baselineVersion()->m_didFailFTLCompilation) {
>         if (Options::verboseOSR())
>             dataLog("Deferring FTL-optimization of ", *codeBlock, " indefinitely because there was an FTL failure.\n");
>@@ -1173,6 +1160,24 @@ void JIT_OPERATION triggerTierUpNow(Exec
>         Operands<JSValue>(), ToFTLDeferredCompilationCallback::create(codeBlock));
> }
> 
>+void JIT_OPERATION triggerTierUpNow(ExecState* exec)
>+{
>+    VM* vm = &exec->vm();
>+    NativeCallFrameTracer tracer(vm, exec);
>+    DeferGC deferGC(vm->heap);
>+    CodeBlock* codeBlock = exec->codeBlock();
>+    
>+    JITCode* jitCode = codeBlock->jitCode()->dfg();
>+    
>+    if (Options::verboseOSR()) {
>+        dataLog(
>+            *codeBlock, ": Entered triggerTierUpNow with executeCounter = ",
>+            jitCode->tierUpCounter, "\n");
>+    }
>+    
>+    triggerFTLReplacementCompile(vm, codeBlock, jitCode);
>+}
>+
> char* JIT_OPERATION triggerOSREntryNow(
>     ExecState* exec, int32_t bytecodeIndex, int32_t streamIndex)
> {
>@@ -1189,57 +1194,47 @@ char* JIT_OPERATION triggerOSREntryNow(
>             jitCode->tierUpCounter, "\n");
>     }
>     
>-    if (codeBlock->baselineVersion()->m_didFailFTLCompilation) {
>-        if (Options::verboseOSR())
>-            dataLog("Deferring FTL-optimization of ", *codeBlock, " indefinitely because there was an FTL failure.\n");
>-        jitCode->dontOptimizeAnytimeSoon(codeBlock);
>+    // - If we don't have an FTL code block, then try to compile one.
>+    // - If we do have an FTL code block, then try to enter for a while.
>+    // - If we couldn't enter for a while, then trigger OSR entry.
>+    
>+    triggerFTLReplacementCompile(vm, codeBlock, jitCode);
>+
>+    if (!codeBlock->hasOptimizedReplacement())
>         return 0;
>-    }
>     
>-    if (!jitCode->checkIfOptimizationThresholdReached(codeBlock)) {
>-        if (Options::verboseOSR())
>-            dataLog("Choosing not to FTL-optimize ", *codeBlock, " yet.\n");
>+    if (jitCode->osrEntryRetry < Options::ftlOSREntryRetryThreshold()) {
>+        jitCode->osrEntryRetry++;
>         return 0;
>     }
>     
>-    Worklist* worklist = existingGlobalFTLWorklistOrNull();
>-
>+    // It's time to try to compile code for OSR entry.
>     Worklist::State worklistState;
>-    if (worklist) {
>+    if (Worklist* worklist = existingGlobalFTLWorklistOrNull()) {
>         worklistState = worklist->completeAllReadyPlansForVM(
>             *vm, CompilationKey(codeBlock->baselineVersion(), FTLForOSREntryMode));
>     } else
>         worklistState = Worklist::NotKnown;
>     
>-    if (worklistState == Worklist::Compiling) {
>-        ASSERT(!jitCode->osrEntryBlock);
>-        jitCode->setOptimizationThresholdBasedOnCompilationResult(
>-            codeBlock, CompilationDeferred);
>+    if (worklistState == Worklist::Compiling)
>         return 0;
>-    }
>     
>     if (CodeBlock* entryBlock = jitCode->osrEntryBlock.get()) {
>         void* address = FTL::prepareOSREntry(
>             exec, codeBlock, entryBlock, bytecodeIndex, streamIndex);
>-        if (address) {
>-            jitCode->optimizeSoon(codeBlock);
>+        if (address)
>             return static_cast<char*>(address);
>-        }
>         
>         FTL::ForOSREntryJITCode* entryCode = entryBlock->jitCode()->ftlForOSREntry();
>         entryCode->countEntryFailure();
>         if (entryCode->entryFailureCount() <
>-            Options::ftlOSREntryFailureCountForReoptimization()) {
>-            
>-            jitCode->optimizeSoon(codeBlock);
>+            Options::ftlOSREntryFailureCountForReoptimization())
>             return 0;
>-        }
>         
>         // OSR entry failed. Oh no! This implies that we need to retry. We retry
>         // without exponential backoff and we only do this for the entry code block.
>         jitCode->osrEntryBlock.clear();
>-        
>-        jitCode->optimizeAfterWarmUp(codeBlock);
>+        jitCode->osrEntryRetry = 0;
>         return 0;
>     }
>     
>@@ -1250,7 +1245,8 @@ char* JIT_OPERATION triggerOSREntryNow(
>         return 0;
>     }
> 
>-    // The first order of business is to trigger a for-entry compile.
>+    // We aren't compiling and haven't compiled anything for OSR entry. So, try to compile
>+    // something.
>     Operands<JSValue> mustHandleValues;
>     jitCode->reconstruct(
>         exec, codeBlock, CodeOrigin(bytecodeIndex), streamIndex, mustHandleValues);
>@@ -1258,24 +1254,6 @@ char* JIT_OPERATION triggerOSREntryNow(
>         *vm, codeBlock->newReplacement().get(), codeBlock, FTLForOSREntryMode, bytecodeIndex,
>         mustHandleValues, ToFTLForOSREntryDeferredCompilationCallback::create(codeBlock));
>     
>-    // But we also want to trigger a replacement compile. Of course, we don't want to
>-    // trigger it if we don't need to. Note that this is kind of weird because we might
>-    // have just finished an FTL compile and that compile failed or was invalidated.
>-    // But this seems uncommon enough that we sort of don't care. It's certainly sound
>-    // to fire off another compile right now so long as we're not already compiling and
>-    // we don't already have an optimized replacement. Note, we don't do this for
>-    // obviously bad cases like global code, where we know that there is a slim chance
>-    // of this code being invoked ever again.
>-    CompilationKey keyForReplacement(codeBlock->baselineVersion(), FTLMode);
>-    if (codeBlock->codeType() != GlobalCode
>-        && !codeBlock->hasOptimizedReplacement()
>-        && (!worklist
>-            || worklist->compilationState(keyForReplacement) == Worklist::NotKnown)) {
>-        compile(
>-            *vm, codeBlock->newReplacement().get(), codeBlock, FTLMode, UINT_MAX,
>-            Operands<JSValue>(), ToFTLDeferredCompilationCallback::create(codeBlock));
>-    }
>-    
>     if (forEntryResult != CompilationSuccessful)
>         return 0;
>     
>@@ -1284,10 +1262,6 @@ char* JIT_OPERATION triggerOSREntryNow(
>     // We signal to try again after a while if that happens.
>     void* address = FTL::prepareOSREntry(
>         exec, codeBlock, jitCode->osrEntryBlock.get(), bytecodeIndex, streamIndex);
>-    if (address)
>-        jitCode->optimizeSoon(codeBlock);
>-    else
>-        jitCode->optimizeAfterWarmUp(codeBlock);
>     return static_cast<char*>(address);
> }
> 
>Index: Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp	(revision 163422)
>+++ Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp	(working copy)
>@@ -73,13 +73,24 @@ void ToFTLForOSREntryDeferredCompilation
>             ") result: ", result, "\n");
>     }
>     
>-    if (result == CompilationSuccessful)
>-        m_dfgCodeBlock->jitCode()->dfg()->osrEntryBlock = codeBlock;
>+    JITCode* jitCode = m_dfgCodeBlock->jitCode()->dfg();
>+        
>+    switch (result) {
>+    case CompilationSuccessful:
>+        jitCode->osrEntryBlock = codeBlock;
>+        return;
>+    case CompilationFailed:
>+        jitCode->osrEntryRetry = 0;
>+        jitCode->abandonOSREntry = true;
>+        return;
>+    case CompilationDeferred:
>+        return;
>+    case CompilationInvalidated:
>+        jitCode->osrEntryRetry = 0;
>+        return;
>+    }
>     
>-    // FIXME: if we failed, we might want to just turn off OSR entry rather than
>-    // totally turning off tier-up.
>-    m_dfgCodeBlock->jitCode()->dfg()->setOptimizationThresholdBasedOnCompilationResult(
>-        m_dfgCodeBlock.get(), result);
>+    RELEASE_ASSERT_NOT_REACHED();
> }
> 
> } } // JSC::DFG
>Index: Source/JavaScriptCore/runtime/Options.h
>===================================================================
>--- Source/JavaScriptCore/runtime/Options.h	(revision 163422)
>+++ Source/JavaScriptCore/runtime/Options.h	(working copy)
>@@ -197,6 +197,7 @@ typedef OptionRange optionRange;
>     v(int32, ftlTierUpCounterIncrementForLoop, 1) \
>     v(int32, ftlTierUpCounterIncrementForReturn, 15) \
>     v(unsigned, ftlOSREntryFailureCountForReoptimization, 15) \
>+    v(unsigned, ftlOSREntryRetryThreshold, 100) \
>     \
>     v(int32, evalThresholdMultiplier, 10) \
>     \

Actions: View | Formatted Diff | Diff

Attachments on bug 128234: 223211 | 223309