Running tasks in the debug terminal can be significantly slower

[connor4312]

Both @kieferrm and I noticed that running e.g. build commands in the PWA debugger terminal was significantly slower than a normal terminal. #111 may resolve the issue.

[roblourens]

Besides that, I think node just generally runs slower when the debugger is enabled.

[connor4312]

Yea, that's my fear. I'll look into it more after 111 is merged and see what we can do. Hopefully that's the cause, but if not hopefully it's still fixable.

[connor4312]

I took another look at this again with my latest predictor performance tweaks. It seems like running from the debug terminal is about twice as slow, from Node 10 to the latest 13.2 release.

With debugger:

  npm run build > /dev/null 26.17s user 1.78s system 132% cpu 21.014 total
  npm run build > /dev/null 26.02s user 1.71s system 135% cpu 20.533 total
  npm run build > /dev/null 26.71s user 1.79s system 133% cpu 21.299 total

Normal console:

  npm run build > /dev/null  14.87s user 1.13s system 152% cpu 10.479 total
  npm run build > /dev/null  15.14s user 1.13s system 151% cpu 10.768 total
  npm run build > /dev/null  15.07s user 1.16s system 151% cpu 10.687 total

In order to isolate the issue I put together a minimal debug client that connects right to the websocket (below). To my surprise, turning Debugger.enable and Runtime.enable have no noticable effect on the runtime, taking about 12 seconds consistently (faster than the terminal above, since this measures after the node binary is already running and some internals loaded).

However, enabling async stack traces immediately bumps the runtime up to around 20 seconds--and disabling them in the debug adapter moves the above time to ~17.5s. These appear to be responsible for the majority of the performance delta.

I don't think this is avoidable. setAsyncCallStackDepth is not configurable, and it's not something we can do without by default. Maybe we simply don't turn it on until the first breakpoint is set in the program? Unsure if this will have negative side effects.

const WebSocket = require('ws');
const ws = new WebSocket(require('fs').readFileSync(0, 'utf-8'));

const start = Date.now();
ws.on('open', () => {
  ws.send('{"id": 1,"method": "Debugger.enable","params": {}}');
  ws.send('{"id": 2,"method": "Runtime.enable","params": {}}');
  ws.send('{"id": 3,"method": "Debugger.setAsyncCallStackDepth","params": {"maxDepth":32}}');
  ws.send('{"id": 4,"method": "Debugger.setPauseOnExceptions","params": {"state":"none"}}');
  ws.send('{"id": 5,"method": "Runtime.runIfWaitingForDebugger","params": {}}');
});

ws.on('message', msg => {
  if (msg.includes('Break on start')) {
    ws.send('{"id": 6,"method": "Debugger.resume","params": {}}');
  } else if (msg.includes('Runtime.executionContextDestroyed')) {
    console.log(Date.now() - start, 'ms');
    ws.close();
  }
});

[roblourens]

That's really interesting. I didn't realize the impact was that bad. Yeah I would support finding a way to lazily enable it. The problem is that it won't be capturing the async callstacks until we enable it, and users will miss async frames from that period of time.

[roblourens]

What do you mean setAsyncCallStackDepth is not configurable, does maxDepth not have an effect?

[connor4312]

I meant that the depth is the only thing we can configure :) The depth does seem to have some effect, setting it to 4 makes the runtime 17.5s down from 20s.

I think it might be worth having the async call stack depth set lazily by default for terminal programs--or perhaps all programs, even--with an option to change the defaults of course.

[connor4312]

@roblourens what do you think of this?

• Update showAsyncStacks to

showAsyncStacks: boolean | { eager: number } | { lazy: number };

• { eager: number } configures the async call stack depth. true aliases to the current { eager: 32 }
• { lazy: number } will trigger a call to async call stack depth only when a CDP notifies us that a breakpoint is resolved, or we hit a debugger statement, whichever happens first
• Update the debugger terminal configuration to default to { lazy: 32 }

This should allow build tasks like webpack to run at full speed (if the user is not intended to debug them) while still preserving stacks in scenarios where we think they might hit a breakpoint.

The downside is that if they hit a debugger statement or hit a breakpoint immediately after a lazy script load, they may not get the stack.

[roblourens]

Yeah, I think that's a reasonable tradeoff. It probably doesn't need to be exposed to the user, since if you have a launch config, you probably want the default, and in the terminal you don't have a launch config.

[connor4312]

I think a way to toggle lazy mode off would be useful in case it causes issues for some people. In #221 the terminal defaults can be configured via debug.javascript.terminalOptions.

I'll get to work on this.