Attachment 233923 Details for Bug 128072 – work in progress

[patch] work in progress

blah.patch (text/plain), 20.13 KB, created by Filip Pizlo on 2014-06-26 12:20:21 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2014-06-26 12:20:21 PDT

Size: 20.13 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 170488)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,45 @@
>+2014-06-26  Filip Pizlo  <fpizlo@apple.com>
>+
>+        GC should keep structures alive if they are inlined into optimized code (DFG or FTL) and their globalObject is alive
>+        https://bugs.webkit.org/show_bug.cgi?id=128072
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        This still incurs some goofy slow-downs, but it's almost done other than that.
>+
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::dumpAssumingJITType):
>+        (JSC::shouldMarkStructure):
>+        (JSC::shouldMarkTransition):
>+        (JSC::CodeBlock::propagateTransitions):
>+        (JSC::CodeBlock::determineLiveness):
>+        * dfg/DFGCapabilities.cpp:
>+        (JSC::DFG::isSupportedForInlining):
>+        (JSC::DFG::mightInlineFunctionForCall):
>+        (JSC::DFG::mightInlineFunctionForClosureCall):
>+        (JSC::DFG::mightInlineFunctionForConstruct):
>+        * dfg/DFGCapabilities.h:
>+        * dfg/DFGCommonData.h:
>+        * dfg/DFGDesiredWeakReferences.cpp:
>+        (JSC::DFG::DesiredWeakReferences::reallyAdd):
>+        * dfg/DFGOSREntry.cpp:
>+        (JSC::DFG::prepareOSREntry):
>+        * dfg/DFGOSRExitCompilerCommon.cpp:
>+        (JSC::DFG::handleExitCounts):
>+        * dfg/DFGOperations.cpp:
>+        * dfg/DFGOperations.h:
>+        * ftl/FTLOSREntry.cpp:
>+        (JSC::FTL::prepareOSREntry):
>+        * runtime/Executable.cpp:
>+        (JSC::ScriptExecutable::installCode):
>+        * runtime/Executable.h:
>+        (JSC::ScriptExecutable::setDidTryToEnterInLoop):
>+        (JSC::ScriptExecutable::didTryToEnterInLoop):
>+        (JSC::ScriptExecutable::addressOfDidTryToEnterInLoop):
>+        * runtime/StructureInlines.h:
>+        (JSC::Structure::storedPrototypeObject):
>+        (JSC::Structure::storedPrototypeStructure):
>+
> 2014-06-25  Mark Hahnenberg  <mhahnenberg@apple.com>
> 
>         Structure bit fields should have a consistent format
>Index: Source/JavaScriptCore/bytecode/CodeBlock.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeBlock.cpp	(revision 170432)
>+++ Source/JavaScriptCore/bytecode/CodeBlock.cpp	(working copy)
>@@ -148,9 +148,11 @@ void CodeBlock::dumpAssumingJITType(Prin
>         out.print(specializationKind());
>     out.print(", ", instructionCount());
>     if (this->jitType() == JITCode::BaselineJIT && m_shouldAlwaysBeInlined)
>-        out.print(" (SABI)");
>+        out.print(" (ShouldAlwaysBeInlined)");
>     if (ownerExecutable()->neverInline())
>         out.print(" (NeverInline)");
>+    if (ownerExecutable()->didTryToEnterInLoop())
>+        out.print(" (DidTryToEnterInLoop)");
>     if (ownerExecutable()->isStrictMode())
>         out.print(" (StrictMode)");
>     if (this->jitType() == JITCode::BaselineJIT && m_didFailFTLCompilation)
>@@ -2131,6 +2133,38 @@ bool CodeBlock::isKnownToBeLiveDuringGC(
> #endif
> }
> 
>+static bool shouldMarkStructure(Structure* structure)
>+{
>+    if (JSGlobalObject* globalObject = structure->globalObject()) {
>+        if (!Heap::isMarked(globalObject))
>+            return false;
>+    }
>+    
>+    if (JSObject* prototypeObject = structure->storedPrototypeObject()) {
>+        if (!Heap::isMarked(prototypeObject))
>+            return false;
>+    }
>+    
>+    // If both the global object and the prototype are already live anyway, then this structure
>+    // is harmless to mark.
>+    
>+    return true;
>+}
>+
>+static bool shouldMarkTransition(DFG::WeakReferenceTransition& transition)
>+{
>+    if (shouldMarkStructure(jsCast<Structure*>(transition.m_to.get())))
>+        return true;
>+    
>+    if (transition.m_codeOrigin && !Heap::isMarked(transition.m_codeOrigin.get()))
>+        return false;
>+    
>+    if (!Heap::isMarked(transition.m_from.get()))
>+        return false;
>+    
>+    return true;
>+}
>+
> void CodeBlock::propagateTransitions(SlotVisitor& visitor)
> {
>     UNUSED_PARAM(visitor);
>@@ -2209,21 +2243,35 @@ void CodeBlock::propagateTransitions(Slo
> #if ENABLE(DFG_JIT)
>     if (JITCode::isOptimizingJIT(jitType())) {
>         DFG::CommonData* dfgCommon = m_jitCode->dfgCommon();
>+        
>+        for (unsigned i = 0; i < dfgCommon->weakStructureReferences.size(); ++i) {
>+            if (shouldMarkStructure(dfgCommon->weakStructureReferences[i].get()))
>+                visitor.append(&dfgCommon->weakStructureReferences[i]);
>+            else
>+                allAreMarkedSoFar = false;
>+        }
>+        
>         for (unsigned i = 0; i < dfgCommon->transitions.size(); ++i) {
>-            if ((!dfgCommon->transitions[i].m_codeOrigin
>-                 || Heap::isMarked(dfgCommon->transitions[i].m_codeOrigin.get()))
>-                && Heap::isMarked(dfgCommon->transitions[i].m_from.get())) {
>+            if (shouldMarkTransition(dfgCommon->transitions[i])) {
>                 // If the following three things are live, then the target of the
>                 // transition is also live:
>+                //
>                 // - This code block. We know it's live already because otherwise
>                 //   we wouldn't be scanning ourselves.
>+                //
>                 // - The code origin of the transition. Transitions may arise from
>                 //   code that was inlined. They are not relevant if the user's
>                 //   object that is required for the inlinee to run is no longer
>                 //   live.
>+                //
>                 // - The source of the transition. The transition checks if some
>                 //   heap location holds the source, and if so, stores the target.
>                 //   Hence the source must be live for the transition to be live.
>+                //
>+                // We also short-circuit the liveness if the structure is harmless
>+                // to mark (i.e. its global object and prototype are both already
>+                // live).
>+                
>                 visitor.append(&dfgCommon->transitions[i].m_to);
>             } else
>                 allAreMarkedSoFar = false;
>@@ -2258,6 +2306,14 @@ void CodeBlock::determineLiveness(SlotVi
>             break;
>         }
>     }
>+    if (allAreLiveSoFar) {
>+        for (unsigned i = 0; i < dfgCommon->weakStructureReferences.size(); ++i) {
>+            if (!Heap::isMarked(dfgCommon->weakStructureReferences[i].get())) {
>+                allAreLiveSoFar = false;
>+                break;
>+            }
>+        }
>+    }
>     
>     // If some weak references are dead, then this fixpoint iteration was
>     // unsuccessful.
>Index: Source/JavaScriptCore/dfg/DFGCapabilities.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGCapabilities.cpp	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGCapabilities.cpp	(working copy)
>@@ -45,6 +45,12 @@ bool isSupported(CodeBlock* codeBlock)
>         && FunctionWhitelist::ensureGlobalWhitelist().contains(codeBlock);
> }
> 
>+bool isSupportedForInlining(CodeBlock* codeBlock)
>+{
>+    return !codeBlock->ownerExecutable()->needsActivation()
>+        && codeBlock->ownerExecutable()->isInliningCandidate();
>+}
>+
> bool mightCompileEval(CodeBlock* codeBlock)
> {
>     return isSupported(codeBlock)
>@@ -69,20 +75,17 @@ bool mightCompileFunctionForConstruct(Co
> bool mightInlineFunctionForCall(CodeBlock* codeBlock)
> {
>     return codeBlock->instructionCount() <= Options::maximumFunctionForCallInlineCandidateInstructionCount()
>-        && !codeBlock->ownerExecutable()->needsActivation()
>-        && codeBlock->ownerExecutable()->isInliningCandidate();
>+        && isSupportedForInlining(codeBlock);
> }
> bool mightInlineFunctionForClosureCall(CodeBlock* codeBlock)
> {
>     return codeBlock->instructionCount() <= Options::maximumFunctionForClosureCallInlineCandidateInstructionCount()
>-        && !codeBlock->ownerExecutable()->needsActivation()
>-        && codeBlock->ownerExecutable()->isInliningCandidate();
>+        && isSupportedForInlining(codeBlock);
> }
> bool mightInlineFunctionForConstruct(CodeBlock* codeBlock)
> {
>     return codeBlock->instructionCount() <= Options::maximumFunctionForConstructInlineCandidateInstructionCount()
>-        && !codeBlock->ownerExecutable()->needsActivation()
>-        && codeBlock->ownerExecutable()->isInliningCandidate();
>+        && isSupportedForInlining(codeBlock);
> }
> 
> inline void debugFail(CodeBlock* codeBlock, OpcodeID opcodeID, CapabilityLevel result)
>Index: Source/JavaScriptCore/dfg/DFGCapabilities.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGCapabilities.h	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGCapabilities.h	(working copy)
>@@ -39,6 +39,7 @@ namespace JSC { namespace DFG {
> // Fast check functions; if they return true it is still necessary to
> // check opcodes.
> bool isSupported(CodeBlock*);
>+bool isSupportedForInlining(CodeBlock*);
> bool mightCompileEval(CodeBlock*);
> bool mightCompileProgram(CodeBlock*);
> bool mightCompileFunctionForCall(CodeBlock*);
>Index: Source/JavaScriptCore/dfg/DFGCommonData.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGCommonData.h	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGCommonData.h	(working copy)
>@@ -95,6 +95,7 @@ public:
>     Vector<Identifier> dfgIdentifiers;
>     Vector<WeakReferenceTransition> transitions;
>     Vector<WriteBarrier<JSCell>> weakReferences;
>+    Vector<WriteBarrier<Structure>> weakStructureReferences;
>     SegmentedVector<CodeBlockJettisoningWatchpoint, 1, 0> watchpoints;
>     SegmentedVector<ProfiledCodeBlockJettisoningWatchpoint, 1, 0> profiledWatchpoints;
>     Vector<JumpReplacement> jumpReplacements;
>Index: Source/JavaScriptCore/dfg/DFGDesiredWeakReferences.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGDesiredWeakReferences.cpp	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGDesiredWeakReferences.cpp	(working copy)
>@@ -57,7 +57,13 @@ void DesiredWeakReferences::reallyAdd(VM
> {
>     for (unsigned i = 0; i < m_references.size(); i++) {
>         JSCell* target = m_references[i];
>-        common->weakReferences.append(WriteBarrier<JSCell>(vm, m_codeBlock->ownerExecutable(), target));
>+        if (Structure* structure = jsDynamicCast<Structure*>(target)) {
>+            common->weakStructureReferences.append(
>+                WriteBarrier<Structure>(vm, m_codeBlock->ownerExecutable(), structure));
>+        } else {
>+            common->weakReferences.append(
>+                WriteBarrier<JSCell>(vm, m_codeBlock->ownerExecutable(), target));
>+        }
>     }
> }
> 
>Index: Source/JavaScriptCore/dfg/DFGOSREntry.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSREntry.cpp	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGOSREntry.cpp	(working copy)
>@@ -58,6 +58,9 @@ void* prepareOSREntry(ExecState* exec, C
> 
>     sanitizeStackForVM(vm);
>     
>+    if (bytecodeIndex)
>+        codeBlock->ownerExecutable()->setDidTryToEnterInLoop(true);
>+    
>     if (codeBlock->jitType() != JITCode::DFGJIT) {
>         RELEASE_ASSERT(codeBlock->jitType() == JITCode::FTLJIT);
>         
>Index: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp	(working copy)
>@@ -54,20 +54,57 @@ void handleExitCounts(CCallHelpers& jit,
>         AssemblyHelpers::GreaterThanOrEqual,
>         AssemblyHelpers::Address(GPRInfo::regT0, CodeBlock::offsetOfJITExecuteCounter()),
>         AssemblyHelpers::TrustedImm32(0));
>+    
>+    // We want to figure out if there's a possibility that we're in a loop. For the outermost
>+    // code block in the inline stack, we handle this appropriately by having the loop OSR trigger
>+    // check the exit count of the replacement of the CodeBlock from which we are OSRing. The
>+    // problem is the inlined functions, which might also have loops, but whose baseline versions
>+    // don't know where to look for the exit count. Figure out if those loops are severe enough
>+    // that we had tried to OSR enter. If so, then we should use the loop reoptimization trigger.
>+    // Otherwise, we should use the normal reoptimization trigger.
>+    
>+    AssemblyHelpers::JumpList loopThreshold;
>+    
>+    for (InlineCallFrame* inlineCallFrame = exit.m_codeOrigin.inlineCallFrame; inlineCallFrame; inlineCallFrame = inlineCallFrame->caller.inlineCallFrame) {
>+        loopThreshold.append(
>+            jit.branchTest8(
>+                AssemblyHelpers::NonZero,
>+                AssemblyHelpers::AbsoluteAddress(
>+                    inlineCallFrame->executable->addressOfDidTryToEnterInLoop())));
>+    }
>+    
>+    jit.move(
>+        AssemblyHelpers::TrustedImm32(jit.codeBlock()->exitCountThresholdForReoptimization()),
>+        GPRInfo::regT1);
>+    
>+    if (!loopThreshold.empty()) {
>+        AssemblyHelpers::Jump done = jit.jump();
>+
>+        loopThreshold.link(&jit);
>+        if (false && jit.codeBlock()->instructionCount() == 681)
>+            jit.breakpoint();
>+        jit.move(
>+            AssemblyHelpers::TrustedImm32(
>+                jit.codeBlock()->exitCountThresholdForReoptimizationFromLoop()),
>+            GPRInfo::regT1);
>         
>-    tooFewFails = jit.branch32(AssemblyHelpers::BelowOrEqual, GPRInfo::regT2, AssemblyHelpers::TrustedImm32(jit.codeBlock()->exitCountThresholdForReoptimization()));
>+        done.link(&jit);
>+    }
>+    
>+    tooFewFails = jit.branch32(AssemblyHelpers::BelowOrEqual, GPRInfo::regT2, GPRInfo::regT1);
>     
>     reoptimizeNow.link(&jit);
>     
>     // Reoptimize as soon as possible.
> #if !NUMBER_OF_ARGUMENT_REGISTERS
>     jit.poke(GPRInfo::regT0);
>+    jit.poke(AssemblyHelpers::TrustedImmPtr(&exit), 1);
> #else
>     jit.move(GPRInfo::regT0, GPRInfo::argumentGPR0);
>-    ASSERT(GPRInfo::argumentGPR0 != GPRInfo::regT1);
>+    jit.move(AssemblyHelpers::TrustedImmPtr(&exit), GPRInfo::argumentGPR1);
> #endif
>-    jit.move(AssemblyHelpers::TrustedImmPtr(bitwise_cast<void*>(triggerReoptimizationNow)), GPRInfo::regT1);
>-    jit.call(GPRInfo::regT1);
>+    jit.move(AssemblyHelpers::TrustedImmPtr(bitwise_cast<void*>(triggerReoptimizationNow)), GPRInfo::nonArgGPR0);
>+    jit.call(GPRInfo::nonArgGPR0);
>     AssemblyHelpers::Jump doneAdjusting = jit.jump();
>     
>     tooFewFails.link(&jit);
>Index: Source/JavaScriptCore/dfg/DFGOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.cpp	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGOperations.cpp	(working copy)
>@@ -1086,7 +1086,7 @@ void JIT_OPERATION debugOperationPrintSp
>     dataLog("\n");
> }
> 
>-extern "C" void JIT_OPERATION triggerReoptimizationNow(CodeBlock* codeBlock)
>+extern "C" void JIT_OPERATION triggerReoptimizationNow(CodeBlock* codeBlock, OSRExitBase* exit)
> {
>     // It's sort of preferable that we don't GC while in here. Anyways, doing so wouldn't
>     // really be profitable.
>@@ -1110,13 +1110,21 @@ extern "C" void JIT_OPERATION triggerReo
>     ASSERT(codeBlock->hasOptimizedReplacement());
>     CodeBlock* optimizedCodeBlock = codeBlock->replacement();
>     ASSERT(JITCode::isOptimizingJIT(optimizedCodeBlock->jitType()));
>+    
>+    bool didTryToEnterIntoInlinedLoops = false;
>+    for (InlineCallFrame* inlineCallFrame = exit->m_codeOrigin.inlineCallFrame; inlineCallFrame; inlineCallFrame = inlineCallFrame->caller.inlineCallFrame) {
>+        if (inlineCallFrame->executable->didTryToEnterInLoop()) {
>+            didTryToEnterIntoInlinedLoops = true;
>+            break;
>+        }
>+    }
> 
>     // In order to trigger reoptimization, one of two things must have happened:
>     // 1) We exited more than some number of times.
>     // 2) We exited and got stuck in a loop, and now we're exiting again.
>     bool didExitABunch = optimizedCodeBlock->shouldReoptimizeNow();
>     bool didGetStuckInLoop =
>-        codeBlock->checkIfOptimizationThresholdReached()
>+        (codeBlock->checkIfOptimizationThresholdReached() || didTryToEnterIntoInlinedLoops)
>         && optimizedCodeBlock->shouldReoptimizeFromLoopNow();
>     
>     if (!didExitABunch && !didGetStuckInLoop) {
>Index: Source/JavaScriptCore/dfg/DFGOperations.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.h	(revision 170432)
>+++ Source/JavaScriptCore/dfg/DFGOperations.h	(working copy)
>@@ -31,9 +31,9 @@
> #include "JITOperations.h"
> #include "PutKind.h"
> 
>-namespace JSC {
>+namespace JSC { namespace DFG {
> 
>-namespace DFG {
>+struct OSRExitBase;
> 
> extern "C" {
> 
>@@ -132,7 +132,7 @@ size_t JIT_OPERATION dfgConvertJSValueTo
> 
> void JIT_OPERATION debugOperationPrintSpeculationFailure(ExecState*, void*, void*) WTF_INTERNAL;
> 
>-void JIT_OPERATION triggerReoptimizationNow(CodeBlock*) WTF_INTERNAL;
>+void JIT_OPERATION triggerReoptimizationNow(CodeBlock*, OSRExitBase*) WTF_INTERNAL;
> 
> #if ENABLE(FTL_JIT)
> void JIT_OPERATION triggerTierUpNow(ExecState*) WTF_INTERNAL;
>Index: Source/JavaScriptCore/ftl/FTLOSREntry.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLOSREntry.cpp	(revision 170432)
>+++ Source/JavaScriptCore/ftl/FTLOSREntry.cpp	(working copy)
>@@ -54,6 +54,9 @@ void* prepareOSREntry(
>             bytecodeIndex, ".\n");
>     }
>     
>+    if (bytecodeIndex)
>+        jsCast<ScriptExecutable*>(executable)->setDidTryToEnterInLoop(true);
>+
>     if (bytecodeIndex != entryCode->bytecodeIndex()) {
>         if (Options::verboseOSR())
>             dataLog("    OSR failed because we don't have an entrypoint for bc#", bytecodeIndex, "; ours is for bc#", entryCode->bytecodeIndex());
>Index: Source/JavaScriptCore/runtime/Executable.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/Executable.cpp	(revision 170432)
>+++ Source/JavaScriptCore/runtime/Executable.cpp	(working copy)
>@@ -108,6 +108,9 @@ void ScriptExecutable::installCode(CodeB
>     RELEASE_ASSERT(genericCodeBlock->ownerExecutable() == this);
>     RELEASE_ASSERT(JITCode::isExecutableScript(genericCodeBlock->jitType()));
>     
>+    if (Options::verboseOSR())
>+        dataLog("Installing ", *genericCodeBlock, "\n");
>+    
>     VM& vm = *genericCodeBlock->vm();
>     
>     if (vm.m_perBytecodeProfiler)
>Index: Source/JavaScriptCore/runtime/Executable.h
>===================================================================
>--- Source/JavaScriptCore/runtime/Executable.h	(revision 170432)
>+++ Source/JavaScriptCore/runtime/Executable.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2009, 2010, 2013 Apple Inc. All rights reserved.
>+ * Copyright (C) 2009, 2010, 2013, 2014 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -397,8 +397,12 @@ public:
>     ECMAMode ecmaMode() const { return isStrictMode() ? StrictMode : NotStrictMode; }
>         
>     void setNeverInline(bool value) { m_neverInline = value; }
>+    void setDidTryToEnterInLoop(bool value) { m_didTryToEnterInLoop = value; }
>     bool neverInline() const { return m_neverInline; }
>+    bool didTryToEnterInLoop() const { return m_didTryToEnterInLoop; }
>     bool isInliningCandidate() const { return !neverInline(); }
>+    
>+    bool* addressOfDidTryToEnterInLoop() { return &m_didTryToEnterInLoop; }
> 
>     void unlinkCalls();
>         
>@@ -450,6 +454,7 @@ protected:
>     CodeFeatures m_features;
>     bool m_hasCapturedVariables;
>     bool m_neverInline;
>+    bool m_didTryToEnterInLoop;
>     int m_firstLine;
>     int m_lastLine;
>     unsigned m_startColumn;
>Index: Source/JavaScriptCore/runtime/StructureInlines.h
>===================================================================
>--- Source/JavaScriptCore/runtime/StructureInlines.h	(revision 170432)
>+++ Source/JavaScriptCore/runtime/StructureInlines.h	(working copy)
>@@ -61,7 +61,7 @@ inline JSObject* Structure::storedProtot
> {
>     JSValue value = m_prototype.get();
>     if (value.isNull())
>-        return 0;
>+        return nullptr;
>     return asObject(value);
> }
> 
>@@ -69,7 +69,7 @@ inline Structure* Structure::storedProto
> {
>     JSObject* object = storedPrototypeObject();
>     if (!object)
>-        return 0;
>+        return nullptr;
>     return object->structure();
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 128072: 222934 | 222935 | 223213 | 233923 | 233951