Unreviewed, rolling back in r188792.
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Aug 2015 02:07:27 +0000 (02:07 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Aug 2015 02:07:27 +0000 (02:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148347

Previously reverted changesets:

"Unify code paths for manually deleting all code"
https://bugs.webkit.org/show_bug.cgi?id=148280
http://trac.webkit.org/changeset/188792

The previous patch caused some inspector tests to hang because it
introduced extra calls to sourceParsed, and sourceParsed is
pathologically slow in WK1 debug builds. This patch restores pre-existing
code to limit calls to sourceParsed, excluding code not being debugged
(i.e., inspector code).

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@188846 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/debugger/Debugger.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/inspector/agents/InspectorRuntimeAgent.cpp
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/runtime/VMEntryScope.cpp
Source/JavaScriptCore/runtime/VMEntryScope.h

index cb1e404..d008baf 100644 (file)
@@ -1,7 +1,26 @@
 2015-08-23  Geoffrey Garen  <ggaren@apple.com>
 
+        Unreviewed, rolling back in r188792.
+        https://bugs.webkit.org/show_bug.cgi?id=148347
+
+        Previously reverted changesets:
+
+        "Unify code paths for manually deleting all code"
+        https://bugs.webkit.org/show_bug.cgi?id=148280
+        http://trac.webkit.org/changeset/188792
+
+        The previous patch caused some inspector tests to hang because it
+        introduced extra calls to sourceParsed, and sourceParsed is
+        pathologically slow in WK1 debug builds. This patch restores pre-existing
+        code to limit calls to sourceParsed, excluding code not being debugged
+        (i.e., inspector code).
+
+2015-08-23  Geoffrey Garen  <ggaren@apple.com>
+
         Unreviewed, rolling back in r188803.
 
+        Previously reverted changesets:
+
         "Debugger's VM should never be null"
         https://bugs.webkit.org/show_bug.cgi?id=148341
         http://trac.webkit.org/changeset/188803
index cc78389..3538372 100644 (file)
@@ -39,65 +39,30 @@ namespace {
 
 using namespace JSC;
 
-class Recompiler : public MarkedBlock::VoidFunctor {
-public:
-    Recompiler(JSC::Debugger*);
-    ~Recompiler();
-    IterationStatus operator()(JSCell*);
-
-private:
-    typedef HashSet<FunctionExecutable*> FunctionExecutableSet;
-    typedef HashMap<SourceProvider*, ExecState*> SourceProviderMap;
-    
-    void visit(JSCell*);
-    
+struct GatherSourceProviders : public MarkedBlock::VoidFunctor {
+    HashSet<SourceProvider*> sourceProviders;
     JSC::Debugger* m_debugger;
-    FunctionExecutableSet m_functionExecutables;
-    SourceProviderMap m_sourceProviders;
-};
 
-inline Recompiler::Recompiler(JSC::Debugger* debugger)
-    : m_debugger(debugger)
-{
-}
+    GatherSourceProviders(JSC::Debugger* debugger)
+        : m_debugger(debugger) { }
 
-inline Recompiler::~Recompiler()
-{
-    // Call sourceParsed() after reparsing all functions because it will execute
-    // JavaScript in the inspector.
-    SourceProviderMap::const_iterator end = m_sourceProviders.end();
-    for (SourceProviderMap::const_iterator iter = m_sourceProviders.begin(); iter != end; ++iter)
-        m_debugger->sourceParsed(iter->value, iter->key, -1, String());
-}
-
-inline void Recompiler::visit(JSCell* cell)
-{
-    if (!cell->inherits(JSFunction::info()))
-        return;
-
-    JSFunction* function = jsCast<JSFunction*>(cell);
-    if (function->executable()->isHostFunction())
-        return;
-
-    FunctionExecutable* executable = function->jsExecutable();
+    IterationStatus operator()(JSCell* cell)
+    {
+        JSFunction* function = jsDynamicCast<JSFunction*>(cell);
+        if (!function)
+            return IterationStatus::Continue;
 
-    // Check if the function is already in the set - if so,
-    // we've already retranslated it, nothing to do here.
-    if (!m_functionExecutables.add(executable).isNewEntry)
-        return;
+        if (function->scope()->globalObject()->debugger() != m_debugger)
+            return IterationStatus::Continue;
 
-    ExecState* exec = function->scope()->globalObject()->JSGlobalObject::globalExec();
-    executable->clearCode();
-    executable->clearUnlinkedCodeForRecompilation();
-    if (m_debugger == function->scope()->globalObject()->debugger())
-        m_sourceProviders.add(executable->source().provider(), exec);
-}
+        if (!function->executable()->isFunctionExecutable())
+            return IterationStatus::Continue;
 
-inline IterationStatus Recompiler::operator()(JSCell* cell)
-{
-    visit(cell);
-    return IterationStatus::Continue;
-}
+        sourceProviders.add(
+            jsCast<FunctionExecutable*>(function->executable())->source().provider());
+        return IterationStatus::Continue;
+    }
+};
 
 } // namespace
 
@@ -176,6 +141,15 @@ void Debugger::attach(JSGlobalObject* globalObject)
     ASSERT(!globalObject->debugger());
     globalObject->setDebugger(this);
     m_globalObjects.add(globalObject);
+
+    // Call sourceParsed() because it will execute JavaScript in the inspector.
+    GatherSourceProviders gatherSourceProviders(this);
+    {
+        HeapIterationScope iterationScope(m_vm.heap);
+        m_vm.heap.objectSpace().forEachLiveCell(iterationScope, gatherSourceProviders);
+    }
+    for (auto* sourceProvider : gatherSourceProviders.sourceProviders)
+        sourceParsed(globalObject->globalExec(), sourceProvider, -1, String());
 }
 
 void Debugger::detach(JSGlobalObject* globalObject, ReasonForDetach reason)
@@ -328,26 +302,7 @@ void Debugger::toggleBreakpoint(Breakpoint& breakpoint, Debugger::BreakpointStat
 
 void Debugger::recompileAllJSFunctions()
 {
-    // If JavaScript is running, it's not safe to recompile, since we'll end
-    // up throwing away code that is live on the stack.
-    if (m_vm.entryScope) {
-        auto listener = [] (VM&, JSGlobalObject* globalObject) 
-        {
-            if (Debugger* debugger = globalObject->debugger())
-                debugger->recompileAllJSFunctions();
-        };
-
-        m_vm.entryScope->setEntryScopeDidPopListener(this, listener);
-        return;
-    }
-
-#if ENABLE(DFG_JIT)
-    DFG::completeAllPlansForVM(m_vm);
-#endif
-
-    Recompiler recompiler(this);
-    HeapIterationScope iterationScope(m_vm.heap);
-    m_vm.heap.objectSpace().forEachLiveCell(iterationScope, recompiler);
+    m_vm.deleteAllCode();
 }
 
 BreakpointID Debugger::setBreakpoint(Breakpoint breakpoint, unsigned& actualLine, unsigned& actualColumn)
index 9cbebdc..3f516d7 100644 (file)
@@ -237,6 +237,8 @@ public:
 
     void addLogicallyEmptyWeakBlock(WeakBlock*);
 
+    Vector<ExecutableBase*>& compiledCode() { return m_compiledCode; }
+
 private:
     friend class CodeBlock;
     friend class CopiedBlock;
index d985714..ae18afc 100644 (file)
@@ -308,40 +308,6 @@ void InspectorRuntimeAgent::getRuntimeTypesForVariablesAtOffsets(ErrorString& er
         dataLogF("Inspector::getRuntimeTypesForVariablesAtOffsets took %lfms\n", end - start);
 }
 
-class TypeRecompiler : public MarkedBlock::VoidFunctor {
-public:
-    inline void visit(JSCell* cell)
-    {
-        if (!cell->inherits(FunctionExecutable::info()))
-            return;
-
-        FunctionExecutable* executable = jsCast<FunctionExecutable*>(cell);
-        executable->clearCode();
-        executable->clearUnlinkedCodeForRecompilation();
-    }
-    inline IterationStatus operator()(JSCell* cell)
-    {
-        visit(cell);
-        return IterationStatus::Continue;
-    }
-};
-
-static void recompileAllJSFunctionsForTypeProfiling(VM& vm, bool shouldEnableTypeProfiling)
-{
-    bool shouldRecompileFromTypeProfiler = (shouldEnableTypeProfiling ? vm.enableTypeProfiler() : vm.disableTypeProfiler());
-    bool shouldRecompileFromControlFlowProfiler = (shouldEnableTypeProfiling ? vm.enableControlFlowProfiler() : vm.disableControlFlowProfiler());
-    bool needsToRecompile = shouldRecompileFromTypeProfiler || shouldRecompileFromControlFlowProfiler;
-
-    if (needsToRecompile) {
-#if ENABLE(DFG_JIT)
-        DFG::completeAllPlansForVM(vm);
-#endif
-        TypeRecompiler recompiler;
-        HeapIterationScope iterationScope(vm.heap);
-        vm.heap.objectSpace().forEachLiveCell(iterationScope, recompiler);
-    }
-}
-
 void InspectorRuntimeAgent::willDestroyFrontendAndBackend(DisconnectReason reason)
 {
     if (reason != DisconnectReason::InspectedTargetDestroyed && m_isTypeProfilingEnabled)
@@ -358,25 +324,21 @@ void InspectorRuntimeAgent::disableTypeProfiler(ErrorString&)
     setTypeProfilerEnabledState(false);
 }
 
-void InspectorRuntimeAgent::setTypeProfilerEnabledState(bool shouldEnableTypeProfiling)
+void InspectorRuntimeAgent::setTypeProfilerEnabledState(bool isTypeProfilingEnabled)
 {
-    if (m_isTypeProfilingEnabled == shouldEnableTypeProfiling)
+    if (m_isTypeProfilingEnabled == isTypeProfilingEnabled)
         return;
-
-    m_isTypeProfilingEnabled = shouldEnableTypeProfiling;
+    m_isTypeProfilingEnabled = isTypeProfilingEnabled;
 
     VM& vm = globalVM();
-
-    // If JavaScript is running, it's not safe to recompile, since we'll end
-    // up throwing away code that is live on the stack.
-    if (vm.entryScope) {
-        vm.entryScope->setEntryScopeDidPopListener(this,
-            [=] (VM& vm, JSGlobalObject*) {
-                recompileAllJSFunctionsForTypeProfiling(vm, shouldEnableTypeProfiling);
-            }
-        );
-    } else
-        recompileAllJSFunctionsForTypeProfiling(vm, shouldEnableTypeProfiling);
+    vm.whenIdle([&vm, isTypeProfilingEnabled] () {
+        bool shouldRecompileFromTypeProfiler = (isTypeProfilingEnabled ? vm.enableTypeProfiler() : vm.disableTypeProfiler());
+        bool shouldRecompileFromControlFlowProfiler = (isTypeProfilingEnabled ? vm.enableControlFlowProfiler() : vm.disableControlFlowProfiler());
+        bool needsToRecompile = shouldRecompileFromTypeProfiler || shouldRecompileFromControlFlowProfiler;
+
+        if (needsToRecompile)
+            vm.deleteAllCode();
+    });
 }
 
 void InspectorRuntimeAgent::getBasicBlocks(ErrorString& errorString, const String& sourceIDAsString, RefPtr<Inspector::Protocol::Array<Inspector::Protocol::Runtime::BasicBlock>>& basicBlocks)
index 3d4a46c..1204f04 100644 (file)
@@ -86,6 +86,7 @@
 #include "TypeProfiler.h"
 #include "TypeProfilerLog.h"
 #include "UnlinkedCodeBlock.h"
+#include "VMEntryScope.h"
 #include "Watchdog.h"
 #include "WeakGCMapInlines.h"
 #include "WeakMapData.h"
@@ -470,16 +471,28 @@ void VM::stopSampling()
     interpreter->stopSampling();
 }
 
+void VM::whenIdle(std::function<void()> callback)
+{
+    if (!entryScope) {
+        callback();
+        return;
+    }
+
+    entryScope->addDidPopListener(callback);
+}
+
 void VM::deleteAllCode()
 {
-    m_codeCache->clear();
-    m_regExpCache->deleteAllCode();
+    whenIdle([this]() {
+        m_codeCache->clear();
+        m_regExpCache->deleteAllCode();
 #if ENABLE(DFG_JIT)
-    DFG::completeAllPlansForVM(*this);
+        DFG::completeAllPlansForVM(*this);
 #endif
-    heap.deleteAllCompiledCode();
-    heap.deleteAllUnlinkedFunctionCode();
-    heap.reportAbandonedObjectGraph();
+        heap.deleteAllCompiledCode();
+        heap.deleteAllUnlinkedFunctionCode();
+        heap.reportAbandonedObjectGraph();
+    });
 }
 
 void VM::dumpSampleData(ExecState* exec)
index e87efdb..9afe9a5 100644 (file)
@@ -539,6 +539,8 @@ public:
     JSLock& apiLock() { return *m_apiLock; }
     CodeCache* codeCache() { return m_codeCache.get(); }
 
+    void whenIdle(std::function<void()>);
+
     JS_EXPORT_PRIVATE void deleteAllCode();
 
     void registerWatchpointForImpureProperty(const Identifier&, Watchpoint*);
index bc901b2..086a2d9 100644 (file)
@@ -57,9 +57,9 @@ VMEntryScope::VMEntryScope(VM& vm, JSGlobalObject* globalObject)
     vm.clearLastException();
 }
 
