Bug 122847 – Enable DFG for the Debugger and Profiler

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

With https://bugs.webkit.org/show_bug.cgi?id=122844, we no longer need to constantly check if we're stepping when the Debugger is in Running mode.  Hence, we should be able to enable the DFG.  However, there are a few changes that still need to be made:

1. Prevent a function from being DFG compiled if there's a breakpoint set in it.
2. Prevent a function from being inlined by the DFG if there's a breakpoint set in it.

3. When in Stepping mode and stepping into the a function call, in the call slow path, the Debugger should de-opt the callee if it is DFG compiled and mark is as temporarily not compilable and inlineable by the DFG.

4. When in Stepping mode and stepping out (i.e. returning to a caller),  OSR de-opt the caller if it is DFG compiled and mark is as temporarily not compilable and inlineable by the DFG.

The philosophy here is that script that is being debugged will not run DFG generated code.  If we're done stepping through that script, the DFG may optimize it again thereafter if needed provided that there is not a breakpoint set in it.

This approach has several benefits:

1. the DFG compiled code like it normally does.  op_debugs are phantom nodes.
2. the Debugger is simpler: no need to support breakpoints in the DFG compiled code.

Hence, we should be able to performance get near parity with the Debugger not enabled when we're not in Stepping mode.  The assumption here is that there are no breakpoints set in fast path functions.  If there are, then we'll get baseline JIT speeds for those functions. 

The challenging part:

1. We need to be able OSR de-opt a DFG function at the point where we return to it.
    1.1 This includes the case where the caller frame is an inlined frame.

If it is too difficult to support OSR de-opt of inlined frames on returns, then we'll only enable the DFG without inlining for a first pass.

[Radar WebKit Bug Importer]

<rdar://problem/15232637>

[Mark Lam]

We're changing strategy from the one previously documented.  Here's the current strategy for this first step in turning on the DFG:

We will implement DFG op_debug as a series of 3 checks:
 1. Check if the debugger pointer is non-null.  This is needed in case the debugger has been detached but the DFG code is still running on the stack.
2. Check if Debugger::m_shouldPause is true.
3. Check if CodeBlock::m_numBreakpoints is non-zero.

These are the same 3 checks done in the LLINT and baselineJIT.  But unlike the LLINT and baselineJIT, these DFG checks are implemented as speculationChecks.  If the check fails, we OSR exit to the baselineJIT and let it do the work of servicing the op_debug callback.

Stepping through code in the debugger would work the same way.  The top function being debugged has to be a LLINT or baselineJIT function because we would have OSR exited if there is a breakpoint in that function.  When we step out of that function to its caller, we expect that the caller will call back to the debugger at the next op_debug.  If the caller function is a DFG function, the op_debug site will fail its speculation check on Debugger::m_shouldPause and deopt into a baselineJIT function.  Execution continues from there as usual, and the debugger gets its callback.

For the profile, op_profile_will_call and op_profile_did_call are implemented as simple runtime calls to service the profiler.

With this patch, Octane performance with the WebInspector open (with or without breakpoints set) jump from ~2000 to ~2500 (25% progression). YMMV.

[Mark Lam]

Created attachment 222022 [details]
the patch.

This patch has passed all layout tests on X86_64.  It is currently being built and tested on 32-bit X86.

[Geoffrey Garen]

Comment on attachment 222022 [details]
the patch.

r=me

The next two improvements you should make are:

(1) Remove the speculation checks for debugger and "shouldPause", and change the debugger to add a breakpoint to every code block when it starts stepping, and remove them all when it stops stepping.

(2) Change the *_profile_* opcodes to speculation check a boolean that indicates whether a profiler is currently recording. (i.e., an attached profiler that is not currently recording should take the fast path.)

[Geoffrey Garen]

(3) Change the speculation checks into watchpoints.

[Mark Lam]

Thanks.  Landed in r162652: <http://trac.webkit.org/r162652>.