Attachment 223551 Details for Bug 128443 – the patch

[patch] the patch

blah.patch (text/plain), 2.82 KB, created by Filip Pizlo on 2014-02-07 22:28:15 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2014-02-07 22:28:15 PST

Size: 2.82 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 163694)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2014-02-07  Filip Pizlo  <fpizlo@apple.com>
>+
>+        Don't throw away code if there is code on the worklists
>+        https://bugs.webkit.org/show_bug.cgi?id=128443
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        If we throw away compiled code and there is code currently being JITed then the JIT
>+        will get confused after it resumes: it will see a code block that had claimed to belong
>+        to an executable except that it doesn't belong to any executables anymore.
>+
>+        * dfg/DFGWorklist.h:
>+        (JSC::DFG::Worklist::isActive):
>+        * heap/Heap.cpp:
>+        (JSC::Heap::deleteAllCompiledCode):
>+
> 2014-02-07  Filip Pizlo  <fpizlo@apple.com>
> 
>         GC should safepoint the DFG worklist in a smarter way rather than just waiting for everything to complete
>Index: Source/JavaScriptCore/dfg/DFGWorklist.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGWorklist.h	(revision 163694)
>+++ Source/JavaScriptCore/dfg/DFGWorklist.h	(working copy)
>@@ -70,6 +70,8 @@ public:
>     void suspendAllThreads();
>     void resumeAllThreads();
>     
>+    bool isActive() { return !!m_plans.size(); }
>+    
>     void visitChildren(SlotVisitor&, CodeBlockSet&); // Only called on the main thread after suspending all threads.
>     
>     void dump(PrintStream&) const;
>Index: Source/JavaScriptCore/heap/Heap.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/Heap.cpp	(revision 163694)
>+++ Source/JavaScriptCore/heap/Heap.cpp	(working copy)
>@@ -732,6 +732,22 @@ void Heap::deleteAllCompiledCode()
>     // up deleting code that is live on the stack.
>     if (m_vm->entryScope)
>         return;
>+    
>+    // If we have things on any worklist, then don't delete code. This is kind of
>+    // a weird heuristic. It's definitely not safe to throw away code that is on
>+    // the worklist. But this change was made in a hurry so we just avoid throwing
>+    // away any code if there is any code on any worklist. I suspect that this
>+    // might not actually be too dumb: if there is code on worklists then that
>+    // means that we are running some hot JS code right now. Maybe causing
>+    // recompilations isn't a good idea.
>+#if ENABLE(DFG_JIT)
>+    for (unsigned i = DFG::numberOfWorklists(); i--;) {
>+        if (DFG::Worklist* worklist = DFG::worklistForIndexOrNull(i)) {
>+            if (worklist->isActive())
>+                return;
>+        }
>+    }
>+#endif // ENABLE(DFG_JIT)
> 
>     for (ExecutableBase* current = m_compiledCode.head(); current; current = current->next()) {
>         if (!current->isFunctionExecutable())

Flags:

joepeck: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 128443: 223551