Bug 130288 – It should be possible to adjust DFG and FTL compiler thread priorities

• Status: RESOLVED
• Resolution: WONTFIX
• Component: JavaScriptCore

[Michael Saboff]

On systems with 1 or 2 cores, the DFG and FTL compiler threads might compete with each other or the main thread.  By lowering the thread priorities of the compilation threads, the main thread would get higher priority.  Given the nature of the DFG and FTL, it makes sense to elevate the DFG compiler threads above the FTL compiler threads.

[Michael Saboff]

Created attachment 226811 [details]
Patch

[Geoffrey Garen]

Comment on attachment 226811 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=226811&action=review

> Source/WTF/ChangeLog:10
> +        pools.  For two core systems, there might be thread runnable threads, the main thread,

I think you meant "three" and not "thread".

> Source/WTF/ChangeLog:14
> +        compilation threads, the main thread can run.  Further tests may suggest better values
> +        for the new options, priorityDeltaOfDFGCompilerThreads and priorityDeltaOfFTLCompilerThreads.

What are the consequences of the values in this patch? We don't usually make performance changes without comment on benchmark effects.

> Source/WTF/wtf/ThreadingPthreads.cpp:207
> +void changedThreadPriority(ThreadIdentifier threadID, int delta)

I think you meant "changeThreadPriority".

> Source/WTF/wtf/ThreadingWin.cpp:249
> +void changedThreadPriority(ThreadIdentifier threadID, int delta)

Ditto.

[Filip Pizlo]

Comment on attachment 226811 [details]
Patch

R=me, but what Geoff said.

[Michael Saboff]

Created attachment 226823 [details]
Updated patch for landing

Made suggested changes.  Fixed windows build.  Added performance benefit of the change to the ChangeLogs.

This changes gives a net performance improvement between 1-3% on a 2 core device across SunSpider, Octane, Kraken and AsmBench.

[Geoffrey Garen]

This patch was wrong because it set the priority of pthread_self() instead of the passed-in thread. It was fixed in <http://trac.webkit.org/changeset/165691>, netting a much bigger speedup.

[Csaba Osztrogonác]

Comment on attachment 226811 [details]
Patch

Cleared Filip Pizlo's review+ from obsolete attachment 226811 [details] so that this bug does not appear in http://webkit.org/pending-commit.