Attachment 343185 Details for Bug 186859 – Patch

[patch] Patch

bug-186859-20180621001846.patch (text/plain), 12.29 KB, created by Tadeu Zagallo on 2018-06-20 15:18:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2018-06-20 15:18:48 PDT

Size: 12.29 KB

patch

obsolete

>Subversion Revision: 232958
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 68a6806855e6f07dfebeff7d5a64728ed1b90e6f..6d1df3e8fc5be9b2812d59beadc55cadbc0fce7e 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-06-20  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Add API for configuring the number of threads used by DFG and FTL
>+        https://bugs.webkit.org/show_bug.cgi?id=186859
>+        <rdar://problem/41093519>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add new private APIs for limiting the number of threads to be used by
>+        the DFG and FTL compilers. It was already possible to configure the
>+        limit through JSC Options, but now it can be changed at runtime, even
>+        in the case when the VM is already running.
>+
>+        Add a test for both cases: when trying to configure the limit before
>+        and after the Worklist has been created, but in order to simulate the
>+        first scenario, we must guarantee that the test runs at the very
>+        beginning, so I also added a check for that.
>+
>+        * API/JSVirtualMachine.mm:
>+        (+[JSVirtualMachine setNumberOfDFGCompilerThreads:]):
>+        (+[JSVirtualMachine setNumberOfFTLCompilerThreads:]):
>+        * API/JSVirtualMachinePrivate.h:
>+        * API/tests/testapi.mm:
>+        (runJITThreadLimitTests):
>+        (testObjectiveCAPIMain):
>+        * dfg/DFGWorklist.cpp:
>+        (JSC::DFG::Worklist::finishCreation):
>+        (JSC::DFG::Worklist::createNewThread):
>+        (JSC::DFG::Worklist::setNumberOfThreads):
>+        * dfg/DFGWorklist.h:
>+
> 2018-06-18  Keith Miller  <keith_miller@apple.com>
> 
>         JSImmutableButterfly should assert m_header is adjacent to the data
>diff --git a/Source/JavaScriptCore/API/JSVirtualMachine.mm b/Source/JavaScriptCore/API/JSVirtualMachine.mm
>index 1e20791b45a884bbdbbcb2a40cb724930e8c076b..30c37e2e5a8d9fca65dfecf52607b88b468bd762 100644
>--- a/Source/JavaScriptCore/API/JSVirtualMachine.mm
>+++ b/Source/JavaScriptCore/API/JSVirtualMachine.mm
>@@ -30,6 +30,7 @@
> #if JSC_OBJC_API_ENABLED
> 
> #import "APICast.h"
>+#import "DFGWorklist.h"
> #import "JSManagedValueInternal.h"
> #import "JSVirtualMachine.h"
> #import "JSVirtualMachineInternal.h"
>@@ -275,6 +276,32 @@ - (void)shrinkFootprintWhenIdle
>     vm->shrinkFootprintWhenIdle();
> }
> 
>+#if ENABLE(DFG_JIT)
>+
>++ (NSUInteger)setNumberOfDFGCompilerThreads:(NSUInteger)numberOfThreads
>+{
>+    JSC::DFG::Worklist* worklist = JSC::DFG::existingGlobalDFGWorklistOrNull();
>+    if (worklist)
>+        return worklist->setNumberOfThreads(numberOfThreads, JSC::Options::priorityDeltaOfDFGCompilerThreads());
>+
>+    auto currentNumberOfThreads = JSC::Options::numberOfDFGCompilerThreads();
>+    JSC::Options::numberOfDFGCompilerThreads() = numberOfThreads;
>+    return currentNumberOfThreads;
>+}
>+
>++ (NSUInteger)setNumberOfFTLCompilerThreads:(NSUInteger)numberOfThreads
>+{
>+    JSC::DFG::Worklist* worklist = JSC::DFG::existingGlobalFTLWorklistOrNull();
>+    if (worklist)
>+        return worklist->setNumberOfThreads(numberOfThreads, JSC::Options::priorityDeltaOfFTLCompilerThreads());
>+
>+    auto currentNumberOfThreads = JSC::Options::numberOfFTLCompilerThreads();
>+    JSC::Options::numberOfFTLCompilerThreads() = numberOfThreads;
>+    return currentNumberOfThreads;
>+}
>+
>+#endif // ENABLE(DFG_JIT)
>+
> @end
> 
> static void scanExternalObjectGraph(JSC::VM& vm, JSC::SlotVisitor& visitor, void* root, bool lockAcquired)
>diff --git a/Source/JavaScriptCore/API/JSVirtualMachinePrivate.h b/Source/JavaScriptCore/API/JSVirtualMachinePrivate.h
>index 486a67bec3fb0384b01b38f294225753337a5703..6faaf06fc504d76260ce51448b2251fef1a34ec2 100644
>--- a/Source/JavaScriptCore/API/JSVirtualMachinePrivate.h
>+++ b/Source/JavaScriptCore/API/JSVirtualMachinePrivate.h
>@@ -23,6 +23,7 @@
>  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  */
> 
>+#include "JSExportMacros.h"
> #include <JavaScriptCore/JavaScript.h>
> 
> #if JSC_OBJC_API_ENABLED
>@@ -44,6 +45,36 @@
> 
> - (void)shrinkFootprintWhenIdle NS_AVAILABLE(10_14, 12_0);
> 
>+#if ENABLE(DFG_JIT)
>+
>+/*!
>+@method
>+@abstract Set the number of threads to be used by the DFG JIT compiler.
>+@discussion If called after the VM has been initialized, it will terminate
>+ threads until it meets the new limit or create new threads accordingly if the
>+ new limit is higher than the previous limit. If called before initialization,
>+ the Options value for the number of DFG threads will be updated to ensure the
>+ DFG compiler already starts with the up-to-date limit.
>+@param numberOfThreads The number of threads the DFG compiler should use going forward
>+@result The previous number of threads being used by the DFG compiler
>+*/
>++ (NSUInteger)setNumberOfDFGCompilerThreads:(NSUInteger)numberOfThreads NS_AVAILABLE(10_14, 12_0);
>+
>+/*!
>+@method
>+@abstract Set the number of threads to be used by the FTL JIT compiler.
>+@discussion If called after the VM has been initialized, it will terminate
>+ threads until it meets the new limit or create new threads accordingly if the
>+ new limit is higher than the previous limit. If called before initialization,
>+ the Options value for the number of FTL threads will be updated to ensure the
>+ FTL compiler already starts with the up-to-date limit.
>+@param numberOfThreads The number of threads the FTL compiler should use going forward
>+@result The previous number of threads being used by the FTL compiler
>+*/
>++ (NSUInteger)setNumberOfFTLCompilerThreads:(NSUInteger)numberOfThreads NS_AVAILABLE(10_14, 12_0);
>+
>+#endif // ENABLE(DFG_JIT)
>+
> @end
> 
> #endif // JSC_OBJC_API_ENABLED
>diff --git a/Source/JavaScriptCore/API/tests/testapi.mm b/Source/JavaScriptCore/API/tests/testapi.mm
>index d7cc2656831bf54412e4d8779af53a349e2a093f..c4c223efb7e427cedbc0c572f2492378468d50ab 100644
>--- a/Source/JavaScriptCore/API/tests/testapi.mm
>+++ b/Source/JavaScriptCore/API/tests/testapi.mm
>@@ -23,9 +23,14 @@
>  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  */
> 
>+#import "JSExportMacros.h"
> #import <JavaScriptCore/JavaScriptCore.h>
> 
>+#undef NS_AVAILABLE
>+#define NS_AVAILABLE(_mac, _ios)
>+
> #import "CurrentThisInsideBlockGetterTest.h"
>+#import "DFGWorklist.h"
> #import "DateTests.h"
> #import "JSExportTests.h"
> #import "JSVirtualMachinePrivate.h"
>@@ -514,8 +519,41 @@ static void* multiVMThreadMain(void* okPtr)
>     return nullptr;
> }
> 
>+static void runJITThreadLimitTests()
>+{
>+    auto testDFG = [] {
>+        unsigned defaultNumberOfThreads = JSC::Options::numberOfDFGCompilerThreads();
>+        unsigned targetNumberOfThreads = 1;
>+        unsigned initialNumberOfThreads = [JSVirtualMachine setNumberOfDFGCompilerThreads:1];
>+        checkResult(@"Initial number of DFG threads should be the value provided through Options", initialNumberOfThreads == defaultNumberOfThreads);
>+        unsigned updatedNumberOfThreads = [JSVirtualMachine setNumberOfDFGCompilerThreads:initialNumberOfThreads];
>+        checkResult(@"Number of DFG threads should have been updated", updatedNumberOfThreads == targetNumberOfThreads);
>+    };
>+
>+    auto testFTL = [] {
>+        unsigned defaultNumberOfThreads = JSC::Options::numberOfFTLCompilerThreads();
>+        unsigned targetNumberOfThreads = 3;
>+        unsigned initialNumberOfThreads = [JSVirtualMachine setNumberOfFTLCompilerThreads:1];
>+        checkResult(@"Initial number of FTL threads should be the value provided through Options", initialNumberOfThreads == defaultNumberOfThreads);
>+        unsigned updatedNumberOfThreads = [JSVirtualMachine setNumberOfFTLCompilerThreads:initialNumberOfThreads];
>+        checkResult(@"Number of FTL threads should have been updated", updatedNumberOfThreads == targetNumberOfThreads);
>+    };
>+
>+    checkResult(@"runJITThreadLimitTests() must run at the very beginning to test the case where the global JIT worklist was not initialized yet", !JSC::DFG::existingGlobalDFGWorklistOrNull() && !JSC::DFG::existingGlobalFTLWorklistOrNull());
>+
>+    testDFG();
>+    JSC::DFG::ensureGlobalDFGWorklist();
>+    testDFG();
>+
>+    testFTL();
>+    JSC::DFG::ensureGlobalFTLWorklist();
>+    testFTL();
>+}
>+
> static void testObjectiveCAPIMain()
> {
>+    runJITThreadLimitTests();
>+
>     @autoreleasepool {
>         JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
>         JSContext* context = [[JSContext alloc] initWithVirtualMachine:vm];
>diff --git a/Source/JavaScriptCore/dfg/DFGWorklist.cpp b/Source/JavaScriptCore/dfg/DFGWorklist.cpp
>index 5bc88c4050aba9c6701d566e6d78e0849ea193fc..601fa8e32386493e2a23e76e91fbbf0451e07545 100644
>--- a/Source/JavaScriptCore/dfg/DFGWorklist.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGWorklist.cpp
>@@ -200,12 +200,17 @@ void Worklist::finishCreation(unsigned numberOfThreads, int relativePriority)
>     RELEASE_ASSERT(numberOfThreads);
>     LockHolder locker(*m_lock);
>     for (unsigned i = numberOfThreads; i--;) {
>-        std::unique_ptr<ThreadData> data = std::make_unique<ThreadData>(this);
>-        data->m_thread = adoptRef(new ThreadBody(locker, *this, *data, m_lock, m_planEnqueued, relativePriority));
>-        m_threads.append(WTFMove(data));
>+        createNewThread(locker, relativePriority);
>     }
> }
> 
>+void Worklist::createNewThread(const AbstractLocker& locker, int relativePriority)
>+{
>+    std::unique_ptr<ThreadData> data = std::make_unique<ThreadData>(this);
>+    data->m_thread = adoptRef(new ThreadBody(locker, *this, *data, m_lock, m_planEnqueued, relativePriority));
>+    m_threads.append(WTFMove(data));
>+}
>+
> Ref<Worklist> Worklist::create(CString worklistName, unsigned numberOfThreads, int relativePriority)
> {
>     Ref<Worklist> result = adoptRef(*new Worklist(worklistName));
>@@ -488,6 +493,40 @@ void Worklist::dump(const AbstractLocker&, PrintStream& out) const
>         ", Num Active Threads = ", m_numberOfActiveThreads, "/", m_threads.size(), "]");
> }
> 
>+unsigned Worklist::setNumberOfThreads(unsigned numberOfThreads, int relativePriority)
>+{
>+    LockHolder locker(m_suspensionLock);
>+    auto currentNumberOfThreads = m_threads.size();
>+    if (numberOfThreads < currentNumberOfThreads) {
>+        {
>+            LockHolder locker(*m_lock);
>+            for (unsigned i = currentNumberOfThreads; i-- > numberOfThreads;) {
>+                if (m_threads[i]->m_thread->hasUnderlyingThread(locker)) {
>+                    m_queue.append(nullptr);
>+                    m_threads[i]->m_thread->notify(locker);
>+                }
>+            }
>+        }
>+        for (unsigned i = currentNumberOfThreads; i-- > numberOfThreads;) {
>+            bool isStopped = false;
>+            {
>+                LockHolder locker(*m_lock);
>+                isStopped = m_threads[i]->m_thread->tryStop(locker);
>+            }
>+            if (!isStopped)
>+                m_threads[i]->m_thread->join();
>+            m_threads.remove(i);
>+        }
>+        m_threads.shrinkToFit();
>+        ASSERT(m_numberOfActiveThreads <= numberOfThreads);
>+    } else if (numberOfThreads > currentNumberOfThreads) {
>+        LockHolder locker(*m_lock);
>+        for (unsigned i = currentNumberOfThreads; i < numberOfThreads; i++)
>+            createNewThread(locker, relativePriority);
>+    }
>+    return currentNumberOfThreads;
>+}
>+
> static Worklist* theGlobalDFGWorklist;
> 
> Worklist& ensureGlobalDFGWorklist()
>diff --git a/Source/JavaScriptCore/dfg/DFGWorklist.h b/Source/JavaScriptCore/dfg/DFGWorklist.h
>index 41205594bd69c0e623ca28c914b3cf1bd3b3b631..0300b58eec2ea5745c5da01058b15f03e1dfb9c7 100644
>--- a/Source/JavaScriptCore/dfg/DFGWorklist.h
>+++ b/Source/JavaScriptCore/dfg/DFGWorklist.h
>@@ -79,10 +79,12 @@ public:
>     void removeNonCompilingPlansForVM(VM&);
>     
>     void dump(PrintStream&) const;
>+    unsigned setNumberOfThreads(unsigned, int);
>     
> private:
>     Worklist(CString worklistName);
>     void finishCreation(unsigned numberOfThreads, int);
>+    void createNewThread(const AbstractLocker&, int);
>     
>     class ThreadBody;
>     friend class ThreadBody;
>@@ -121,12 +123,12 @@ private:
> };
> 
> // For DFGMode compilations.
>-Worklist& ensureGlobalDFGWorklist();
>-Worklist* existingGlobalDFGWorklistOrNull();
>+JS_EXPORT_PRIVATE Worklist& ensureGlobalDFGWorklist();
>+JS_EXPORT_PRIVATE Worklist* existingGlobalDFGWorklistOrNull();
> 
> // For FTLMode and FTLForOSREntryMode compilations.
>-Worklist& ensureGlobalFTLWorklist();
>-Worklist* existingGlobalFTLWorklistOrNull();
>+JS_EXPORT_PRIVATE Worklist& ensureGlobalFTLWorklist();
>+JS_EXPORT_PRIVATE Worklist* existingGlobalFTLWorklistOrNull();
> 
> Worklist& ensureGlobalWorklistFor(CompilationMode);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 186859: 343185 | 343502