Summary: | It should be possible to adjust DFG and FTL compiler thread priorities | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||
Status: | RESOLVED WONTFIX | ||||||||
Severity: | Normal | CC: | benjamin, cmarcelo, commit-queue, ggaren | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Michael Saboff
2014-03-14 22:53:51 PDT
Created attachment 226811 [details]
Patch
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. Comment on attachment 226811 [details]
Patch
R=me, but what Geoff said.
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.
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. 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. |