Bug 115134 – Polling based watchdog for the DFG

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Getting the DFG in on the watchdog action.  The strategy is to emit a speculation check for the watchdog timer firing.  If we have a watchdog timer fire, we'll OSR exit and let the baseline JIT do the dirty work.  Since this is a rare and probably near terminal condition, the perf of using an OSR exit to handle the watchdog timer is not such a big issue.

Of greater interest is the perf when the watchdog is enabled but not firing (compared to watchdog disabled as a baseline).  Benchmark results show that overall hit (on all benchmarks combined) is between 1 - 2%.  Some individual benchmarks e.g. temporal-structure shows a big drop.  However, disassembly of the DFG generated code and profiling info shows that the only difference is that the loops are executing the extra polling checks for the watchdog timer.  Nothing else has changed.  Hence the perf difference is due to cache alignment or other random factors.

Patch coming soon.

[Mark Lam]

ref: <rdar://problem/4115952>

[Mark Lam]

Created attachment 199524 [details]
benchmark result: run 1: DFG enabled, watchdog enabled.

[Mark Lam]

Created attachment 199525 [details]
benchmark result: run 2: DFG enabled, watchdog enabled.

[Mark Lam]

Created attachment 199527 [details]
the patch.

To be clear, the watchdog's setTimeLimit() still needs to be called before scripts are executed (or more strictly speaking ... when no JS frames are on the JS stack) in order for the script code to have the watchdog timer checks emitted and thereby be able to time out.

[WebKit Commit Bot]

Attachment 199527 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/bytecode/ExitKind.h', u'Source/JavaScriptCore/dfg/DFGAbstractState.cpp', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGFixupPhase.cpp', u'Source/JavaScriptCore/dfg/DFGNodeType.h', u'Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp', u'Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp', u'Source/JavaScriptCore/runtime/Watchdog.cpp']" exit_code: 1
Source/JavaScriptCore/dfg/DFGNodeType.h:269:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/JavaScriptCore/dfg/DFGNodeType.h:270:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 199527 [details]
the patch.

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

r=me

> Source/JavaScriptCore/runtime/Watchdog.cpp:82
>          // And if we've previously compiled any functions, we need to deopt

"revert" is a better word than "deopt" here -- we're not necessarily forcing a less optimal form -- we're just throwing away the form we have right now, and letting the engine choose whatever form it chooses the next time the function executes.

> Source/JavaScriptCore/runtime/Watchdog.cpp:83
>          // them because they don't habe the needed polling checks yet.

Typo: "habe".

[Geoffrey Garen]

Is our current API test sufficient to enter the DFG and exercise this code?

[Mark Lam]

Created attachment 199609 [details]
updated patch w/ Geoff's feedback addressed, and the API test updated.  Will go ahead and land this version.

[Mark Lam]

Landed in r149089: <http://trac.webkit.org/changeset/149089>.