-void VMEntryScope::setEntryScopeDidPopListener(void* key, EntryScopeDidPopListener listener)
+void VMEntryScope::addDidPopListener(std::function<void ()> listener)
 {
-    m_allEntryScopeDidPopListeners.set(key, listener);
+    m_didPopListeners.append(listener);
 }
 
 VMEntryScope::~VMEntryScope()
@@ -72,8 +72,8 @@ VMEntryScope::~VMEntryScope()
 
     m_vm.entryScope = nullptr;
 
-    for (auto& listener : m_allEntryScopeDidPopListeners.values())
-        listener(m_vm, m_globalObject);
+    for (auto& listener : m_didPopListeners)
+        listener();
 }
 
 } // namespace JSC
index 6a62831..95c60ac 100644 (file)
@@ -27,9 +27,9 @@
 #define VMEntryScope_h
 
 #include "Interpreter.h"
-#include <wtf/HashMap.h>
 #include <wtf/StackBounds.h>
 #include <wtf/StackStats.h>
+#include <wtf/Vector.h>
 
 namespace JSC {
 
@@ -43,13 +43,12 @@ public:
 
     JSGlobalObject* globalObject() const { return m_globalObject; }
 
-    typedef std::function<void (VM&, JSGlobalObject*)> EntryScopeDidPopListener;
-    void setEntryScopeDidPopListener(void*, EntryScopeDidPopListener);
+    void addDidPopListener(std::function<void ()>);
 
 private:
     VM& m_vm;
     JSGlobalObject* m_globalObject;
-    HashMap<void*, EntryScopeDidPopListener> m_allEntryScopeDidPopListeners;
+    Vector<std::function<void ()>> m_didPopListeners;
 };
 
 } // namespace JSC